diff --git a/src/org/kde/kdeconnect/Device.java b/src/org/kde/kdeconnect/Device.java index 9216bb28..f4bd240d 100644 --- a/src/org/kde/kdeconnect/Device.java +++ b/src/org/kde/kdeconnect/Device.java @@ -531,7 +531,7 @@ public class Device implements BaseLink.PacketReceiver { @Override public void onPacketReceived(@NonNull NetworkPacket np) { - PacketStats.countReceived(getDeviceId(), np.getType()); + DeviceStats.countReceived(getDeviceId(), np.getType()); if (NetworkPacket.PACKET_TYPE_PAIR.equals(np.getType())) { @@ -694,7 +694,7 @@ public class Device implements BaseLink.PacketReceiver { } catch (IOException e) { e.printStackTrace(); } - PacketStats.countSent(getDeviceId(), np.getType(), success); + DeviceStats.countSent(getDeviceId(), np.getType(), success); if (success) break; } diff --git a/src/org/kde/kdeconnect/PacketStats.java b/src/org/kde/kdeconnect/DeviceStats.java similarity index 56% rename from src/org/kde/kdeconnect/PacketStats.java rename to src/org/kde/kdeconnect/DeviceStats.java index c25cdf98..2a9c2ca4 100644 --- a/src/org/kde/kdeconnect/PacketStats.java +++ b/src/org/kde/kdeconnect/DeviceStats.java @@ -5,6 +5,7 @@ import android.util.Log; import androidx.annotation.NonNull; import androidx.annotation.RequiresApi; +import androidx.annotation.VisibleForTesting; import java.util.ArrayList; import java.util.Collection; @@ -14,67 +15,74 @@ import java.util.Iterator; import java.util.Map; import java.util.concurrent.TimeUnit; -public class PacketStats { +public class DeviceStats { - private static final long EVENT_KEEP_WINDOW_MILLIS = 24 * 60 * 60 * 1000; // Keep 24 hours of events - private static final long CLEANUP_INTERVAL_MILLIS = EVENT_KEEP_WINDOW_MILLIS/4; // Delete old (>24 hours) events every 6 hours + /** + * Keep 24 hours of events + */ + private static final long EVENT_KEEP_WINDOW_MILLIS = 24 * 60 * 60 * 1000; - static class DeviceEvents { + /** + * Delete old (>24 hours, see EVENT_KEEP_WINDOW_MILLIS) events every 6 hours + */ + private static final long CLEANUP_INTERVAL_MILLIS = EVENT_KEEP_WINDOW_MILLIS/4; + + private final static HashMap eventsByDevice = new HashMap<>(); + private static long nextCleanup = System.currentTimeMillis() + CLEANUP_INTERVAL_MILLIS; + + static class PacketStats { public long createdAtMillis = System.currentTimeMillis(); public HashMap> receivedByType = new HashMap<>(); public HashMap> sentSuccessfulByType = new HashMap<>(); public HashMap> sentFailedByType = new HashMap<>(); - static class Counts { - @NonNull String packetType; + static class Summary { + final @NonNull String packetType; int received = 0; int sentSuccessful = 0; int sentFailed = 0; int total = 0; - Counts(@NonNull String packetType) { + Summary(@NonNull String packetType) { this.packetType = packetType; } } @RequiresApi(api = Build.VERSION_CODES.N) - public @NonNull Collection getCounts() { - HashMap countsByType = new HashMap<>(); + public @NonNull Collection getSummaries() { + HashMap countsByType = new HashMap<>(); for (Map.Entry> entry : receivedByType.entrySet()) { - Counts counts = countsByType.computeIfAbsent(entry.getKey(), Counts::new); - counts.received += entry.getValue().size(); - counts.total += entry.getValue().size(); + Summary summary = countsByType.computeIfAbsent(entry.getKey(), Summary::new); + summary.received += entry.getValue().size(); + summary.total += entry.getValue().size(); } for (Map.Entry> entry : sentSuccessfulByType.entrySet()) { - Counts counts = countsByType.computeIfAbsent(entry.getKey(), Counts::new); - counts.sentSuccessful += entry.getValue().size(); - counts.total += entry.getValue().size(); + Summary summary = countsByType.computeIfAbsent(entry.getKey(), Summary::new); + summary.sentSuccessful += entry.getValue().size(); + summary.total += entry.getValue().size(); } for (Map.Entry> entry : sentFailedByType.entrySet()) { - Counts counts = countsByType.computeIfAbsent(entry.getKey(), Counts::new); - counts.sentFailed += entry.getValue().size(); - counts.total += entry.getValue().size(); + Summary summary = countsByType.computeIfAbsent(entry.getKey(), Summary::new); + summary.sentFailed += entry.getValue().size(); + summary.total += entry.getValue().size(); } return countsByType.values(); } } - private final static HashMap eventsByDevice = new HashMap<>(); - private static long nextCleanup = System.currentTimeMillis() + CLEANUP_INTERVAL_MILLIS; - @RequiresApi(api = Build.VERSION_CODES.N) public static @NonNull String getStatsForDevice(@NonNull String deviceId) { cleanupIfNeeded(); - DeviceEvents deviceEvents = eventsByDevice.get(deviceId); - if (deviceEvents == null) { + PacketStats packetStats = eventsByDevice.get(deviceId); + if (packetStats == null) { return ""; } StringBuilder ret = new StringBuilder(); - long timeInMillis = System.currentTimeMillis() - deviceEvents.createdAtMillis; + long timeInMillis = System.currentTimeMillis() - packetStats.createdAtMillis; if (timeInMillis > EVENT_KEEP_WINDOW_MILLIS) { timeInMillis = EVENT_KEEP_WINDOW_MILLIS; } @@ -86,10 +94,10 @@ public class PacketStats { ret.append(minutes); ret.append("m\n\n"); - ArrayList counts = new ArrayList<>(deviceEvents.getCounts()); + ArrayList counts = new ArrayList<>(packetStats.getSummaries()); Collections.sort(counts, (o1, o2) -> Integer.compare(o2.total, o1.total)); // Sort them by total number of events - for (DeviceEvents.Counts count : counts) { + for (PacketStats.Summary count : counts) { String name = count.packetType; if (name.startsWith("kdeconnect.")) { name = name.substring("kdeconnect.".length()); @@ -111,7 +119,13 @@ public class PacketStats { if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.N) { return; // computeIfAbsent not present in API < 24 } - eventsByDevice.computeIfAbsent(deviceId, key -> new DeviceEvents()).receivedByType.computeIfAbsent(packetType, key -> new ArrayList<>()).add(System.currentTimeMillis()); + synchronized (DeviceStats.class) { + eventsByDevice + .computeIfAbsent(deviceId, key -> new PacketStats()) + .receivedByType + .computeIfAbsent(packetType, key -> new ArrayList<>()) + .add(System.currentTimeMillis()); + } cleanupIfNeeded(); } @@ -120,9 +134,21 @@ public class PacketStats { return; // computeIfAbsent not present in API < 24 } if (success) { - eventsByDevice.computeIfAbsent(deviceId, key -> new DeviceEvents()).sentSuccessfulByType.computeIfAbsent(packetType, key -> new ArrayList<>()).add(System.currentTimeMillis()); + synchronized (DeviceStats.class) { + eventsByDevice + .computeIfAbsent(deviceId, key -> new PacketStats()) + .sentSuccessfulByType + .computeIfAbsent(packetType, key -> new ArrayList<>()) + .add(System.currentTimeMillis()); + } } else { - eventsByDevice.computeIfAbsent(deviceId, key ->new DeviceEvents()).sentFailedByType.computeIfAbsent(packetType, key -> new ArrayList<>()).add(System.currentTimeMillis()); + synchronized (DeviceStats.class) { + eventsByDevice + .computeIfAbsent(deviceId, key -> new PacketStats()) + .sentFailedByType + .computeIfAbsent(packetType, key -> new ArrayList<>()) + .add(System.currentTimeMillis()); + } } cleanupIfNeeded(); } @@ -130,16 +156,19 @@ public class PacketStats { private static void cleanupIfNeeded() { final long cutoutTimestamp = System.currentTimeMillis() - EVENT_KEEP_WINDOW_MILLIS; if (System.currentTimeMillis() > nextCleanup) { - Log.i("PacketStats", "Doing periodic cleanup"); - for (DeviceEvents de : eventsByDevice.values()) { - removeOldEvents(de.receivedByType, cutoutTimestamp); - removeOldEvents(de.sentFailedByType, cutoutTimestamp); - removeOldEvents(de.sentSuccessfulByType, cutoutTimestamp); + synchronized (DeviceStats.class) { + Log.i("PacketStats", "Doing periodic cleanup"); + for (PacketStats de : eventsByDevice.values()) { + removeOldEvents(de.receivedByType, cutoutTimestamp); + removeOldEvents(de.sentFailedByType, cutoutTimestamp); + removeOldEvents(de.sentSuccessfulByType, cutoutTimestamp); + } + nextCleanup = System.currentTimeMillis() + CLEANUP_INTERVAL_MILLIS; } - nextCleanup = System.currentTimeMillis() + CLEANUP_INTERVAL_MILLIS; } } + @VisibleForTesting static void removeOldEvents(HashMap> eventsByType, final long cutoutTimestamp) { Iterator>> iterator = eventsByType.entrySet().iterator(); @@ -149,13 +178,13 @@ public class PacketStats { int index = Collections.binarySearch(events, cutoutTimestamp); if (index < 0) { - index = -(index + 1); // Convert the negative index to insertion point + index = -(index + 1); // Convert the negative index to insertion point } if (index < events.size()) { events.subList(0, index).clear(); } else { - iterator.remove(); // No element greater than the threshold + iterator.remove(); // No element greater than the threshold } } } diff --git a/src/org/kde/kdeconnect/UserInterface/PluginSettingsActivity.java b/src/org/kde/kdeconnect/UserInterface/PluginSettingsActivity.java index 126b56c7..7baaf22e 100644 --- a/src/org/kde/kdeconnect/UserInterface/PluginSettingsActivity.java +++ b/src/org/kde/kdeconnect/UserInterface/PluginSettingsActivity.java @@ -10,7 +10,10 @@ import android.os.Build; import android.os.Bundle; import android.view.Menu; import android.view.MenuItem; +import android.view.View; +import android.widget.TextView; +import androidx.appcompat.app.AlertDialog; import androidx.appcompat.app.AppCompatActivity; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; @@ -18,8 +21,8 @@ import androidx.fragment.app.FragmentManager; import com.google.android.material.dialog.MaterialAlertDialogBuilder; import org.kde.kdeconnect.Device; +import org.kde.kdeconnect.DeviceStats; import org.kde.kdeconnect.KdeConnect; -import org.kde.kdeconnect.PacketStats; import org.kde.kdeconnect.Plugins.Plugin; import org.kde.kdeconnect_tp.R; @@ -101,12 +104,16 @@ public class PluginSettingsActivity return false; // PacketStats not working in API < 24 } menu.add(R.string.plugin_stats).setOnMenuItemClickListener(item -> { - String stats = PacketStats.getStatsForDevice(deviceId); - MaterialAlertDialogBuilder builder = new MaterialAlertDialogBuilder(PluginSettingsActivity.this); - builder.setTitle(R.string.plugin_stats); - builder.setPositiveButton(R.string.ok, (dialog, which) -> dialog.dismiss()); - builder.setMessage(stats); - builder.show(); + String stats = DeviceStats.getStatsForDevice(deviceId); + AlertDialog alertDialog = new MaterialAlertDialogBuilder(PluginSettingsActivity.this) + .setTitle(R.string.plugin_stats) + .setPositiveButton(R.string.ok, (dialog, which) -> dialog.dismiss()) + .setMessage(stats) + .show(); + View messageView = alertDialog.findViewById(android.R.id.message); + if (messageView instanceof TextView) { + ((TextView) messageView).setTextIsSelectable(true); + } return true; }); return true; diff --git a/tests/org/kde/kdeconnect/PacketStatsTest.java b/tests/org/kde/kdeconnect/DeviceStatsTest.java similarity index 86% rename from tests/org/kde/kdeconnect/PacketStatsTest.java rename to tests/org/kde/kdeconnect/DeviceStatsTest.java index 0aef8d02..60a3ee16 100644 --- a/tests/org/kde/kdeconnect/PacketStatsTest.java +++ b/tests/org/kde/kdeconnect/DeviceStatsTest.java @@ -2,14 +2,11 @@ package org.kde.kdeconnect; import org.junit.Assert; import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.modules.junit4.PowerMockRunner; import java.util.ArrayList; import java.util.HashMap; -@RunWith(PowerMockRunner.class) -public class PacketStatsTest { +public class DeviceStatsTest { @Test public void removeOldEvents_cutoutExists() { @@ -21,7 +18,7 @@ public class PacketStatsTest { events.add(20L); events.add(30L); final long cutout = 20L; - PacketStats.removeOldEvents(eventsByType, cutout); + DeviceStats.removeOldEvents(eventsByType, cutout); ArrayList eventsAfter = eventsByType.get(key); Assert.assertNotNull(eventsAfter); Assert.assertEquals(2, eventsAfter.size()); @@ -39,7 +36,7 @@ public class PacketStatsTest { events.add(20L); events.add(30L); final long cutout = 25L; - PacketStats.removeOldEvents(eventsByType, cutout); + DeviceStats.removeOldEvents(eventsByType, cutout); ArrayList eventsAfter = eventsByType.get(key); Assert.assertNotNull(eventsAfter); Assert.assertEquals(1, eventsAfter.size()); @@ -55,7 +52,7 @@ public class PacketStatsTest { events.add(10L); events.add(20L); final long cutout = 25L; - PacketStats.removeOldEvents(eventsByType, cutout); + DeviceStats.removeOldEvents(eventsByType, cutout); ArrayList eventsAfter = eventsByType.get(key); Assert.assertNull(eventsAfter); } @@ -68,7 +65,7 @@ public class PacketStatsTest { eventsByType.put(key, events); events.add(10L); final long cutout = 5L; - PacketStats.removeOldEvents(eventsByType, cutout); + DeviceStats.removeOldEvents(eventsByType, cutout); ArrayList eventsAfter = eventsByType.get(key); Assert.assertNotNull(eventsAfter); Assert.assertEquals(1, eventsAfter.size());