mir/kdeconnect-android
2
0
Fork 0
mirror of https://github.com/KDE/kdeconnect-android synced 2025-08-22 09:58:08 +00:00
Code Issues Releases Wiki Activity

Changes from code review

Browse Source
This commit is contained in:
Albert Vaca Cintora 2023-06-04 19:40:34 +02:00
parent 1ccf15010e
commit 476304d6fb
4 changed files with 87 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/org/kde/kdeconnect/Device.java
View File

@ -531,7 +531,7 @@ public class Device implements BaseLink.PacketReceiver {
@Override @Override
public void onPacketReceived(@NonNull NetworkPacket np) { public void onPacketReceived(@NonNull NetworkPacket np) {
PacketStats.countReceived(getDeviceId(), np.getType()); DeviceStats.countReceived(getDeviceId(), np.getType());
if (NetworkPacket.PACKET_TYPE_PAIR.equals(np.getType())) { if (NetworkPacket.PACKET_TYPE_PAIR.equals(np.getType())) {
@ -694,7 +694,7 @@ public class Device implements BaseLink.PacketReceiver {
} catch (IOException e) { } catch (IOException e) {
e.printStackTrace(); e.printStackTrace();
} }
PacketStats.countSent(getDeviceId(), np.getType(), success); DeviceStats.countSent(getDeviceId(), np.getType(), success);
if (success) break; if (success) break;
} }

103
src/org/kde/kdeconnect/PacketStats.java → src/org/kde/kdeconnect/DeviceStats.java
View File

@ -5,6 +5,7 @@ import android.util.Log;
import androidx.annotation.NonNull; import androidx.annotation.NonNull;
import androidx.annotation.RequiresApi; import androidx.annotation.RequiresApi;
import androidx.annotation.VisibleForTesting;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
@ -14,67 +15,74 @@ import java.util.Iterator;
import java.util.Map; import java.util.Map;
import java.util.concurrent.TimeUnit; 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<String, PacketStats> eventsByDevice = new HashMap<>();
private static long nextCleanup = System.currentTimeMillis() + CLEANUP_INTERVAL_MILLIS;
static class PacketStats {
public long createdAtMillis = System.currentTimeMillis(); public long createdAtMillis = System.currentTimeMillis();
public HashMap<String, ArrayList<Long>> receivedByType = new HashMap<>(); public HashMap<String, ArrayList<Long>> receivedByType = new HashMap<>();
public HashMap<String, ArrayList<Long>> sentSuccessfulByType = new HashMap<>(); public HashMap<String, ArrayList<Long>> sentSuccessfulByType = new HashMap<>();
public HashMap<String, ArrayList<Long>> sentFailedByType = new HashMap<>(); public HashMap<String, ArrayList<Long>> sentFailedByType = new HashMap<>();
static class Counts { static class Summary {
@NonNull String packetType; final @NonNull String packetType;
int received = 0; int received = 0;
int sentSuccessful = 0; int sentSuccessful = 0;
int sentFailed = 0; int sentFailed = 0;
int total = 0; int total = 0;
Counts(@NonNull String packetType) { Summary(@NonNull String packetType) {
this.packetType = packetType; this.packetType = packetType;
} }
} }
@RequiresApi(api = Build.VERSION_CODES.N) @RequiresApi(api = Build.VERSION_CODES.N)
public @NonNull Collection<Counts> getCounts() { public @NonNull Collection<Summary> getSummaries() {
HashMap<String, Counts> countsByType = new HashMap<>(); HashMap<String, Summary> countsByType = new HashMap<>();
for (Map.Entry<String, ArrayList<Long>> entry : receivedByType.entrySet()) { for (Map.Entry<String, ArrayList<Long>> entry : receivedByType.entrySet()) {
Counts counts = countsByType.computeIfAbsent(entry.getKey(), Counts::new); Summary summary = countsByType.computeIfAbsent(entry.getKey(), Summary::new);
counts.received += entry.getValue().size(); summary.received += entry.getValue().size();
counts.total += entry.getValue().size(); summary.total += entry.getValue().size();
} }
for (Map.Entry<String, ArrayList<Long>> entry : sentSuccessfulByType.entrySet()) { for (Map.Entry<String, ArrayList<Long>> entry : sentSuccessfulByType.entrySet()) {
Counts counts = countsByType.computeIfAbsent(entry.getKey(), Counts::new); Summary summary = countsByType.computeIfAbsent(entry.getKey(), Summary::new);
counts.sentSuccessful += entry.getValue().size(); summary.sentSuccessful += entry.getValue().size();
counts.total += entry.getValue().size(); summary.total += entry.getValue().size();
} }
for (Map.Entry<String, ArrayList<Long>> entry : sentFailedByType.entrySet()) { for (Map.Entry<String, ArrayList<Long>> entry : sentFailedByType.entrySet()) {
Counts counts = countsByType.computeIfAbsent(entry.getKey(), Counts::new); Summary summary = countsByType.computeIfAbsent(entry.getKey(), Summary::new);
counts.sentFailed += entry.getValue().size(); summary.sentFailed += entry.getValue().size();
counts.total += entry.getValue().size(); summary.total += entry.getValue().size();
} }
return countsByType.values(); return countsByType.values();
} }
} }
private final static HashMap<String, DeviceEvents> eventsByDevice = new HashMap<>();
private static long nextCleanup = System.currentTimeMillis() + CLEANUP_INTERVAL_MILLIS;
@RequiresApi(api = Build.VERSION_CODES.N) @RequiresApi(api = Build.VERSION_CODES.N)
public static @NonNull String getStatsForDevice(@NonNull String deviceId) { public static @NonNull String getStatsForDevice(@NonNull String deviceId) {
cleanupIfNeeded(); cleanupIfNeeded();
DeviceEvents deviceEvents = eventsByDevice.get(deviceId); PacketStats packetStats = eventsByDevice.get(deviceId);
if (deviceEvents == null) { if (packetStats == null) {
return ""; return "";
} }
StringBuilder ret = new StringBuilder(); StringBuilder ret = new StringBuilder();
long timeInMillis = System.currentTimeMillis() - deviceEvents.createdAtMillis; long timeInMillis = System.currentTimeMillis() - packetStats.createdAtMillis;
if (timeInMillis > EVENT_KEEP_WINDOW_MILLIS) { if (timeInMillis > EVENT_KEEP_WINDOW_MILLIS) {
timeInMillis = EVENT_KEEP_WINDOW_MILLIS; timeInMillis = EVENT_KEEP_WINDOW_MILLIS;
} }
@ -86,10 +94,10 @@ public class PacketStats {
ret.append(minutes); ret.append(minutes);
ret.append("m\n\n"); ret.append("m\n\n");
ArrayList<DeviceEvents.Counts> counts = new ArrayList<>(deviceEvents.getCounts()); ArrayList<PacketStats.Summary> counts = new ArrayList<>(packetStats.getSummaries());
Collections.sort(counts, (o1, o2) -> Integer.compare(o2.total, o1.total)); // Sort them by total number of events 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; String name = count.packetType;
if (name.startsWith("kdeconnect.")) { if (name.startsWith("kdeconnect.")) {
name = name.substring("kdeconnect.".length()); name = name.substring("kdeconnect.".length());
@ -111,7 +119,13 @@ public class PacketStats {
if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.N) { if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.N) {
return; // computeIfAbsent not present in API < 24 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(); cleanupIfNeeded();
} }
@ -120,9 +134,21 @@ public class PacketStats {
return; // computeIfAbsent not present in API < 24 return; // computeIfAbsent not present in API < 24
} }
if (success) { 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 { } 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(); cleanupIfNeeded();
} }
@ -130,16 +156,19 @@ public class PacketStats {
private static void cleanupIfNeeded() { private static void cleanupIfNeeded() {
final long cutoutTimestamp = System.currentTimeMillis() - EVENT_KEEP_WINDOW_MILLIS; final long cutoutTimestamp = System.currentTimeMillis() - EVENT_KEEP_WINDOW_MILLIS;
if (System.currentTimeMillis() > nextCleanup) { if (System.currentTimeMillis() > nextCleanup) {
Log.i("PacketStats", "Doing periodic cleanup"); synchronized (DeviceStats.class) {
for (DeviceEvents de : eventsByDevice.values()) { Log.i("PacketStats", "Doing periodic cleanup");
removeOldEvents(de.receivedByType, cutoutTimestamp); for (PacketStats de : eventsByDevice.values()) {
removeOldEvents(de.sentFailedByType, cutoutTimestamp); removeOldEvents(de.receivedByType, cutoutTimestamp);
removeOldEvents(de.sentSuccessfulByType, 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<String, ArrayList<Long>> eventsByType, final long cutoutTimestamp) { static void removeOldEvents(HashMap<String, ArrayList<Long>> eventsByType, final long cutoutTimestamp) {
Iterator<Map.Entry<String, ArrayList<Long>>> iterator = eventsByType.entrySet().iterator(); Iterator<Map.Entry<String, ArrayList<Long>>> iterator = eventsByType.entrySet().iterator();
@ -149,13 +178,13 @@ public class PacketStats {
int index = Collections.binarySearch(events, cutoutTimestamp); int index = Collections.binarySearch(events, cutoutTimestamp);
if (index < 0) { 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()) { if (index < events.size()) {
events.subList(0, index).clear(); events.subList(0, index).clear();
} else { } else {
iterator.remove(); // No element greater than the threshold iterator.remove(); // No element greater than the threshold
} }
} }
} }

21
src/org/kde/kdeconnect/UserInterface/PluginSettingsActivity.java
View File

@ -10,7 +10,10 @@ import android.os.Build;
import android.os.Bundle; import android.os.Bundle;
import android.view.Menu; import android.view.Menu;
import android.view.MenuItem; import android.view.MenuItem;
import android.view.View;
import android.widget.TextView;
import androidx.appcompat.app.AlertDialog;
import androidx.appcompat.app.AppCompatActivity; import androidx.appcompat.app.AppCompatActivity;
import androidx.fragment.app.Fragment; import androidx.fragment.app.Fragment;
import androidx.fragment.app.FragmentManager; import androidx.fragment.app.FragmentManager;
@ -18,8 +21,8 @@ import androidx.fragment.app.FragmentManager;
import com.google.android.material.dialog.MaterialAlertDialogBuilder; import com.google.android.material.dialog.MaterialAlertDialogBuilder;
import org.kde.kdeconnect.Device; import org.kde.kdeconnect.Device;
import org.kde.kdeconnect.DeviceStats;
import org.kde.kdeconnect.KdeConnect; import org.kde.kdeconnect.KdeConnect;
import org.kde.kdeconnect.PacketStats;
import org.kde.kdeconnect.Plugins.Plugin; import org.kde.kdeconnect.Plugins.Plugin;
import org.kde.kdeconnect_tp.R; import org.kde.kdeconnect_tp.R;
@ -101,12 +104,16 @@ public class PluginSettingsActivity
return false; // PacketStats not working in API < 24 return false; // PacketStats not working in API < 24
} }
menu.add(R.string.plugin_stats).setOnMenuItemClickListener(item -> { menu.add(R.string.plugin_stats).setOnMenuItemClickListener(item -> {
String stats = PacketStats.getStatsForDevice(deviceId); String stats = DeviceStats.getStatsForDevice(deviceId);
MaterialAlertDialogBuilder builder = new MaterialAlertDialogBuilder(PluginSettingsActivity.this); AlertDialog alertDialog = new MaterialAlertDialogBuilder(PluginSettingsActivity.this)
builder.setTitle(R.string.plugin_stats); .setTitle(R.string.plugin_stats)
builder.setPositiveButton(R.string.ok, (dialog, which) -> dialog.dismiss()); .setPositiveButton(R.string.ok, (dialog, which) -> dialog.dismiss())
builder.setMessage(stats); .setMessage(stats)
builder.show(); .show();
View messageView = alertDialog.findViewById(android.R.id.message);
if (messageView instanceof TextView) {
((TextView) messageView).setTextIsSelectable(true);
}
return true; return true;
}); });
return true; return true;

13
tests/org/kde/kdeconnect/PacketStatsTest.java → tests/org/kde/kdeconnect/DeviceStatsTest.java
View File

@ -2,14 +2,11 @@ package org.kde.kdeconnect;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.modules.junit4.PowerMockRunner;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap; import java.util.HashMap;
@RunWith(PowerMockRunner.class) public class DeviceStatsTest {
public class PacketStatsTest {
@Test @Test
public void removeOldEvents_cutoutExists() { public void removeOldEvents_cutoutExists() {
@ -21,7 +18,7 @@ public class PacketStatsTest {
events.add(20L); events.add(20L);
events.add(30L); events.add(30L);
final long cutout = 20L; final long cutout = 20L;
PacketStats.removeOldEvents(eventsByType, cutout); DeviceStats.removeOldEvents(eventsByType, cutout);
ArrayList<Long> eventsAfter = eventsByType.get(key); ArrayList<Long> eventsAfter = eventsByType.get(key);
Assert.assertNotNull(eventsAfter); Assert.assertNotNull(eventsAfter);
Assert.assertEquals(2, eventsAfter.size()); Assert.assertEquals(2, eventsAfter.size());
@ -39,7 +36,7 @@ public class PacketStatsTest {
events.add(20L); events.add(20L);
events.add(30L); events.add(30L);
final long cutout = 25L; final long cutout = 25L;
PacketStats.removeOldEvents(eventsByType, cutout); DeviceStats.removeOldEvents(eventsByType, cutout);
ArrayList<Long> eventsAfter = eventsByType.get(key); ArrayList<Long> eventsAfter = eventsByType.get(key);
Assert.assertNotNull(eventsAfter); Assert.assertNotNull(eventsAfter);
Assert.assertEquals(1, eventsAfter.size()); Assert.assertEquals(1, eventsAfter.size());
@ -55,7 +52,7 @@ public class PacketStatsTest {
events.add(10L); events.add(10L);
events.add(20L); events.add(20L);
final long cutout = 25L; final long cutout = 25L;
PacketStats.removeOldEvents(eventsByType, cutout); DeviceStats.removeOldEvents(eventsByType, cutout);
ArrayList<Long> eventsAfter = eventsByType.get(key); ArrayList<Long> eventsAfter = eventsByType.get(key);
Assert.assertNull(eventsAfter); Assert.assertNull(eventsAfter);
} }
@ -68,7 +65,7 @@ public class PacketStatsTest {
eventsByType.put(key, events); eventsByType.put(key, events);
events.add(10L); events.add(10L);
final long cutout = 5L; final long cutout = 5L;
PacketStats.removeOldEvents(eventsByType, cutout); DeviceStats.removeOldEvents(eventsByType, cutout);
ArrayList<Long> eventsAfter = eventsByType.get(key); ArrayList<Long> eventsAfter = eventsByType.get(key);
Assert.assertNotNull(eventsAfter); Assert.assertNotNull(eventsAfter);
Assert.assertEquals(1, eventsAfter.size()); Assert.assertEquals(1, eventsAfter.size());