From 593342288acd754ba3c0a7c690b81657ff729446 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sat, 9 Apr 2022 16:00:00 +0900 Subject: [PATCH 01/18] feat: add debug field depending on variants --- sdk/build.gradle | 6 +++++- sdk/src/debug/java/com/deploygate/sdk/internal/Config.java | 5 +++++ sdk/src/main/java/com/deploygate/sdk/DeployGate.java | 3 --- .../release/java/com/deploygate/sdk/internal/Config.java | 5 +++++ 4 files changed, 15 insertions(+), 4 deletions(-) create mode 100644 sdk/src/debug/java/com/deploygate/sdk/internal/Config.java create mode 100644 sdk/src/release/java/com/deploygate/sdk/internal/Config.java diff --git a/sdk/build.gradle b/sdk/build.gradle index 6493ebc..538e40d 100644 --- a/sdk/build.gradle +++ b/sdk/build.gradle @@ -1,6 +1,10 @@ apply from: rootProject.file('sdk.build.gradle') -android.defaultConfig.consumerProguardFiles 'deploygate-sdk-proguard-rules.txt' +android { + defaultConfig { + consumerProguardFiles 'deploygate-sdk-proguard-rules.txt' + } +} ext { displayName = "DeployGate SDK" diff --git a/sdk/src/debug/java/com/deploygate/sdk/internal/Config.java b/sdk/src/debug/java/com/deploygate/sdk/internal/Config.java new file mode 100644 index 0000000..bf9fb0f --- /dev/null +++ b/sdk/src/debug/java/com/deploygate/sdk/internal/Config.java @@ -0,0 +1,5 @@ +package com.deploygate.sdk.internal; + +public class Config { + public static final boolean DEBUG = true; +} diff --git a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java index b333aba..988fc84 100644 --- a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java +++ b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java @@ -48,7 +48,6 @@ * @author tnj */ public class DeployGate { - private static final String TAG = "DeployGate"; private static final int SDK_VERSION = 4; @@ -116,8 +115,6 @@ public void onEvent( } } - ; - private void onInitialized( final boolean isManaged, final boolean isAuthorized, diff --git a/sdk/src/release/java/com/deploygate/sdk/internal/Config.java b/sdk/src/release/java/com/deploygate/sdk/internal/Config.java new file mode 100644 index 0000000..6b1aeef --- /dev/null +++ b/sdk/src/release/java/com/deploygate/sdk/internal/Config.java @@ -0,0 +1,5 @@ +package com.deploygate.sdk.internal; + +public class Config { + public static final boolean DEBUG = false; +} From 02ea1e9f6d74fbd271b5626d1fdb5f97fbee8e04 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sat, 9 Apr 2022 19:42:19 +0900 Subject: [PATCH 02/18] chore: ignore .DS_Store --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index c61d2d0..12bf2a8 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,5 @@ build/ #idea *.iml .idea/ + +.DS_Store \ No newline at end of file From 02d67378fc1051c37097eecc3c49be429200352f Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sat, 9 Apr 2022 18:24:34 +0900 Subject: [PATCH 03/18] feat: add a logger and modified proguard rules --- sdk/deploygate-sdk-proguard-rules.txt | 9 +++ .../com/deploygate/sdk/internal/Logger.java | 80 +++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 sdk/src/main/java/com/deploygate/sdk/internal/Logger.java diff --git a/sdk/deploygate-sdk-proguard-rules.txt b/sdk/deploygate-sdk-proguard-rules.txt index 564865b..7512270 100644 --- a/sdk/deploygate-sdk-proguard-rules.txt +++ b/sdk/deploygate-sdk-proguard-rules.txt @@ -1 +1,10 @@ -keep class * extends android.os.IInterface + +# To allow removing our logger +-assumenosideeffects class com.deploygate.sdk.internal.Logger { + public static void v(...); + public static void i(...); + public static void w(...); + public static void d(...); + public static void e(...); +} diff --git a/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java b/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java new file mode 100644 index 0000000..b09252e --- /dev/null +++ b/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java @@ -0,0 +1,80 @@ +package com.deploygate.sdk.internal; + +import android.util.Log; + +import java.util.Locale; + +/** + * @hide + */ +public class Logger { + private static final String TAG = "DeployGateSDK"; + + public static void d( + String format, + Object... args + ) { + if (!Config.DEBUG) { + return; + } + + Log.d(TAG, String.format(Locale.US, format, args)); + } + + public static void i( + String format, + Object... args + ) { + if (!Config.DEBUG) { + return; + } + + Log.i(TAG, String.format(Locale.US, format, args)); + } + + public static void w( + String format, + Object... args + ) { + if (!Config.DEBUG) { + return; + } + + Log.w(TAG, String.format(Locale.US, format, args)); + } + + public static void w( + Throwable th, + String format, + Object... args + ) { + if (!Config.DEBUG) { + return; + } + + Log.w(TAG, String.format(Locale.US, format, args), th); + } + + public static void e( + String format, + Object... args + ) { + if (!Config.DEBUG) { + return; + } + + Log.e(TAG, String.format(Locale.US, format, args)); + } + + public static void e( + Throwable th, + String format, + Object... args + ) { + if (!Config.DEBUG) { + return; + } + + Log.e(TAG, String.format(Locale.US, format, args), th); + } +} From efcd30e2ee498d9149258a507f1782f6b213c82f Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sat, 9 Apr 2022 16:00:41 +0900 Subject: [PATCH 04/18] feat: a new transmitter for event logs --- .../java/com/deploygate/sdk/Backpressure.java | 6 + .../java/com/deploygate/sdk/DeployGate.java | 16 +- .../java/com/deploygate/sdk/EventLog.java | 48 +++++ .../deploygate/sdk/EventLogConfiguration.java | 50 +++++ .../deploygate/sdk/EventLogTransmitter.java | 197 ++++++++++++++++++ .../sdk/EventLogTransmitterTest.java | 10 + 6 files changed, 317 insertions(+), 10 deletions(-) create mode 100644 sdk/src/main/java/com/deploygate/sdk/Backpressure.java create mode 100644 sdk/src/main/java/com/deploygate/sdk/EventLog.java create mode 100644 sdk/src/main/java/com/deploygate/sdk/EventLogConfiguration.java create mode 100644 sdk/src/main/java/com/deploygate/sdk/EventLogTransmitter.java create mode 100644 sdk/src/test/java/com/deploygate/sdk/EventLogTransmitterTest.java diff --git a/sdk/src/main/java/com/deploygate/sdk/Backpressure.java b/sdk/src/main/java/com/deploygate/sdk/Backpressure.java new file mode 100644 index 0000000..9c8800e --- /dev/null +++ b/sdk/src/main/java/com/deploygate/sdk/Backpressure.java @@ -0,0 +1,6 @@ +package com.deploygate.sdk; + +public enum Backpressure { + DROP_LATEST, + DROP_OLDEST +} diff --git a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java index 988fc84..9541a69 100644 --- a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java +++ b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java @@ -69,6 +69,7 @@ public class DeployGate { private final Context mApplicationContext; private final Handler mHandler; + private final EventLogTransmitter mEventLogTransmitter; private final HashSet mCallbacks; private final String mExpectedAuthor; private String mAuthor; @@ -225,8 +226,9 @@ private DeployGate( String author, DeployGateCallback callback ) { - mHandler = new Handler(); mApplicationContext = applicationContext; + mHandler = new Handler(); + mEventLogTransmitter = new EventLogTransmitter(mApplicationContext.getPackageName(), new EventLogConfiguration.Builder().build()); mCallbacks = new HashSet(); mExpectedAuthor = author; @@ -302,6 +304,7 @@ public void onServiceConnected( public void onServiceDisconnected(ComponentName name) { Log.v(TAG, "DeployGate service disconneced"); mRemoteService = null; + mEventLogTransmitter.disconnect(); } }, Context.BIND_AUTO_CREATE); } @@ -314,6 +317,7 @@ private void requestServiceInit(final boolean isBoot) { args.putInt(DeployGateEvent.EXTRA_SDK_VERSION, SDK_VERSION); try { mRemoteService.init(mRemoteCallback, mApplicationContext.getPackageName(), args); + mEventLogTransmitter.connect(mRemoteService); } catch (RemoteException e) { Log.w(TAG, "DeployGate service failed to be initialized."); } @@ -1062,15 +1066,7 @@ void sendLog( String type, String body ) { - Bundle extras = new Bundle(); - extras.putSerializable(DeployGateEvent.EXTRA_LOG, body); - extras.putSerializable(DeployGateEvent.EXTRA_LOG_TYPE, type); - - try { - mRemoteService.sendEvent(mApplicationContext.getPackageName(), DeployGateEvent.ACTION_SEND_CUSTOM_LOG, extras); - } catch (RemoteException e) { - Log.w(TAG, "failed to send custom log: " + e.getMessage()); - } + mEventLogTransmitter.transmit(type, body); } /** diff --git a/sdk/src/main/java/com/deploygate/sdk/EventLog.java b/sdk/src/main/java/com/deploygate/sdk/EventLog.java new file mode 100644 index 0000000..c2ce274 --- /dev/null +++ b/sdk/src/main/java/com/deploygate/sdk/EventLog.java @@ -0,0 +1,48 @@ +package com.deploygate.sdk; + +import android.os.Parcel; +import android.os.Parcelable; + +class EventLog implements Parcelable { + public final String type; + public final String body; + + EventLog( + String type, + String body + ) { + this.type = type; + this.body = body; + } + + protected EventLog(Parcel in) { + type = in.readString(); + body = in.readString(); + } + + public static final Creator CREATOR = new Creator() { + @Override + public EventLog createFromParcel(Parcel in) { + return new EventLog(in); + } + + @Override + public EventLog[] newArray(int size) { + return new EventLog[size]; + } + }; + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel( + Parcel dest, + int flags + ) { + dest.writeString(type); + dest.writeString(body); + } +} diff --git a/sdk/src/main/java/com/deploygate/sdk/EventLogConfiguration.java b/sdk/src/main/java/com/deploygate/sdk/EventLogConfiguration.java new file mode 100644 index 0000000..9ff9248 --- /dev/null +++ b/sdk/src/main/java/com/deploygate/sdk/EventLogConfiguration.java @@ -0,0 +1,50 @@ +package com.deploygate.sdk; + +import com.deploygate.sdk.internal.Logger; + +public class EventLogConfiguration { + private static final int MAX_BUFFER_SIZE = 100; // FIXME experimental + + public final Backpressure backpressure; + public final int bufferSize; + + private EventLogConfiguration( + Backpressure backpressure, + int bufferSize + ) { + this.backpressure = backpressure; + this.bufferSize = bufferSize; + } + + public static class Builder { + private Backpressure backpressure = Backpressure.DROP_OLDEST; + private int bufferSize = MAX_BUFFER_SIZE; + + public Builder setBackpressure(Backpressure backpressure) { + if (backpressure == null) { + throw new IllegalArgumentException("backpressure must be non-null"); + } + + this.backpressure = backpressure; + return this; + } + + public Builder setBufferSize(int bufferSize) { + if (bufferSize <= 0) { + throw new IllegalArgumentException("buffer size must be greater than 0"); + } + + if (bufferSize > MAX_BUFFER_SIZE) { + Logger.w("buffer size is exceeded %d so it's rounded to %d", bufferSize, MAX_BUFFER_SIZE); + bufferSize = MAX_BUFFER_SIZE; + } + + this.bufferSize = bufferSize; + return this; + } + + public EventLogConfiguration build() { + return new EventLogConfiguration(backpressure, bufferSize); + } + } +} diff --git a/sdk/src/main/java/com/deploygate/sdk/EventLogTransmitter.java b/sdk/src/main/java/com/deploygate/sdk/EventLogTransmitter.java new file mode 100644 index 0000000..2da52f7 --- /dev/null +++ b/sdk/src/main/java/com/deploygate/sdk/EventLogTransmitter.java @@ -0,0 +1,197 @@ +package com.deploygate.sdk; + +import android.os.Bundle; +import android.os.Handler; +import android.os.HandlerThread; +import android.os.Looper; +import android.os.Message; +import android.os.RemoteException; + +import com.deploygate.sdk.internal.Logger; +import com.deploygate.service.DeployGateEvent; +import com.deploygate.service.IDeployGateSdkService; + +import java.util.LinkedList; + +class EventLogTransmitter { + private final String packageName; + private final EventLogConfiguration configuration; + + @SuppressWarnings("FieldCanBeLocal") + private final HandlerThread thread; + private EventLogHandler handler; + + private volatile IDeployGateSdkService service; + + EventLogTransmitter( + String packageName, + EventLogConfiguration configuration + ) { + this.packageName = packageName; + this.configuration = configuration; + + this.thread = new HandlerThread("deploygate-sdk-event-log"); + this.thread.start(); + } + + public final synchronized void connect(IDeployGateSdkService service) { + ensureHandlerInitialized(); + + handler.cancelExtruding(); + this.service = service; + handler.extrudeAllLogs(); + } + + public final void disconnect() { + ensureHandlerInitialized(); + + handler.cancelExtruding(); + this.service = null; + } + + public final synchronized void transmit( + String type, + String body + ) { + ensureHandlerInitialized(); + + EventLog log = new EventLog(type, body); + handler.enqueueNewLog(log); + } + + private boolean sendLog(EventLog log) { + IDeployGateSdkService service = this.service; + + if (service == null) { + return false; + } + + try { + Bundle extras = new Bundle(); + extras.putSerializable(DeployGateEvent.EXTRA_LOG, log.body); + extras.putSerializable(DeployGateEvent.EXTRA_LOG_TYPE, log.type); + + service.sendEvent(packageName, DeployGateEvent.ACTION_SEND_CUSTOM_LOG, extras); + return true; + } catch (RemoteException e) { + Logger.w("failed to send custom log: %s", e.getMessage()); + return false; + } + } + + private void ensureHandlerInitialized() { + if (handler != null) { + return; + } + + synchronized (configuration) { + if (handler != null) { + return; + } + + handler = new EventLogHandler(thread.getLooper(), this, configuration.backpressure, configuration.bufferSize); + } + } + + static class EventLogHandler extends Handler { + private static final int WHAT_EXTRUDE_ALL_BUFFER = 0x30; + private static final int WHAT_PUSH_LOG = 0x100; + + private final EventLogTransmitter extruder; + private final Backpressure backpressure; + private final int bufferSize; + private final LinkedList eventLogs; + private int pushWhatOffset = 0; + + EventLogHandler( + Looper looper, + EventLogTransmitter extruder, + Backpressure backpressure, + int bufferSize + ) { + super(looper); + this.extruder = extruder; + this.backpressure = backpressure; + this.bufferSize = bufferSize; + this.eventLogs = new LinkedList<>(); + } + + void cancelExtruding() { + removeMessages(WHAT_EXTRUDE_ALL_BUFFER); + } + + void extrudeAllLogs() { + if (hasMessages(WHAT_EXTRUDE_ALL_BUFFER)) { + return; + } + + sendEmptyMessage(WHAT_EXTRUDE_ALL_BUFFER); + } + + void enqueueNewLog(EventLog log) { + Message msg = obtainMessage(WHAT_PUSH_LOG + getAndIncrementPushWhatOffset(), log); + sendMessage(msg); + } + + void addLogToLast(EventLog log) { + boolean dropFirst = backpressure == Backpressure.DROP_OLDEST; + int droppedCount = 0; + + while (eventLogs.size() >= bufferSize) { + if (dropFirst) { + eventLogs.poll(); + droppedCount++; + } else { + Logger.d("the queue is already full and reject the new element."); + return; + } + } + + Logger.d("filtered out %d overflowed old elements.", droppedCount); + + eventLogs.addLast(log); + } + + void extrudeAllInBuffer() { + while (!eventLogs.isEmpty()) { + EventLog log = eventLogs.poll(); + + if (!extruder.sendLog(log)) { + // push back + eventLogs.addFirst(log); + break; + } + } + + if (!eventLogs.isEmpty()) { + sendEmptyMessageDelayed(WHAT_EXTRUDE_ALL_BUFFER, 1000L); + } + } + + @Override + public void handleMessage(Message msg) { + switch (msg.what) { + case WHAT_EXTRUDE_ALL_BUFFER: { + extrudeAllInBuffer(); + + break; + } + default: { + if (msg.what >= WHAT_PUSH_LOG) { + EventLog log = (EventLog) msg.obj; + addLogToLast(log); + } + + break; + } + } + } + + private int getAndIncrementPushWhatOffset() { + if (pushWhatOffset > 1000) { + pushWhatOffset = 0; + } + return pushWhatOffset++; + } + } +} diff --git a/sdk/src/test/java/com/deploygate/sdk/EventLogTransmitterTest.java b/sdk/src/test/java/com/deploygate/sdk/EventLogTransmitterTest.java new file mode 100644 index 0000000..87e3f9c --- /dev/null +++ b/sdk/src/test/java/com/deploygate/sdk/EventLogTransmitterTest.java @@ -0,0 +1,10 @@ +package com.deploygate.sdk; + +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class EventLogTransmitterTest { + +} From a3d949bb61096bc4f63dc59566ad72364263a188 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sat, 9 Apr 2022 21:59:21 +0900 Subject: [PATCH 05/18] feat: add mockito for testing and renamed event log to custom log --- sdk/build.gradle | 4 + .../sdk/{EventLog.java => CustomLog.java} | 16 ++-- ...ation.java => CustomLogConfiguration.java} | 8 +- ...smitter.java => CustomLogTransmitter.java} | 76 +++++++++------- .../java/com/deploygate/sdk/DeployGate.java | 44 ++++++++-- .../com/deploygate/sdk/internal/Logger.java | 3 - .../com/deploygate/sdk/BundleMatcher.java | 43 +++++++++ .../sdk/CustomLogTransmitterTest.java | 87 +++++++++++++++++++ .../sdk/EventLogTransmitterTest.java | 10 --- 9 files changed, 230 insertions(+), 61 deletions(-) rename sdk/src/main/java/com/deploygate/sdk/{EventLog.java => CustomLog.java} (64%) rename sdk/src/main/java/com/deploygate/sdk/{EventLogConfiguration.java => CustomLogConfiguration.java} (87%) rename sdk/src/main/java/com/deploygate/sdk/{EventLogTransmitter.java => CustomLogTransmitter.java} (72%) create mode 100644 sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java create mode 100644 sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java delete mode 100644 sdk/src/test/java/com/deploygate/sdk/EventLogTransmitterTest.java diff --git a/sdk/build.gradle b/sdk/build.gradle index 538e40d..c81259c 100644 --- a/sdk/build.gradle +++ b/sdk/build.gradle @@ -6,6 +6,10 @@ android { } } +dependencies { + testImplementation 'org.mockito:mockito-core:4.4.0' +} + ext { displayName = "DeployGate SDK" artifactId = 'sdk' diff --git a/sdk/src/main/java/com/deploygate/sdk/EventLog.java b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java similarity index 64% rename from sdk/src/main/java/com/deploygate/sdk/EventLog.java rename to sdk/src/main/java/com/deploygate/sdk/CustomLog.java index c2ce274..a8e306a 100644 --- a/sdk/src/main/java/com/deploygate/sdk/EventLog.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java @@ -3,11 +3,11 @@ import android.os.Parcel; import android.os.Parcelable; -class EventLog implements Parcelable { +class CustomLog implements Parcelable { public final String type; public final String body; - EventLog( + CustomLog( String type, String body ) { @@ -15,20 +15,20 @@ class EventLog implements Parcelable { this.body = body; } - protected EventLog(Parcel in) { + protected CustomLog(Parcel in) { type = in.readString(); body = in.readString(); } - public static final Creator CREATOR = new Creator() { + public static final Creator CREATOR = new Creator() { @Override - public EventLog createFromParcel(Parcel in) { - return new EventLog(in); + public CustomLog createFromParcel(Parcel in) { + return new CustomLog(in); } @Override - public EventLog[] newArray(int size) { - return new EventLog[size]; + public CustomLog[] newArray(int size) { + return new CustomLog[size]; } }; diff --git a/sdk/src/main/java/com/deploygate/sdk/EventLogConfiguration.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java similarity index 87% rename from sdk/src/main/java/com/deploygate/sdk/EventLogConfiguration.java rename to sdk/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java index 9ff9248..e4613af 100644 --- a/sdk/src/main/java/com/deploygate/sdk/EventLogConfiguration.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java @@ -2,13 +2,13 @@ import com.deploygate.sdk.internal.Logger; -public class EventLogConfiguration { +public class CustomLogConfiguration { private static final int MAX_BUFFER_SIZE = 100; // FIXME experimental public final Backpressure backpressure; public final int bufferSize; - private EventLogConfiguration( + private CustomLogConfiguration( Backpressure backpressure, int bufferSize ) { @@ -43,8 +43,8 @@ public Builder setBufferSize(int bufferSize) { return this; } - public EventLogConfiguration build() { - return new EventLogConfiguration(backpressure, bufferSize); + public CustomLogConfiguration build() { + return new CustomLogConfiguration(backpressure, bufferSize); } } } diff --git a/sdk/src/main/java/com/deploygate/sdk/EventLogTransmitter.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java similarity index 72% rename from sdk/src/main/java/com/deploygate/sdk/EventLogTransmitter.java rename to sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java index 2da52f7..cffc376 100644 --- a/sdk/src/main/java/com/deploygate/sdk/EventLogTransmitter.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java @@ -13,19 +13,19 @@ import java.util.LinkedList; -class EventLogTransmitter { +class CustomLogTransmitter { private final String packageName; - private final EventLogConfiguration configuration; + private final CustomLogConfiguration configuration; @SuppressWarnings("FieldCanBeLocal") private final HandlerThread thread; - private EventLogHandler handler; + private CustomLogHandler handler; private volatile IDeployGateSdkService service; - EventLogTransmitter( + CustomLogTransmitter( String packageName, - EventLogConfiguration configuration + CustomLogConfiguration configuration ) { this.packageName = packageName; this.configuration = configuration; @@ -55,11 +55,24 @@ public final synchronized void transmit( ) { ensureHandlerInitialized(); - EventLog log = new EventLog(type, body); + CustomLog log = new CustomLog(type, body); handler.enqueueNewLog(log); } - private boolean sendLog(EventLog log) { + /** + * @return + * + * @hide Only for testing. + */ + Looper getLooper() { + return handler.getLooper(); + } + + int getPendingCount() { + return handler.customLogs.size(); + } + + private boolean sendLog(CustomLog log) { IDeployGateSdkService service = this.service; if (service == null) { @@ -79,6 +92,10 @@ private boolean sendLog(EventLog log) { } } + private boolean isConnected() { + return service != null; + } + private void ensureHandlerInitialized() { if (handler != null) { return; @@ -89,23 +106,23 @@ private void ensureHandlerInitialized() { return; } - handler = new EventLogHandler(thread.getLooper(), this, configuration.backpressure, configuration.bufferSize); + handler = new CustomLogHandler(thread.getLooper(), this, configuration.backpressure, configuration.bufferSize); } } - static class EventLogHandler extends Handler { - private static final int WHAT_EXTRUDE_ALL_BUFFER = 0x30; - private static final int WHAT_PUSH_LOG = 0x100; + static class CustomLogHandler extends Handler { + static final int WHAT_EXTRUDE_ALL_BUFFER = 0x30; + static final int WHAT_PUSH_LOG = 0x100; - private final EventLogTransmitter extruder; + private final CustomLogTransmitter extruder; private final Backpressure backpressure; private final int bufferSize; - private final LinkedList eventLogs; + private final LinkedList customLogs; private int pushWhatOffset = 0; - EventLogHandler( + CustomLogHandler( Looper looper, - EventLogTransmitter extruder, + CustomLogTransmitter extruder, Backpressure backpressure, int bufferSize ) { @@ -113,7 +130,7 @@ static class EventLogHandler extends Handler { this.extruder = extruder; this.backpressure = backpressure; this.bufferSize = bufferSize; - this.eventLogs = new LinkedList<>(); + this.customLogs = new LinkedList<>(); } void cancelExtruding() { @@ -128,18 +145,18 @@ void extrudeAllLogs() { sendEmptyMessage(WHAT_EXTRUDE_ALL_BUFFER); } - void enqueueNewLog(EventLog log) { + void enqueueNewLog(CustomLog log) { Message msg = obtainMessage(WHAT_PUSH_LOG + getAndIncrementPushWhatOffset(), log); sendMessage(msg); } - void addLogToLast(EventLog log) { + void addLogToLast(CustomLog log) { boolean dropFirst = backpressure == Backpressure.DROP_OLDEST; int droppedCount = 0; - while (eventLogs.size() >= bufferSize) { + while (customLogs.size() >= bufferSize) { if (dropFirst) { - eventLogs.poll(); + customLogs.poll(); droppedCount++; } else { Logger.d("the queue is already full and reject the new element."); @@ -149,23 +166,24 @@ void addLogToLast(EventLog log) { Logger.d("filtered out %d overflowed old elements.", droppedCount); - eventLogs.addLast(log); + customLogs.addLast(log); + + if (extruder.isConnected()) { + extrudeAllInBuffer(); + } } void extrudeAllInBuffer() { - while (!eventLogs.isEmpty()) { - EventLog log = eventLogs.poll(); + while (!customLogs.isEmpty()) { + CustomLog log = customLogs.poll(); if (!extruder.sendLog(log)) { // push back - eventLogs.addFirst(log); + customLogs.addFirst(log); + sendEmptyMessageDelayed(WHAT_EXTRUDE_ALL_BUFFER, 1000L); break; } } - - if (!eventLogs.isEmpty()) { - sendEmptyMessageDelayed(WHAT_EXTRUDE_ALL_BUFFER, 1000L); - } } @Override @@ -178,7 +196,7 @@ public void handleMessage(Message msg) { } default: { if (msg.what >= WHAT_PUSH_LOG) { - EventLog log = (EventLog) msg.obj; + CustomLog log = (CustomLog) msg.obj; addLogToLast(log); } diff --git a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java index 9541a69..7ceb74a 100644 --- a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java +++ b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java @@ -69,7 +69,7 @@ public class DeployGate { private final Context mApplicationContext; private final Handler mHandler; - private final EventLogTransmitter mEventLogTransmitter; + private final CustomLogTransmitter mCustomLogTransmitter; private final HashSet mCallbacks; private final String mExpectedAuthor; private String mAuthor; @@ -224,11 +224,12 @@ public void run() { private DeployGate( Context applicationContext, String author, - DeployGateCallback callback + DeployGateCallback callback, + CustomLogConfiguration customLogConfiguration ) { mApplicationContext = applicationContext; mHandler = new Handler(); - mEventLogTransmitter = new EventLogTransmitter(mApplicationContext.getPackageName(), new EventLogConfiguration.Builder().build()); + mCustomLogTransmitter = new CustomLogTransmitter(mApplicationContext.getPackageName(), customLogConfiguration); mCallbacks = new HashSet(); mExpectedAuthor = author; @@ -304,7 +305,7 @@ public void onServiceConnected( public void onServiceDisconnected(ComponentName name) { Log.v(TAG, "DeployGate service disconneced"); mRemoteService = null; - mEventLogTransmitter.disconnect(); + mCustomLogTransmitter.disconnect(); } }, Context.BIND_AUTO_CREATE); } @@ -317,7 +318,7 @@ private void requestServiceInit(final boolean isBoot) { args.putInt(DeployGateEvent.EXTRA_SDK_VERSION, SDK_VERSION); try { mRemoteService.init(mRemoteCallback, mApplicationContext.getPackageName(), args); - mEventLogTransmitter.connect(mRemoteService); + mCustomLogTransmitter.connect(mRemoteService); } catch (RemoteException e) { Log.w(TAG, "DeployGate service failed to be initialized."); } @@ -567,6 +568,35 @@ public static void install( String author, DeployGateCallback callback, boolean forceApplyOnReleaseBuild + ) { + install(app, author, callback, forceApplyOnReleaseBuild, new CustomLogConfiguration.Builder().build()); + } + + /** + * Install DeployGate on your application instance and register a callback + * listener. Call this method inside of your {@link Application#onCreate()} + * once. + * + * @param app + * Application instance, typically just pass this. + * @param author + * author username of this app. Can be null. + * @param callback + * Callback interface to listen events. Can be null. + * @param forceApplyOnReleaseBuild + * if you want to keep DeployGate alive on + * the release build, set this true. + * @param customLogConfiguration + * set a configuration for custom logging + * + * @since r4.4 + */ + public static void install( + Application app, + String author, + DeployGateCallback callback, + boolean forceApplyOnReleaseBuild, + CustomLogConfiguration customLogConfiguration ) { if (sInstance != null) { Log.w(TAG, "DeployGate.install was already called. Ignoring."); @@ -578,7 +608,7 @@ public static void install( } Thread.setDefaultUncaughtExceptionHandler(new DeployGateUncaughtExceptionHandler(Thread.getDefaultUncaughtExceptionHandler())); - sInstance = new DeployGate(app.getApplicationContext(), author, callback); + sInstance = new DeployGate(app.getApplicationContext(), author, callback, customLogConfiguration); } /** @@ -1066,7 +1096,7 @@ void sendLog( String type, String body ) { - mEventLogTransmitter.transmit(type, body); + mCustomLogTransmitter.transmit(type, body); } /** diff --git a/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java b/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java index b09252e..11e3ea4 100644 --- a/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java +++ b/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java @@ -4,9 +4,6 @@ import java.util.Locale; -/** - * @hide - */ public class Logger { private static final String TAG = "DeployGateSDK"; diff --git a/sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java b/sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java new file mode 100644 index 0000000..e5d52be --- /dev/null +++ b/sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java @@ -0,0 +1,43 @@ +package com.deploygate.sdk; + +import android.os.Bundle; + +import org.mockito.ArgumentMatcher; + +import java.util.Objects; + +import static org.mockito.internal.progress.ThreadSafeMockingProgress.mockingProgress; + +class BundleMatcher { + public static Bundle eq(Bundle expected) { + mockingProgress().getArgumentMatcherStorage().reportMatcher(new Equals(expected)); + return expected; + } + + private static class Equals implements ArgumentMatcher { + private final Bundle expected; + + public Equals(Bundle expected) { + this.expected = expected; + } + + @Override + public boolean matches(Bundle argument) { + if (expected.size() != argument.size()) { + return false; + } + + if (!expected.keySet().equals(argument.keySet())) { + return false; + } + + for (final String key : expected.keySet()) { + if (!Objects.equals(expected.get(key), argument.get(key))) { + return false; + } + } + + return true; + } + } +} diff --git a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java new file mode 100644 index 0000000..cf5526c --- /dev/null +++ b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java @@ -0,0 +1,87 @@ +package com.deploygate.sdk; + +import android.os.Bundle; +import android.os.RemoteException; + +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import com.deploygate.service.DeployGateEvent; +import com.deploygate.service.IDeployGateSdkService; +import com.google.common.truth.Truth; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.verification.VerificationMode; +import org.robolectric.Shadows; +import org.robolectric.annotation.LooperMode; + +import static org.robolectric.annotation.LooperMode.Mode.PAUSED; + +@RunWith(AndroidJUnit4.class) +@LooperMode(PAUSED) +public class CustomLogTransmitterTest { + + private static final String PACKAGE_NAME = "com.deploygate.sample"; + + @Mock + private IDeployGateSdkService service; + + private CustomLogTransmitter customLogTransmitter; + + private CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); + + @Before + public void before() { + MockitoAnnotations.openMocks(this); + + customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + } + + @Test(timeout = 3000L) + public void check_buffer_size_works() throws RemoteException { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(5).build(); + CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + + for (int i = 0; i < 10; i++) { + customLogTransmitter.transmit("type", String.valueOf(i)); + } + + customLogTransmitter.connect(service); + + Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + + VerificationMode once = Mockito.times(1); + + for (int i = 5; i < 10; i++) { + Mockito.verify(service, once).sendEvent(Mockito.eq(PACKAGE_NAME), Mockito.eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), BundleMatcher.eq(createLogExtra("type", String.valueOf(i)))); + } + } + + @Test(timeout = 3000L) + public void transmit_works_regardless_of_service() { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(8).build(); + CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + + for (int i = 0; i < 10; i++) { + customLogTransmitter.transmit("type", String.valueOf(i)); + } + + Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + + Truth.assertThat(customLogTransmitter.getPendingCount()).isEqualTo(8); + } + + private static Bundle createLogExtra( + String type, + String body + ) { + Bundle bundle = new Bundle(); + bundle.putString("logType", type); + bundle.putString("log", body); + return bundle; + } +} diff --git a/sdk/src/test/java/com/deploygate/sdk/EventLogTransmitterTest.java b/sdk/src/test/java/com/deploygate/sdk/EventLogTransmitterTest.java deleted file mode 100644 index 87e3f9c..0000000 --- a/sdk/src/test/java/com/deploygate/sdk/EventLogTransmitterTest.java +++ /dev/null @@ -1,10 +0,0 @@ -package com.deploygate.sdk; - -import androidx.test.ext.junit.runners.AndroidJUnit4; - -import org.junit.runner.RunWith; - -@RunWith(AndroidJUnit4.class) -public class EventLogTransmitterTest { - -} From d895903881d00e63abaaa95c2feff0eee151340e Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sun, 10 Apr 2022 00:44:39 +0900 Subject: [PATCH 06/18] chore: Add javadocs and improve junit tests --- .../java/com/deploygate/sdk/Backpressure.java | 6 - .../sdk/CustomLogConfiguration.java | 47 ++++++- .../deploygate/sdk/CustomLogTransmitter.java | 119 +++++++++++++----- .../com/deploygate/sdk/BundleMatcher.java | 5 + .../sdk/CustomLogTransmitterTest.java | 36 +++++- .../sdk/DeployGateInterfaceTest.java | 5 + 6 files changed, 175 insertions(+), 43 deletions(-) delete mode 100644 sdk/src/main/java/com/deploygate/sdk/Backpressure.java diff --git a/sdk/src/main/java/com/deploygate/sdk/Backpressure.java b/sdk/src/main/java/com/deploygate/sdk/Backpressure.java deleted file mode 100644 index 9c8800e..0000000 --- a/sdk/src/main/java/com/deploygate/sdk/Backpressure.java +++ /dev/null @@ -1,6 +0,0 @@ -package com.deploygate.sdk; - -public enum Backpressure { - DROP_LATEST, - DROP_OLDEST -} diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java index e4613af..0b2fd69 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java @@ -3,11 +3,38 @@ import com.deploygate.sdk.internal.Logger; public class CustomLogConfiguration { - private static final int MAX_BUFFER_SIZE = 100; // FIXME experimental + public enum Backpressure { + /** + * SDK rejects new logs if buffer size is exceeded + */ + PRESERVE_BUFFER, + + /** + * SDK drops logs from the oldest if buffer size is exceeded + */ + DROP_BUFFER_BY_OLDEST + } + + /** + * the log buffer is required until DeployGate client app receives BOOT_COMPLETED broadcast. + *

+ * This is an experimental value. + *

+ * - 10 seconds until boot-completed + * - 10 logs per 1 seconds + * - plus some buffer + */ + private static final int DEFAULT_BUFFER_SIZE = 150; + private static final int MAX_BUFFER_SIZE = DEFAULT_BUFFER_SIZE; public final Backpressure backpressure; public final int bufferSize; + /** + * Do not bypass {@link Builder} to instantiate this class. + * + * @see Builder + */ private CustomLogConfiguration( Backpressure backpressure, int bufferSize @@ -17,9 +44,17 @@ private CustomLogConfiguration( } public static class Builder { - private Backpressure backpressure = Backpressure.DROP_OLDEST; - private int bufferSize = MAX_BUFFER_SIZE; + private Backpressure backpressure = Backpressure.DROP_BUFFER_BY_OLDEST; + private int bufferSize = DEFAULT_BUFFER_SIZE; + /** + * @param backpressure + * the strategy of the backpressure in the log buffer + * + * @return self + * + * @see Backpressure + */ public Builder setBackpressure(Backpressure backpressure) { if (backpressure == null) { throw new IllegalArgumentException("backpressure must be non-null"); @@ -29,6 +64,12 @@ public Builder setBackpressure(Backpressure backpressure) { return this; } + /** + * @param bufferSize + * the max size of the log buffer + * + * @return self + */ public Builder setBufferSize(int bufferSize) { if (bufferSize <= 0) { throw new IllegalArgumentException("buffer size must be greater than 0"); diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java index cffc376..7450d4a 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java @@ -13,6 +13,19 @@ import java.util.LinkedList; +/** + * This class transmits pending logs to the DeployGate client. + * The internal handler class creates a buffer pool and assure the sending-order. + * + *

+ * - Enqueue a new log to the buffer pool + * - Send logs in the order of FIFO + * + * The cost of polling the log buffer pool may be high, so SDK starts sending logs only while a service is active. + *

+ * - When a new log is stored to the buffer + * - + */ class CustomLogTransmitter { private final String packageName; private final CustomLogConfiguration configuration; @@ -37,15 +50,15 @@ class CustomLogTransmitter { public final synchronized void connect(IDeployGateSdkService service) { ensureHandlerInitialized(); - handler.cancelExtruding(); + handler.cancelPendingSendLogsInstruction(); this.service = service; - handler.extrudeAllLogs(); + handler.enqueueSendLogsInstruction(); } public final void disconnect() { ensureHandlerInitialized(); - handler.cancelExtruding(); + handler.cancelPendingSendLogsInstruction(); this.service = null; } @@ -56,7 +69,7 @@ public final synchronized void transmit( ensureHandlerInitialized(); CustomLog log = new CustomLog(type, body); - handler.enqueueNewLog(log); + handler.enqueueAddNewLogInstruction(log); } /** @@ -110,48 +123,81 @@ private void ensureHandlerInitialized() { } } - static class CustomLogHandler extends Handler { - static final int WHAT_EXTRUDE_ALL_BUFFER = 0x30; - static final int WHAT_PUSH_LOG = 0x100; + /** + * This handler behaves as a ordered-buffer of instructions. + *

+ * The instruction of adding a new log and sending buffered logs are synchronized. + */ + private static class CustomLogHandler extends Handler { + private static final int WHAT_SEND_LOGS = 0x30; + private static final int WHAT_ADD_NEW_LOG = 0x100; - private final CustomLogTransmitter extruder; - private final Backpressure backpressure; + private final CustomLogTransmitter transmitter; + private final CustomLogConfiguration.Backpressure backpressure; private final int bufferSize; + private final int maxWhatOffset; private final LinkedList customLogs; private int pushWhatOffset = 0; + /** + * @param looper + * Do not use Main Looper to avoid wasting the main thread resource. + * @param transmitter + * an instance to send logs + * @param backpressure + * the backpressure strategy of the log buffer, not of instructions. + * @param bufferSize + * the max size of the log buffer, not of instructions. + */ CustomLogHandler( Looper looper, - CustomLogTransmitter extruder, - Backpressure backpressure, + CustomLogTransmitter transmitter, + CustomLogConfiguration.Backpressure backpressure, int bufferSize ) { super(looper); - this.extruder = extruder; + this.transmitter = transmitter; this.backpressure = backpressure; this.bufferSize = bufferSize; + this.maxWhatOffset = bufferSize * 2; this.customLogs = new LinkedList<>(); } - void cancelExtruding() { - removeMessages(WHAT_EXTRUDE_ALL_BUFFER); + /** + * Cancel the send-logs instruction in the handler message queue. + * This doesn't interrupt the thread and stop the sending-logs instruction that is running at the time. + */ + void cancelPendingSendLogsInstruction() { + removeMessages(WHAT_SEND_LOGS); } - void extrudeAllLogs() { - if (hasMessages(WHAT_EXTRUDE_ALL_BUFFER)) { + /** + * Enqueue the send-logs instruction in the handler message queue unless enqueued. + */ + void enqueueSendLogsInstruction() { + if (hasMessages(WHAT_SEND_LOGS)) { return; } - sendEmptyMessage(WHAT_EXTRUDE_ALL_BUFFER); + sendEmptyMessage(WHAT_SEND_LOGS); } - void enqueueNewLog(CustomLog log) { - Message msg = obtainMessage(WHAT_PUSH_LOG + getAndIncrementPushWhatOffset(), log); + /** + * Enqueue new add-new-log instruction to the handler message queue. + */ + void enqueueAddNewLogInstruction(CustomLog log) { + Message msg = obtainMessage(WHAT_ADD_NEW_LOG + getAndIncrementPushWhatOffset(), log); sendMessage(msg); } + /** + * Append the new log to the log buffer if the backpressure is not dropping LATEST. + * + * @param log + * a new log + */ void addLogToLast(CustomLog log) { - boolean dropFirst = backpressure == Backpressure.DROP_OLDEST; + boolean dropFirst = backpressure == CustomLogConfiguration.Backpressure.DROP_BUFFER_BY_OLDEST; int droppedCount = 0; while (customLogs.size() >= bufferSize) { @@ -159,28 +205,32 @@ void addLogToLast(CustomLog log) { customLogs.poll(); droppedCount++; } else { - Logger.d("the queue is already full and reject the new element."); + Logger.d("the log buffer is already full and reject the new log."); return; } } - Logger.d("filtered out %d overflowed old elements.", droppedCount); + Logger.d("filtered out %d overflowed logs from the oldests.", droppedCount); customLogs.addLast(log); - if (extruder.isConnected()) { - extrudeAllInBuffer(); + if (transmitter.isConnected()) { + sendAllInBuffer(); } } - void extrudeAllInBuffer() { + /** + * send all logs from the oldest in the log buffers + * If sending a log failed, it will be put into the head of the log buffer. And then, this schedules the sending-logs instruction with some delay. + */ + void sendAllInBuffer() { while (!customLogs.isEmpty()) { CustomLog log = customLogs.poll(); - if (!extruder.sendLog(log)) { - // push back + if (!transmitter.sendLog(log)) { + // Don't lost the failed log customLogs.addFirst(log); - sendEmptyMessageDelayed(WHAT_EXTRUDE_ALL_BUFFER, 1000L); + sendEmptyMessageDelayed(WHAT_SEND_LOGS, 1000L); break; } } @@ -189,13 +239,13 @@ void extrudeAllInBuffer() { @Override public void handleMessage(Message msg) { switch (msg.what) { - case WHAT_EXTRUDE_ALL_BUFFER: { - extrudeAllInBuffer(); + case WHAT_SEND_LOGS: { + sendAllInBuffer(); break; } default: { - if (msg.what >= WHAT_PUSH_LOG) { + if (msg.what >= WHAT_ADD_NEW_LOG) { CustomLog log = (CustomLog) msg.obj; addLogToLast(log); } @@ -205,8 +255,13 @@ public void handleMessage(Message msg) { } } + /** + * To avoid what number conflicts, sdk uses a simple counter. + * + * @return positive number, which is greater than or equal to the value of {@link #WHAT_ADD_NEW_LOG} + */ private int getAndIncrementPushWhatOffset() { - if (pushWhatOffset > 1000) { + if (pushWhatOffset > maxWhatOffset) { pushWhatOffset = 0; } return pushWhatOffset++; diff --git a/sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java b/sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java index e5d52be..644e284 100644 --- a/sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java +++ b/sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java @@ -39,5 +39,10 @@ public boolean matches(Bundle argument) { return true; } + + @Override + public String toString() { + return expected.toString(); + } } } diff --git a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java index cf5526c..fbb0efc 100644 --- a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java +++ b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java @@ -42,18 +42,24 @@ public void before() { } @Test(timeout = 3000L) - public void check_buffer_size_works() throws RemoteException { - CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(5).build(); + public void check_buffer_size_works_with_drop_by_oldest() throws RemoteException, InterruptedException { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(5).setBackpressure(CustomLogConfiguration.Backpressure.DROP_BUFFER_BY_OLDEST).build(); CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); for (int i = 0; i < 10; i++) { customLogTransmitter.transmit("type", String.valueOf(i)); } + Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + customLogTransmitter.connect(service); Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + for (int i = 0; i < 5; i++) { + Mockito.verify(service, Mockito.never()).sendEvent(Mockito.eq(PACKAGE_NAME), Mockito.eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), BundleMatcher.eq(createLogExtra("type", String.valueOf(i)))); + } + VerificationMode once = Mockito.times(1); for (int i = 5; i < 10; i++) { @@ -61,6 +67,32 @@ public void check_buffer_size_works() throws RemoteException { } } + @Test(timeout = 3000L) + public void check_buffer_size_works_with_preserve_buffer() throws RemoteException { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(5).setBackpressure(CustomLogConfiguration.Backpressure.PRESERVE_BUFFER).build(); + CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + + for (int i = 0; i < 10; i++) { + customLogTransmitter.transmit("type", String.valueOf(i)); + } + + Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + + customLogTransmitter.connect(service); + + Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + + VerificationMode once = Mockito.times(1); + + for (int i = 0; i < 5; i++) { + Mockito.verify(service, once).sendEvent(Mockito.eq(PACKAGE_NAME), Mockito.eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), BundleMatcher.eq(createLogExtra("type", String.valueOf(i)))); + } + + for (int i = 5; i < 10; i++) { + Mockito.verify(service, Mockito.never()).sendEvent(Mockito.eq(PACKAGE_NAME), Mockito.eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), BundleMatcher.eq(createLogExtra("type", String.valueOf(i)))); + } + } + @Test(timeout = 3000L) public void transmit_works_regardless_of_service() { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(8).build(); diff --git a/sdk/src/test/java/com/deploygate/sdk/DeployGateInterfaceTest.java b/sdk/src/test/java/com/deploygate/sdk/DeployGateInterfaceTest.java index 0505652..08956e8 100644 --- a/sdk/src/test/java/com/deploygate/sdk/DeployGateInterfaceTest.java +++ b/sdk/src/test/java/com/deploygate/sdk/DeployGateInterfaceTest.java @@ -90,6 +90,11 @@ public void install__Application_String_DeployGateCallback_boolean() { DeployGate.install(app, "author", callback, true); } + @Test + public void install__Application_String_DeployGateCallback_CustomLogConfiguration() { + DeployGate.install(app, "author", callback, true, new CustomLogConfiguration.Builder().build()); + } + @Test public void refresh() { DeployGate.refresh(); From 02b374c6da0f162b3a398b252741d87e29ccd248 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sun, 10 Apr 2022 14:42:27 +0900 Subject: [PATCH 07/18] fix: modify r8 configuration of sample project to check how sdk is r8-ed --- sample/proguard-project.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sample/proguard-project.txt b/sample/proguard-project.txt index 8bb8ea0..3b27d29 100644 --- a/sample/proguard-project.txt +++ b/sample/proguard-project.txt @@ -19,3 +19,8 @@ # public *; #} -keep class * extends android.os.IInterface + +# to check the R8 results except obfuscation +-keepnames class com.deploygate.sdk.** { + *; +} \ No newline at end of file From a606bdf20ed524b1e07e76d96cbd5639a67742a9 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sun, 10 Apr 2022 14:44:42 +0900 Subject: [PATCH 08/18] feat: add a barrier to reject too large log transmitter without any barrier holds a log that always fails on the head of the queue. --- .../java/com/deploygate/sdk/CustomLog.java | 11 +++ .../deploygate/sdk/CustomLogTransmitter.java | 98 +++++++++++++++++-- .../java/com/deploygate/sdk/DeployGate.java | 2 + .../com/deploygate/sdk/internal/Logger.java | 24 ----- 4 files changed, 102 insertions(+), 33 deletions(-) diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLog.java b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java index a8e306a..60b6570 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLog.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java @@ -6,6 +6,7 @@ class CustomLog implements Parcelable { public final String type; public final String body; + private int retryCount; CustomLog( String type, @@ -13,11 +14,20 @@ class CustomLog implements Parcelable { ) { this.type = type; this.body = body; + this.retryCount = 0; } protected CustomLog(Parcel in) { type = in.readString(); body = in.readString(); + retryCount = in.readInt(); + } + + /** + * @return the number of current attempts + */ + int getAndIncrementRetryCount() { + return retryCount++; } public static final Creator CREATOR = new Creator() { @@ -44,5 +54,6 @@ public void writeToParcel( ) { dest.writeString(type); dest.writeString(body); + dest.writeInt(retryCount); } } diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java index 7450d4a..5832f79 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java @@ -6,6 +6,7 @@ import android.os.Looper; import android.os.Message; import android.os.RemoteException; +import android.text.TextUtils; import com.deploygate.sdk.internal.Logger; import com.deploygate.service.DeployGateEvent; @@ -20,19 +21,22 @@ *

* - Enqueue a new log to the buffer pool * - Send logs in the order of FIFO - * + *

* The cost of polling the log buffer pool may be high, so SDK starts sending logs only while a service is active. *

* - When a new log is stored to the buffer * - */ class CustomLogTransmitter { + private static final int MAX_RETRY_COUNT = 3; + private final String packageName; private final CustomLogConfiguration configuration; @SuppressWarnings("FieldCanBeLocal") private final HandlerThread thread; private CustomLogHandler handler; + private boolean isDisabled; private volatile IDeployGateSdkService service; @@ -40,14 +44,33 @@ class CustomLogTransmitter { String packageName, CustomLogConfiguration configuration ) { + if (TextUtils.isEmpty(packageName)) { + throw new IllegalArgumentException("packageName must be present"); + } + + if (configuration == null) { + throw new IllegalArgumentException("configuration must not be null"); + } + this.packageName = packageName; this.configuration = configuration; + this.isDisabled = true; this.thread = new HandlerThread("deploygate-sdk-event-log"); this.thread.start(); } + /** + * Bind a service and trigger several instructions immediately. + * + * @param service + * the latest service connection + */ public final synchronized void connect(IDeployGateSdkService service) { + if (service == null) { + throw new IllegalArgumentException("service must not be null"); + } + ensureHandlerInitialized(); handler.cancelPendingSendLogsInstruction(); @@ -55,6 +78,9 @@ public final synchronized void connect(IDeployGateSdkService service) { handler.enqueueSendLogsInstruction(); } + /** + * Release a service connection and cancel all pending instructions. + */ public final void disconnect() { ensureHandlerInitialized(); @@ -62,16 +88,55 @@ public final void disconnect() { this.service = null; } + /** + * Transmit custom logs to DeployGate client service. All transmissions will be scheduled to an exclusive thread. + * + * @param type + * custom log type + * @param body + * custom log body + */ public final synchronized void transmit( String type, String body ) { + if (isDisabled) { + return; + } + ensureHandlerInitialized(); CustomLog log = new CustomLog(type, body); handler.enqueueAddNewLogInstruction(log); } + /** + * Disable transmissions. + * + * @param disabled + * specify true if wanna disable the transmitter, otherwise false. + */ + public final void setDisabled(boolean disabled) { + isDisabled = disabled; + + if (disabled) { + Logger.d("Disabled custom log transmitter"); + } else { + Logger.d("Enabled custom log transmitter"); + } + } + + /** + * Check if this transmitter has a service connection. + *

+ * The connection may return {@link android.os.DeadObjectException} even if this returns true; + * + * @return true if a service connection is assigned, otherwise false. + */ + public final boolean hasServiceConnection() { + return service != null; + } + /** * @return * @@ -85,7 +150,19 @@ int getPendingCount() { return handler.customLogs.size(); } - private boolean sendLog(CustomLog log) { + /** + * Send a single log to the receiver. + *

+ * SDK can't send logs in bulk to avoid TransactionTooLargeException + *

+ * Visible only for testing + * + * @param log + * a custom log to send + * + * @return true if this can transmit the custom log, otherwise false. + */ + boolean sendLog(CustomLog log) { IDeployGateSdkService service = this.service; if (service == null) { @@ -100,15 +177,18 @@ private boolean sendLog(CustomLog log) { service.sendEvent(packageName, DeployGateEvent.ACTION_SEND_CUSTOM_LOG, extras); return true; } catch (RemoteException e) { - Logger.w("failed to send custom log: %s", e.getMessage()); + int currentAttempts = log.getAndIncrementRetryCount(); + + if (currentAttempts >= MAX_RETRY_COUNT) { + Logger.e("failed to send custom log and exceeded the max retry count: %s", e.getMessage()); + } else { + Logger.w("failed to send custom log %d times: %s", currentAttempts + 1, e.getMessage()); + } + return false; } } - private boolean isConnected() { - return service != null; - } - private void ensureHandlerInitialized() { if (handler != null) { return; @@ -214,7 +294,7 @@ void addLogToLast(CustomLog log) { customLogs.addLast(log); - if (transmitter.isConnected()) { + if (transmitter.hasServiceConnection()) { sendAllInBuffer(); } } @@ -256,7 +336,7 @@ public void handleMessage(Message msg) { } /** - * To avoid what number conflicts, sdk uses a simple counter. + * To avoid WHAT number conflicts, sdk uses a simple counter. * * @return positive number, which is greater than or equal to the value of {@link #WHAT_ADD_NEW_LOG} */ diff --git a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java index 7ceb74a..2a7cd4f 100644 --- a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java +++ b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java @@ -246,10 +246,12 @@ private DeployGate( private boolean initService(boolean isBoot) { if (isDeployGateAvailable()) { Log.v(TAG, "DeployGate installation detected. Initializing."); + mCustomLogTransmitter.setDisabled(false); bindToService(isBoot); return true; } else { Log.v(TAG, "DeployGate is not available on this device."); + mCustomLogTransmitter.setDisabled(true); mInitializedLatch.countDown(); mIsDeployGateAvailable = false; callbackDeployGateUnavailable(); diff --git a/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java b/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java index 11e3ea4..64ed6b0 100644 --- a/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java +++ b/sdk/src/main/java/com/deploygate/sdk/internal/Logger.java @@ -11,10 +11,6 @@ public static void d( String format, Object... args ) { - if (!Config.DEBUG) { - return; - } - Log.d(TAG, String.format(Locale.US, format, args)); } @@ -22,10 +18,6 @@ public static void i( String format, Object... args ) { - if (!Config.DEBUG) { - return; - } - Log.i(TAG, String.format(Locale.US, format, args)); } @@ -33,10 +25,6 @@ public static void w( String format, Object... args ) { - if (!Config.DEBUG) { - return; - } - Log.w(TAG, String.format(Locale.US, format, args)); } @@ -45,10 +33,6 @@ public static void w( String format, Object... args ) { - if (!Config.DEBUG) { - return; - } - Log.w(TAG, String.format(Locale.US, format, args), th); } @@ -56,10 +40,6 @@ public static void e( String format, Object... args ) { - if (!Config.DEBUG) { - return; - } - Log.e(TAG, String.format(Locale.US, format, args)); } @@ -68,10 +48,6 @@ public static void e( String format, Object... args ) { - if (!Config.DEBUG) { - return; - } - Log.e(TAG, String.format(Locale.US, format, args), th); } } From 9af198440aed4b8e797767dd9c52e5cda75fb9e7 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sun, 10 Apr 2022 16:01:29 +0900 Subject: [PATCH 09/18] fix: sdkMock interface --- .../sdk/{ => mockito}/BundleMatcher.java | 0 .../sdk/CustomLogConfiguration.java | 51 +++++++++++++++++++ .../java/com/deploygate/sdk/DeployGate.java | 9 ++++ 3 files changed, 60 insertions(+) rename sdk/src/test/java/com/deploygate/sdk/{ => mockito}/BundleMatcher.java (100%) create mode 100644 sdkMock/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java diff --git a/sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java b/sdk/src/test/java/com/deploygate/sdk/mockito/BundleMatcher.java similarity index 100% rename from sdk/src/test/java/com/deploygate/sdk/BundleMatcher.java rename to sdk/src/test/java/com/deploygate/sdk/mockito/BundleMatcher.java diff --git a/sdkMock/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java b/sdkMock/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java new file mode 100644 index 0000000..2978d90 --- /dev/null +++ b/sdkMock/src/main/java/com/deploygate/sdk/CustomLogConfiguration.java @@ -0,0 +1,51 @@ +package com.deploygate.sdk; + +public class CustomLogConfiguration { + public enum Backpressure { + /** + * SDK rejects new logs if buffer size is exceeded + */ + PRESERVE_BUFFER, + + /** + * SDK drops logs from the oldest if buffer size is exceeded + */ + DROP_BUFFER_BY_OLDEST + } + + private CustomLogConfiguration( + ) { + } + + public static class Builder { + /** + * @param backpressure + * the strategy of the backpressure in the log buffer + * + * @return self + * + * @see Backpressure + */ + public Builder setBackpressure(Backpressure backpressure) { + if (backpressure == null) { + throw new IllegalArgumentException("backpressure must be non-null"); + } + + return this; + } + + /** + * @param bufferSize + * the max size of the log buffer + * + * @return self + */ + public Builder setBufferSize(int bufferSize) { + return this; + } + + public CustomLogConfiguration build() { + return new CustomLogConfiguration(); + } + } +} diff --git a/sdkMock/src/main/java/com/deploygate/sdk/DeployGate.java b/sdkMock/src/main/java/com/deploygate/sdk/DeployGate.java index 976d778..be7867f 100644 --- a/sdkMock/src/main/java/com/deploygate/sdk/DeployGate.java +++ b/sdkMock/src/main/java/com/deploygate/sdk/DeployGate.java @@ -50,6 +50,15 @@ public static void install( ) { } + public static void install( + Application app, + String author, + DeployGateCallback callback, + boolean forceApplyOnReleaseBuild, + CustomLogConfiguration configuration + ) { + } + public static void refresh() { } From f98be2373cf0332c3e66d38b1f8e56d213a62df3 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sun, 10 Apr 2022 16:01:54 +0900 Subject: [PATCH 10/18] feat: Bundle matcher for Truth --- .../com/deploygate/sdk/helper/Bundles.java | 28 +++++++++ .../deploygate/sdk/mockito/BundleMatcher.java | 24 ++------ .../deploygate/sdk/truth/BundleSubject.java | 61 +++++++++++++++++++ 3 files changed, 94 insertions(+), 19 deletions(-) create mode 100644 sdk/src/test/java/com/deploygate/sdk/helper/Bundles.java create mode 100644 sdk/src/test/java/com/deploygate/sdk/truth/BundleSubject.java diff --git a/sdk/src/test/java/com/deploygate/sdk/helper/Bundles.java b/sdk/src/test/java/com/deploygate/sdk/helper/Bundles.java new file mode 100644 index 0000000..4f74555 --- /dev/null +++ b/sdk/src/test/java/com/deploygate/sdk/helper/Bundles.java @@ -0,0 +1,28 @@ +package com.deploygate.sdk.helper; + +import android.os.Bundle; + +import java.util.Objects; + +public class Bundles { + public static boolean equals( + Bundle left, + Bundle right + ) { + if (left.size() != right.size()) { + return false; + } + + if (!left.keySet().equals(right.keySet())) { + return false; + } + + for (final String key : left.keySet()) { + if (!Objects.equals(left.get(key), right.get(key))) { + return false; + } + } + + return true; + } +} diff --git a/sdk/src/test/java/com/deploygate/sdk/mockito/BundleMatcher.java b/sdk/src/test/java/com/deploygate/sdk/mockito/BundleMatcher.java index 644e284..a9f8771 100644 --- a/sdk/src/test/java/com/deploygate/sdk/mockito/BundleMatcher.java +++ b/sdk/src/test/java/com/deploygate/sdk/mockito/BundleMatcher.java @@ -1,14 +1,14 @@ -package com.deploygate.sdk; +package com.deploygate.sdk.mockito; import android.os.Bundle; -import org.mockito.ArgumentMatcher; +import com.deploygate.sdk.helper.Bundles; -import java.util.Objects; +import org.mockito.ArgumentMatcher; import static org.mockito.internal.progress.ThreadSafeMockingProgress.mockingProgress; -class BundleMatcher { +public class BundleMatcher { public static Bundle eq(Bundle expected) { mockingProgress().getArgumentMatcherStorage().reportMatcher(new Equals(expected)); return expected; @@ -23,21 +23,7 @@ public Equals(Bundle expected) { @Override public boolean matches(Bundle argument) { - if (expected.size() != argument.size()) { - return false; - } - - if (!expected.keySet().equals(argument.keySet())) { - return false; - } - - for (final String key : expected.keySet()) { - if (!Objects.equals(expected.get(key), argument.get(key))) { - return false; - } - } - - return true; + return Bundles.equals(expected, argument); } @Override diff --git a/sdk/src/test/java/com/deploygate/sdk/truth/BundleSubject.java b/sdk/src/test/java/com/deploygate/sdk/truth/BundleSubject.java new file mode 100644 index 0000000..0e8402b --- /dev/null +++ b/sdk/src/test/java/com/deploygate/sdk/truth/BundleSubject.java @@ -0,0 +1,61 @@ +package com.deploygate.sdk.truth; + +import android.os.Bundle; + +import com.deploygate.sdk.helper.Bundles; +import com.google.common.truth.Fact; +import com.google.common.truth.FailureMetadata; +import com.google.common.truth.Subject; + +import java.util.Locale; + +import static com.google.common.truth.Truth.assertAbout; + +public class BundleSubject extends Subject { + public static Factory bundles() { + return new Factory() { + @Override + public BundleSubject createSubject( + FailureMetadata metadata, + Bundle actual + ) { + return new BundleSubject(metadata, actual); + } + }; + } + + public static BundleSubject assertThat(Bundle actual) { + return assertAbout(bundles()).that(actual); + } + + private final Bundle actual; + + /** + * Constructor for use by subclasses. If you want to create an instance of this class itself, call + * {@link Subject#check(String, Object ..) check(...)}{@code .that(actual)}. + * + * @param metadata + * @param actual + */ + protected BundleSubject( + FailureMetadata metadata, + Bundle actual + ) { + super(metadata, actual); + this.actual = actual; + } + + @Override + public void isEqualTo(Object expected) { + if (!Bundles.equals((Bundle) expected, actual)) { + failWithActual(Fact.simpleFact(String.format(Locale.US, "%s to be same to %s", expected.toString(), actual.toString()))); + } + } + + @Override + public void isNotEqualTo(Object unexpected) { + if (Bundles.equals((Bundle) unexpected, actual)) { + failWithActual(Fact.simpleFact(String.format(Locale.US, "%s to unexpectedly be same to %s", unexpected.toString(), actual.toString()))); + } + } +} From 721566c2f26f9ab3a6a007e3dd8ccd63254a6edc Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Sun, 10 Apr 2022 16:02:47 +0900 Subject: [PATCH 11/18] feat: add a retry barrier test --- .../java/com/deploygate/sdk/CustomLog.java | 13 +++ .../deploygate/sdk/CustomLogTransmitter.java | 36 ++++---- .../java/com/deploygate/sdk/DeployGate.java | 4 +- .../com/deploygate/sdk/CustomLogTest.java | 31 +++++++ .../sdk/CustomLogTransmitterTest.java | 87 ++++++++++++++++--- 5 files changed, 138 insertions(+), 33 deletions(-) create mode 100644 sdk/src/test/java/com/deploygate/sdk/CustomLogTest.java diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLog.java b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java index 60b6570..13cfa28 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLog.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java @@ -1,8 +1,11 @@ package com.deploygate.sdk; +import android.os.Bundle; import android.os.Parcel; import android.os.Parcelable; +import com.deploygate.service.DeployGateEvent; + class CustomLog implements Parcelable { public final String type; public final String body; @@ -30,6 +33,16 @@ int getAndIncrementRetryCount() { return retryCount++; } + /** + * @return a bundle to send to the client service + */ + Bundle toExtras() { + Bundle extras = new Bundle(); + extras.putSerializable(DeployGateEvent.EXTRA_LOG, body); + extras.putSerializable(DeployGateEvent.EXTRA_LOG_TYPE, type); + return extras; + } + public static final Creator CREATOR = new Creator() { @Override public CustomLog createFromParcel(Parcel in) { diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java index 5832f79..6b34036 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java @@ -1,6 +1,5 @@ package com.deploygate.sdk; -import android.os.Bundle; import android.os.Handler; import android.os.HandlerThread; import android.os.Looper; @@ -29,6 +28,9 @@ */ class CustomLogTransmitter { private static final int MAX_RETRY_COUNT = 3; + static final int SEND_LOG_RESULT_SUCCESS = 0; + static final int SEND_LOG_RESULT_FAILURE_RETRIABLE = -1; + static final int SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED = -2; private final String packageName; private final CustomLogConfiguration configuration; @@ -36,7 +38,7 @@ class CustomLogTransmitter { @SuppressWarnings("FieldCanBeLocal") private final HandlerThread thread; private CustomLogHandler handler; - private boolean isDisabled; + private boolean isDisabledTransmission; private volatile IDeployGateSdkService service; @@ -54,7 +56,7 @@ class CustomLogTransmitter { this.packageName = packageName; this.configuration = configuration; - this.isDisabled = true; + this.isDisabledTransmission = false; this.thread = new HandlerThread("deploygate-sdk-event-log"); this.thread.start(); @@ -100,7 +102,7 @@ public final synchronized void transmit( String type, String body ) { - if (isDisabled) { + if (isDisabledTransmission) { return; } @@ -113,13 +115,13 @@ public final synchronized void transmit( /** * Disable transmissions. * - * @param disabled + * @param disabledTransmission * specify true if wanna disable the transmitter, otherwise false. */ - public final void setDisabled(boolean disabled) { - isDisabled = disabled; + public final void setDisabledTransmission(boolean disabledTransmission) { + isDisabledTransmission = disabledTransmission; - if (disabled) { + if (disabledTransmission) { Logger.d("Disabled custom log transmitter"); } else { Logger.d("Enabled custom log transmitter"); @@ -162,30 +164,28 @@ int getPendingCount() { * * @return true if this can transmit the custom log, otherwise false. */ - boolean sendLog(CustomLog log) { + int sendLog(CustomLog log) { IDeployGateSdkService service = this.service; if (service == null) { - return false; + // Don't increment retry count + return SEND_LOG_RESULT_FAILURE_RETRIABLE; } try { - Bundle extras = new Bundle(); - extras.putSerializable(DeployGateEvent.EXTRA_LOG, log.body); - extras.putSerializable(DeployGateEvent.EXTRA_LOG_TYPE, log.type); - - service.sendEvent(packageName, DeployGateEvent.ACTION_SEND_CUSTOM_LOG, extras); - return true; + service.sendEvent(packageName, DeployGateEvent.ACTION_SEND_CUSTOM_LOG, log.toExtras()); + return SEND_LOG_RESULT_SUCCESS; } catch (RemoteException e) { int currentAttempts = log.getAndIncrementRetryCount(); if (currentAttempts >= MAX_RETRY_COUNT) { Logger.e("failed to send custom log and exceeded the max retry count: %s", e.getMessage()); + return SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED; } else { Logger.w("failed to send custom log %d times: %s", currentAttempts + 1, e.getMessage()); } - return false; + return SEND_LOG_RESULT_FAILURE_RETRIABLE; } } @@ -307,7 +307,7 @@ void sendAllInBuffer() { while (!customLogs.isEmpty()) { CustomLog log = customLogs.poll(); - if (!transmitter.sendLog(log)) { + if (transmitter.sendLog(log) == SEND_LOG_RESULT_FAILURE_RETRIABLE) { // Don't lost the failed log customLogs.addFirst(log); sendEmptyMessageDelayed(WHAT_SEND_LOGS, 1000L); diff --git a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java index 2a7cd4f..f8b097a 100644 --- a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java +++ b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java @@ -246,12 +246,12 @@ private DeployGate( private boolean initService(boolean isBoot) { if (isDeployGateAvailable()) { Log.v(TAG, "DeployGate installation detected. Initializing."); - mCustomLogTransmitter.setDisabled(false); + mCustomLogTransmitter.setDisabledTransmission(false); bindToService(isBoot); return true; } else { Log.v(TAG, "DeployGate is not available on this device."); - mCustomLogTransmitter.setDisabled(true); + mCustomLogTransmitter.setDisabledTransmission(true); mInitializedLatch.countDown(); mIsDeployGateAvailable = false; callbackDeployGateUnavailable(); diff --git a/sdk/src/test/java/com/deploygate/sdk/CustomLogTest.java b/sdk/src/test/java/com/deploygate/sdk/CustomLogTest.java new file mode 100644 index 0000000..bc1391b --- /dev/null +++ b/sdk/src/test/java/com/deploygate/sdk/CustomLogTest.java @@ -0,0 +1,31 @@ +package com.deploygate.sdk; + +import android.os.Bundle; + +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import com.deploygate.sdk.truth.BundleSubject; + +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class CustomLogTest { + + @Test + public void CustomLog_toExtras_must_be_valid_format() { + CustomLog log = new CustomLog("error", "yes"); + + BundleSubject.assertThat(log.toExtras()).isEqualTo(createLogExtra("error", "yes")); + } + + private static Bundle createLogExtra( + String type, + String body + ) { + Bundle bundle = new Bundle(); + bundle.putString("logType", type); + bundle.putString("log", body); + return bundle; + } +} diff --git a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java index fbb0efc..17f3ac0 100644 --- a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java +++ b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java @@ -1,7 +1,9 @@ package com.deploygate.sdk; import android.os.Bundle; +import android.os.DeadObjectException; import android.os.RemoteException; +import android.os.TransactionTooLargeException; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -15,10 +17,20 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import org.mockito.verification.VerificationMode; import org.robolectric.Shadows; import org.robolectric.annotation.LooperMode; +import java.util.ArrayList; +import java.util.List; + +import static com.deploygate.sdk.mockito.BundleMatcher.eq; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.robolectric.annotation.LooperMode.Mode.PAUSED; @RunWith(AndroidJUnit4.class) @@ -57,13 +69,15 @@ public void check_buffer_size_works_with_drop_by_oldest() throws RemoteException Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); for (int i = 0; i < 5; i++) { - Mockito.verify(service, Mockito.never()).sendEvent(Mockito.eq(PACKAGE_NAME), Mockito.eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), BundleMatcher.eq(createLogExtra("type", String.valueOf(i)))); + CustomLog log = new CustomLog("type", String.valueOf(i)); + Mockito.verify(service, Mockito.never()).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); } VerificationMode once = Mockito.times(1); for (int i = 5; i < 10; i++) { - Mockito.verify(service, once).sendEvent(Mockito.eq(PACKAGE_NAME), Mockito.eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), BundleMatcher.eq(createLogExtra("type", String.valueOf(i)))); + CustomLog log = new CustomLog("type", String.valueOf(i)); + Mockito.verify(service, once).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); } } @@ -85,16 +99,40 @@ public void check_buffer_size_works_with_preserve_buffer() throws RemoteExceptio VerificationMode once = Mockito.times(1); for (int i = 0; i < 5; i++) { - Mockito.verify(service, once).sendEvent(Mockito.eq(PACKAGE_NAME), Mockito.eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), BundleMatcher.eq(createLogExtra("type", String.valueOf(i)))); + CustomLog log = new CustomLog("type", String.valueOf(i)); + Mockito.verify(service, once).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); } for (int i = 5; i < 10; i++) { - Mockito.verify(service, Mockito.never()).sendEvent(Mockito.eq(PACKAGE_NAME), Mockito.eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), BundleMatcher.eq(createLogExtra("type", String.valueOf(i)))); + CustomLog log = new CustomLog("type", String.valueOf(i)); + Mockito.verify(service, Mockito.never()).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); } } @Test(timeout = 3000L) - public void transmit_works_regardless_of_service() { + public void transmit_works_regardless_of_service() throws RemoteException { + customLogTransmitter.connect(service); + + CustomLog noIssue = new CustomLog("type", "noIssue"); + CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); + CustomLog retryExceeded = new CustomLog("type", "retryExceeded"); + + doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(noIssue.toExtras())); + doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); + doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); + + Truth.assertThat(customLogTransmitter.sendLog(noIssue)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_SUCCESS); + Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_SUCCESS); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED); + } + + @Test(timeout = 3000L) + public void retry_barrier_can_prevent_holding_logs_that_always_fail() { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(8).build(); CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); @@ -107,13 +145,36 @@ public void transmit_works_regardless_of_service() { Truth.assertThat(customLogTransmitter.getPendingCount()).isEqualTo(8); } - private static Bundle createLogExtra( - String type, - String body - ) { - Bundle bundle = new Bundle(); - bundle.putString("logType", type); - bundle.putString("log", body); - return bundle; + @Test(timeout = 3000L) + public void transmit_works_as_expected_with_retry_barrier() throws RemoteException { + List extras = new ArrayList<>(); + Answer capture = new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + Bundle extra = invocation.getArgument(2); + extras.add(extra); + return null; + } + }; + + CustomLog noIssue = new CustomLog("type", "noIssue"); + CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); + CustomLog retryExceeded = new CustomLog("type", "retryExceeded"); + + doAnswer(capture).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(noIssue.toExtras())); + doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doAnswer(capture).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); + doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); + + customLogTransmitter.connect(service); + + customLogTransmitter.transmit(successAfterRetries.type, successAfterRetries.body); + customLogTransmitter.transmit(noIssue.type, noIssue.body); + customLogTransmitter.transmit(retryExceeded.type, retryExceeded.body); + + Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + + Truth.assertThat(extras).hasSize(2); + Truth.assertThat(extras.get(0).getString(DeployGateEvent.EXTRA_LOG)).isEqualTo(successAfterRetries.body); + Truth.assertThat(extras.get(1).getString(DeployGateEvent.EXTRA_LOG)).isEqualTo(noIssue.body); } } From 2ea725f1cad0c659af22fb906d54f292debba501 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Mon, 11 Apr 2022 02:24:30 +0900 Subject: [PATCH 12/18] tweak: fix a typo --- sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java index 6b34036..c63fe30 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java @@ -58,7 +58,7 @@ class CustomLogTransmitter { this.configuration = configuration; this.isDisabledTransmission = false; - this.thread = new HandlerThread("deploygate-sdk-event-log"); + this.thread = new HandlerThread("deploygate-sdk-custom-log"); this.thread.start(); } From b8552e2cd281b20912d2e51977d9a367b8719b19 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Mon, 11 Apr 2022 23:39:41 +0900 Subject: [PATCH 13/18] test: Prepared a fake implementation --- .../java/com/deploygate/sdk/CustomLog.java | 24 ++++++ .../sdk/CustomLogTransmitterTest.java | 78 +++++++++--------- .../service/FakeDeployGateClientService.java | 79 +++++++++++++++++++ 3 files changed, 140 insertions(+), 41 deletions(-) create mode 100644 sdk/src/test/java/com/deploygate/service/FakeDeployGateClientService.java diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLog.java b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java index 13cfa28..a0d6bfc 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLog.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java @@ -69,4 +69,28 @@ public void writeToParcel( dest.writeString(body); dest.writeInt(retryCount); } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + CustomLog customLog = (CustomLog) o; + + if (!type.equals(customLog.type)) { + return false; + } + return body.equals(customLog.body); + } + + @Override + public int hashCode() { + int result = type.hashCode(); + result = 31 * result + body.hashCode(); + return result; + } } diff --git a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java index 17f3ac0..7302be9 100644 --- a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java +++ b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java @@ -7,30 +7,27 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; +import com.deploygate.sdk.truth.BundleSubject; import com.deploygate.service.DeployGateEvent; -import com.deploygate.service.IDeployGateSdkService; +import com.deploygate.service.FakeDeployGateClientService; import com.google.common.truth.Truth; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.Mock; +import org.mockito.InOrder; import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; import org.mockito.verification.VerificationMode; import org.robolectric.Shadows; import org.robolectric.annotation.LooperMode; -import java.util.ArrayList; import java.util.List; import static com.deploygate.sdk.mockito.BundleMatcher.eq; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; import static org.robolectric.annotation.LooperMode.Mode.PAUSED; @RunWith(AndroidJUnit4.class) @@ -39,23 +36,18 @@ public class CustomLogTransmitterTest { private static final String PACKAGE_NAME = "com.deploygate.sample"; - @Mock - private IDeployGateSdkService service; - - private CustomLogTransmitter customLogTransmitter; - - private CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); + private FakeDeployGateClientService service; @Before public void before() { - MockitoAnnotations.openMocks(this); - - customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + service = Mockito.spy(new FakeDeployGateClientService(PACKAGE_NAME)); } @Test(timeout = 3000L) public void check_buffer_size_works_with_drop_by_oldest() throws RemoteException, InterruptedException { - CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(5).setBackpressure(CustomLogConfiguration.Backpressure.DROP_BUFFER_BY_OLDEST).build(); + final int bufferSize = 5; + + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(bufferSize).setBackpressure(CustomLogConfiguration.Backpressure.DROP_BUFFER_BY_OLDEST).build(); CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); for (int i = 0; i < 10; i++) { @@ -68,22 +60,25 @@ public void check_buffer_size_works_with_drop_by_oldest() throws RemoteException Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); - for (int i = 0; i < 5; i++) { + for (int i = 0; i < bufferSize; i++) { CustomLog log = new CustomLog("type", String.valueOf(i)); - Mockito.verify(service, Mockito.never()).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); + Mockito.verify(service, never()).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); } VerificationMode once = Mockito.times(1); + InOrder inOrder = Mockito.inOrder(service); - for (int i = 5; i < 10; i++) { + for (int i = bufferSize; i < 10; i++) { CustomLog log = new CustomLog("type", String.valueOf(i)); - Mockito.verify(service, once).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); + inOrder.verify(service, once).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); } } @Test(timeout = 3000L) public void check_buffer_size_works_with_preserve_buffer() throws RemoteException { - CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(5).setBackpressure(CustomLogConfiguration.Backpressure.PRESERVE_BUFFER).build(); + final int bufferSize = 5; + + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(bufferSize).setBackpressure(CustomLogConfiguration.Backpressure.PRESERVE_BUFFER).build(); CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); for (int i = 0; i < 10; i++) { @@ -97,20 +92,23 @@ public void check_buffer_size_works_with_preserve_buffer() throws RemoteExceptio Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); VerificationMode once = Mockito.times(1); + InOrder inOrder = Mockito.inOrder(service); - for (int i = 0; i < 5; i++) { + for (int i = 0; i < bufferSize; i++) { CustomLog log = new CustomLog("type", String.valueOf(i)); - Mockito.verify(service, once).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); + inOrder.verify(service, once).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); } - for (int i = 5; i < 10; i++) { + for (int i = bufferSize; i < 10; i++) { CustomLog log = new CustomLog("type", String.valueOf(i)); - Mockito.verify(service, Mockito.never()).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); + Mockito.verify(service, never()).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); } } @Test(timeout = 3000L) public void transmit_works_regardless_of_service() throws RemoteException { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); + CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); customLogTransmitter.connect(service); CustomLog noIssue = new CustomLog("type", "noIssue"); @@ -132,12 +130,16 @@ public void transmit_works_regardless_of_service() throws RemoteException { } @Test(timeout = 3000L) - public void retry_barrier_can_prevent_holding_logs_that_always_fail() { + public void retry_barrier_can_prevent_holding_logs_that_always_fail() throws RemoteException { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(8).build(); CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); for (int i = 0; i < 10; i++) { - customLogTransmitter.transmit("type", String.valueOf(i)); + CustomLog log = new CustomLog("type", String.valueOf(i)); + + doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); + + customLogTransmitter.transmit(log.type, log.body); } Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); @@ -147,22 +149,14 @@ public void retry_barrier_can_prevent_holding_logs_that_always_fail() { @Test(timeout = 3000L) public void transmit_works_as_expected_with_retry_barrier() throws RemoteException { - List extras = new ArrayList<>(); - Answer capture = new Answer() { - @Override - public Object answer(InvocationOnMock invocation) throws Throwable { - Bundle extra = invocation.getArgument(2); - extras.add(extra); - return null; - } - }; + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); + CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); CustomLog noIssue = new CustomLog("type", "noIssue"); CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); CustomLog retryExceeded = new CustomLog("type", "retryExceeded"); - doAnswer(capture).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(noIssue.toExtras())); - doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doAnswer(capture).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); + doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doCallRealMethod().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); customLogTransmitter.connect(service); @@ -173,8 +167,10 @@ public Object answer(InvocationOnMock invocation) throws Throwable { Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + List extras = service.getEventExtraList(DeployGateEvent.ACTION_SEND_CUSTOM_LOG); + Truth.assertThat(extras).hasSize(2); - Truth.assertThat(extras.get(0).getString(DeployGateEvent.EXTRA_LOG)).isEqualTo(successAfterRetries.body); - Truth.assertThat(extras.get(1).getString(DeployGateEvent.EXTRA_LOG)).isEqualTo(noIssue.body); + BundleSubject.assertThat(extras.get(0)).isEqualTo(successAfterRetries.toExtras()); + BundleSubject.assertThat(extras.get(1)).isEqualTo(noIssue.toExtras()); } } diff --git a/sdk/src/test/java/com/deploygate/service/FakeDeployGateClientService.java b/sdk/src/test/java/com/deploygate/service/FakeDeployGateClientService.java new file mode 100644 index 0000000..d2cf8ec --- /dev/null +++ b/sdk/src/test/java/com/deploygate/service/FakeDeployGateClientService.java @@ -0,0 +1,79 @@ +package com.deploygate.service; + +import android.os.Bundle; +import android.os.IBinder; +import android.os.RemoteException; + +import com.google.common.truth.Truth; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; + +public class FakeDeployGateClientService implements IDeployGateSdkService { + public static final String ACTION_INIT = "com.deploygate.sdk.fake.INIT"; + + private final String packageName; + private final Map> eventExtrasMap; + + public FakeDeployGateClientService( + String packageName + ) { + this.packageName = packageName; + this.eventExtrasMap = new HashMap<>(); + + for (final Field field : DeployGateEvent.class.getFields()) { + final String name = field.getName(); + + if (name.startsWith("ACTION_")) { + try { + eventExtrasMap.put((String) field.get(null), new ArrayList<>()); + } catch (IllegalAccessException e) { + throw new IllegalArgumentException(String.format(Locale.US, "cannot access %s", name), e); + } + } + } + } + + public List getEventExtraList(String action) { + return Collections.unmodifiableList(eventExtrasMap.getOrDefault(action, new ArrayList<>())); + } + + @Override + public void init( + IDeployGateSdkServiceCallback callback, + String packageName, + Bundle extras + ) throws RemoteException { + Truth.assertThat(packageName).isEqualTo(this.packageName); + recordEvent(ACTION_INIT, extras); + } + + @Override + public void sendEvent( + String packageName, + String action, + Bundle extras + ) throws RemoteException { + Truth.assertThat(packageName).isEqualTo(this.packageName); + recordEvent(action, extras); + } + + @Override + public IBinder asBinder() { + return null; + } + + private void recordEvent( + String action, + Bundle extras + ) { + List extraList = Objects.requireNonNull(eventExtrasMap.get(action), String.format(Locale.US, "%s is an unknown action", action)); + extraList.add(extras); + } +} From efbf39dcb606b7edb5f4df987e9f4e3db2e8321e Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Tue, 12 Apr 2022 00:01:13 +0900 Subject: [PATCH 14/18] test: retry barrier and several edge cases --- .../deploygate/sdk/CustomLogTransmitter.java | 2 +- .../sdk/CustomLogTransmitterTest.java | 70 +++++++++++++++++-- 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java index c63fe30..78f7ca6 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java @@ -27,7 +27,7 @@ * - */ class CustomLogTransmitter { - private static final int MAX_RETRY_COUNT = 3; + static final int MAX_RETRY_COUNT = 3; static final int SEND_LOG_RESULT_SUCCESS = 0; static final int SEND_LOG_RESULT_FAILURE_RETRIABLE = -1; static final int SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED = -2; diff --git a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java index 7302be9..fcfc298 100644 --- a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java +++ b/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java @@ -22,12 +22,15 @@ import org.robolectric.annotation.LooperMode; import java.util.List; +import java.util.concurrent.TimeUnit; import static com.deploygate.sdk.mockito.BundleMatcher.eq; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.robolectric.annotation.LooperMode.Mode.PAUSED; @RunWith(AndroidJUnit4.class) @@ -105,6 +108,52 @@ public void check_buffer_size_works_with_preserve_buffer() throws RemoteExceptio } } + @Test(timeout = 3000L) + public void sendLog_always_returns_retriable_status_if_service_is_none() throws RemoteException { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); + CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + + CustomLog noIssue = new CustomLog("type", "noIssue"); + CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); + CustomLog retryExceeded = new CustomLog("type", "retryExceeded"); + + doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(noIssue.toExtras())); + doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); + doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); + + for (int i = 0; i < 10; i++) { + Truth.assertThat(customLogTransmitter.sendLog(noIssue)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + } + } + + @Test(timeout = 3000L) + public void sendLog_uses_retry_barrier() throws RemoteException { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); + CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + customLogTransmitter.connect(service); + + Shadows.shadowOf(customLogTransmitter.getLooper()).pause(); + + CustomLog noIssue = new CustomLog("type", "noIssue"); + CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); + CustomLog retryExceeded = new CustomLog("type", "retryExceeded"); + + doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(noIssue.toExtras())); + doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); + doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); + + Truth.assertThat(customLogTransmitter.sendLog(noIssue)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_SUCCESS); + Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_SUCCESS); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED); + } + @Test(timeout = 3000L) public void transmit_works_regardless_of_service() throws RemoteException { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); @@ -130,21 +179,27 @@ public void transmit_works_regardless_of_service() throws RemoteException { } @Test(timeout = 3000L) - public void retry_barrier_can_prevent_holding_logs_that_always_fail() throws RemoteException { - CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(8).build(); + public void retry_barrier_can_prevent_holding_logs_that_always_fail() throws RemoteException, InterruptedException { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); - for (int i = 0; i < 10; i++) { - CustomLog log = new CustomLog("type", String.valueOf(i)); + doThrow(RemoteException.class).when(service).sendEvent(any(), any(), any()); - doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(log.toExtras())); + customLogTransmitter.connect(service); + for (int i = 0; i < 10; i++) { + CustomLog log = new CustomLog("type", String.valueOf(i)); customLogTransmitter.transmit(log.type, log.body); } Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); - Truth.assertThat(customLogTransmitter.getPendingCount()).isEqualTo(8); + while (customLogTransmitter.getPendingCount() > 0) { + Shadows.shadowOf(customLogTransmitter.getLooper()).idleFor(100, TimeUnit.MILLISECONDS); + } + + Truth.assertThat(customLogTransmitter.getPendingCount()).isEqualTo(0); + Mockito.verify(service, times((CustomLogTransmitter.MAX_RETRY_COUNT + 1) * 10)).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), any()); } @Test(timeout = 3000L) @@ -156,7 +211,8 @@ public void transmit_works_as_expected_with_retry_barrier() throws RemoteExcepti CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); CustomLog retryExceeded = new CustomLog("type", "retryExceeded"); - doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doCallRealMethod().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); + //noinspection unchecked + doThrow(TransactionTooLargeException.class, DeadObjectException.class).doCallRealMethod().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); customLogTransmitter.connect(service); From 9a3064e4693b08350fbad7cbb604cb8a50634336 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Tue, 12 Apr 2022 04:10:13 +0900 Subject: [PATCH 15/18] fix: retrying while a remote service has a trouble cause logs disappear unexpectedly renamed transmitter to instruction serializer --- ...va => CustomLogInstructionSerializer.java} | 80 +++++++---- .../java/com/deploygate/sdk/DeployGate.java | 14 +- ...> CustomLogInstructionSerializerTest.java} | 133 ++++++++++-------- 3 files changed, 131 insertions(+), 96 deletions(-) rename sdk/src/main/java/com/deploygate/sdk/{CustomLogTransmitter.java => CustomLogInstructionSerializer.java} (80%) rename sdk/src/test/java/com/deploygate/sdk/{CustomLogTransmitterTest.java => CustomLogInstructionSerializerTest.java} (54%) diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogInstructionSerializer.java similarity index 80% rename from sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java rename to sdk/src/main/java/com/deploygate/sdk/CustomLogInstructionSerializer.java index 78f7ca6..c754833 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLogTransmitter.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogInstructionSerializer.java @@ -14,20 +14,17 @@ import java.util.LinkedList; /** - * This class transmits pending logs to the DeployGate client. - * The internal handler class creates a buffer pool and assure the sending-order. + * This class serialize the instructions for custom logs and process them in another thread in order of enqueued. + * The internal handler class creates a buffer pool and grantee the order. * *

* - Enqueue a new log to the buffer pool * - Send logs in the order of FIFO *

* The cost of polling the log buffer pool may be high, so SDK starts sending logs only while a service is active. - *

- * - When a new log is stored to the buffer - * - */ -class CustomLogTransmitter { - static final int MAX_RETRY_COUNT = 3; +class CustomLogInstructionSerializer { + static final int MAX_RETRY_COUNT = 2; static final int SEND_LOG_RESULT_SUCCESS = 0; static final int SEND_LOG_RESULT_FAILURE_RETRIABLE = -1; static final int SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED = -2; @@ -38,11 +35,11 @@ class CustomLogTransmitter { @SuppressWarnings("FieldCanBeLocal") private final HandlerThread thread; private CustomLogHandler handler; - private boolean isDisabledTransmission; + private boolean isDisabled; private volatile IDeployGateSdkService service; - CustomLogTransmitter( + CustomLogInstructionSerializer( String packageName, CustomLogConfiguration configuration ) { @@ -56,7 +53,7 @@ class CustomLogTransmitter { this.packageName = packageName; this.configuration = configuration; - this.isDisabledTransmission = false; + this.isDisabled = false; this.thread = new HandlerThread("deploygate-sdk-custom-log"); this.thread.start(); @@ -91,18 +88,18 @@ public final void disconnect() { } /** - * Transmit custom logs to DeployGate client service. All transmissions will be scheduled to an exclusive thread. + * Request sending custom logs to DeployGate client service. All requests will be scheduled to an exclusive thread. * * @param type * custom log type * @param body * custom log body */ - public final synchronized void transmit( + public final synchronized void requestSendingLog( String type, String body ) { - if (isDisabledTransmission) { + if (isDisabled) { return; } @@ -113,23 +110,23 @@ public final synchronized void transmit( } /** - * Disable transmissions. + * Disable accepting instructions. This does not mean interrupting and/or terminating the exclusive thread. * - * @param disabledTransmission - * specify true if wanna disable the transmitter, otherwise false. + * @param isDisabled + * specify true if wanna disable this serializer, otherwise false. */ - public final void setDisabledTransmission(boolean disabledTransmission) { - isDisabledTransmission = disabledTransmission; + public final void setDisabled(boolean isDisabled) { + this.isDisabled = isDisabled; - if (disabledTransmission) { - Logger.d("Disabled custom log transmitter"); + if (isDisabled) { + Logger.d("Disabled custom log instruction serializer"); } else { - Logger.d("Enabled custom log transmitter"); + Logger.d("Enabled custom log instruction serializer"); } } /** - * Check if this transmitter has a service connection. + * Check if this serializer a service connection. *

* The connection may return {@link android.os.DeadObjectException} even if this returns true; * @@ -148,7 +145,15 @@ Looper getLooper() { return handler.getLooper(); } + boolean hasHandlerInitialized() { + return handler != null; + } + int getPendingCount() { + if (handler == null) { + return 0; + } + return handler.customLogs.size(); } @@ -162,7 +167,7 @@ int getPendingCount() { * @param log * a custom log to send * - * @return true if this can transmit the custom log, otherwise false. + * @return true if this could send the custom log, otherwise false. */ int sendLog(CustomLog log) { IDeployGateSdkService service = this.service; @@ -212,7 +217,7 @@ private static class CustomLogHandler extends Handler { private static final int WHAT_SEND_LOGS = 0x30; private static final int WHAT_ADD_NEW_LOG = 0x100; - private final CustomLogTransmitter transmitter; + private final CustomLogInstructionSerializer serializer; private final CustomLogConfiguration.Backpressure backpressure; private final int bufferSize; private final int maxWhatOffset; @@ -222,7 +227,7 @@ private static class CustomLogHandler extends Handler { /** * @param looper * Do not use Main Looper to avoid wasting the main thread resource. - * @param transmitter + * @param serializer * an instance to send logs * @param backpressure * the backpressure strategy of the log buffer, not of instructions. @@ -231,12 +236,12 @@ private static class CustomLogHandler extends Handler { */ CustomLogHandler( Looper looper, - CustomLogTransmitter transmitter, + CustomLogInstructionSerializer serializer, CustomLogConfiguration.Backpressure backpressure, int bufferSize ) { super(looper); - this.transmitter = transmitter; + this.serializer = serializer; this.backpressure = backpressure; this.bufferSize = bufferSize; this.maxWhatOffset = bufferSize * 2; @@ -290,11 +295,11 @@ void addLogToLast(CustomLog log) { } } - Logger.d("filtered out %d overflowed logs from the oldests.", droppedCount); + Logger.d("filtered out %d overflowed logs from the oldest.", droppedCount); customLogs.addLast(log); - if (transmitter.hasServiceConnection()) { + if (serializer.hasServiceConnection()) { sendAllInBuffer(); } } @@ -304,16 +309,29 @@ void addLogToLast(CustomLog log) { * If sending a log failed, it will be put into the head of the log buffer. And then, this schedules the sending-logs instruction with some delay. */ void sendAllInBuffer() { + boolean retry = false; + while (!customLogs.isEmpty()) { CustomLog log = customLogs.poll(); - if (transmitter.sendLog(log) == SEND_LOG_RESULT_FAILURE_RETRIABLE) { + if (serializer.sendLog(log) == SEND_LOG_RESULT_FAILURE_RETRIABLE) { // Don't lost the failed log customLogs.addFirst(log); - sendEmptyMessageDelayed(WHAT_SEND_LOGS, 1000L); + retry = true; break; } } + + if (retry) { + try { + removeMessages(WHAT_SEND_LOGS); + Message msg = obtainMessage(WHAT_SEND_LOGS); + // Put the retry message at front of the queue because delay or enqueuing a message may cause unexpected overflow of the buffer. + sendMessageAtFrontOfQueue(msg); + Thread.sleep(600); // experimental valuea + } catch (InterruptedException ignore) { + } + } } @Override diff --git a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java index f8b097a..7888258 100644 --- a/sdk/src/main/java/com/deploygate/sdk/DeployGate.java +++ b/sdk/src/main/java/com/deploygate/sdk/DeployGate.java @@ -69,7 +69,7 @@ public class DeployGate { private final Context mApplicationContext; private final Handler mHandler; - private final CustomLogTransmitter mCustomLogTransmitter; + private final CustomLogInstructionSerializer mCustomLogInstructionSerializer; private final HashSet mCallbacks; private final String mExpectedAuthor; private String mAuthor; @@ -229,7 +229,7 @@ private DeployGate( ) { mApplicationContext = applicationContext; mHandler = new Handler(); - mCustomLogTransmitter = new CustomLogTransmitter(mApplicationContext.getPackageName(), customLogConfiguration); + mCustomLogInstructionSerializer = new CustomLogInstructionSerializer(mApplicationContext.getPackageName(), customLogConfiguration); mCallbacks = new HashSet(); mExpectedAuthor = author; @@ -246,12 +246,12 @@ private DeployGate( private boolean initService(boolean isBoot) { if (isDeployGateAvailable()) { Log.v(TAG, "DeployGate installation detected. Initializing."); - mCustomLogTransmitter.setDisabledTransmission(false); + mCustomLogInstructionSerializer.setDisabled(false); bindToService(isBoot); return true; } else { Log.v(TAG, "DeployGate is not available on this device."); - mCustomLogTransmitter.setDisabledTransmission(true); + mCustomLogInstructionSerializer.setDisabled(true); mInitializedLatch.countDown(); mIsDeployGateAvailable = false; callbackDeployGateUnavailable(); @@ -307,7 +307,7 @@ public void onServiceConnected( public void onServiceDisconnected(ComponentName name) { Log.v(TAG, "DeployGate service disconneced"); mRemoteService = null; - mCustomLogTransmitter.disconnect(); + mCustomLogInstructionSerializer.disconnect(); } }, Context.BIND_AUTO_CREATE); } @@ -320,7 +320,7 @@ private void requestServiceInit(final boolean isBoot) { args.putInt(DeployGateEvent.EXTRA_SDK_VERSION, SDK_VERSION); try { mRemoteService.init(mRemoteCallback, mApplicationContext.getPackageName(), args); - mCustomLogTransmitter.connect(mRemoteService); + mCustomLogInstructionSerializer.connect(mRemoteService); } catch (RemoteException e) { Log.w(TAG, "DeployGate service failed to be initialized."); } @@ -1098,7 +1098,7 @@ void sendLog( String type, String body ) { - mCustomLogTransmitter.transmit(type, body); + mCustomLogInstructionSerializer.requestSendingLog(type, body); } /** diff --git a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java b/sdk/src/test/java/com/deploygate/sdk/CustomLogInstructionSerializerTest.java similarity index 54% rename from sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java rename to sdk/src/test/java/com/deploygate/sdk/CustomLogInstructionSerializerTest.java index fcfc298..67d8571 100644 --- a/sdk/src/test/java/com/deploygate/sdk/CustomLogTransmitterTest.java +++ b/sdk/src/test/java/com/deploygate/sdk/CustomLogInstructionSerializerTest.java @@ -35,7 +35,7 @@ @RunWith(AndroidJUnit4.class) @LooperMode(PAUSED) -public class CustomLogTransmitterTest { +public class CustomLogInstructionSerializerTest { private static final String PACKAGE_NAME = "com.deploygate.sample"; @@ -51,17 +51,17 @@ public void check_buffer_size_works_with_drop_by_oldest() throws RemoteException final int bufferSize = 5; CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(bufferSize).setBackpressure(CustomLogConfiguration.Backpressure.DROP_BUFFER_BY_OLDEST).build(); - CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + CustomLogInstructionSerializer customLogInstructionSerializer = new CustomLogInstructionSerializer(PACKAGE_NAME, configuration); for (int i = 0; i < 10; i++) { - customLogTransmitter.transmit("type", String.valueOf(i)); + customLogInstructionSerializer.requestSendingLog("type", String.valueOf(i)); } - Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idle(); - customLogTransmitter.connect(service); + customLogInstructionSerializer.connect(service); - Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idle(); for (int i = 0; i < bufferSize; i++) { CustomLog log = new CustomLog("type", String.valueOf(i)); @@ -82,17 +82,17 @@ public void check_buffer_size_works_with_preserve_buffer() throws RemoteExceptio final int bufferSize = 5; CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().setBufferSize(bufferSize).setBackpressure(CustomLogConfiguration.Backpressure.PRESERVE_BUFFER).build(); - CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + CustomLogInstructionSerializer customLogInstructionSerializer = new CustomLogInstructionSerializer(PACKAGE_NAME, configuration); for (int i = 0; i < 10; i++) { - customLogTransmitter.transmit("type", String.valueOf(i)); + customLogInstructionSerializer.requestSendingLog("type", String.valueOf(i)); } - Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idle(); - customLogTransmitter.connect(service); + customLogInstructionSerializer.connect(service); - Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idle(); VerificationMode once = Mockito.times(1); InOrder inOrder = Mockito.inOrder(service); @@ -111,7 +111,7 @@ public void check_buffer_size_works_with_preserve_buffer() throws RemoteExceptio @Test(timeout = 3000L) public void sendLog_always_returns_retriable_status_if_service_is_none() throws RemoteException { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); - CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + CustomLogInstructionSerializer customLogInstructionSerializer = new CustomLogInstructionSerializer(PACKAGE_NAME, configuration); CustomLog noIssue = new CustomLog("type", "noIssue"); CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); @@ -122,19 +122,19 @@ public void sendLog_always_returns_retriable_status_if_service_is_none() throws doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); for (int i = 0; i < 10; i++) { - Truth.assertThat(customLogTransmitter.sendLog(noIssue)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogInstructionSerializer.sendLog(noIssue)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogInstructionSerializer.sendLog(successAfterRetries)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogInstructionSerializer.sendLog(retryExceeded)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_FAILURE_RETRIABLE); } } @Test(timeout = 3000L) public void sendLog_uses_retry_barrier() throws RemoteException { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); - CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); - customLogTransmitter.connect(service); + CustomLogInstructionSerializer customLogInstructionSerializer = new CustomLogInstructionSerializer(PACKAGE_NAME, configuration); + customLogInstructionSerializer.connect(service); - Shadows.shadowOf(customLogTransmitter.getLooper()).pause(); + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).pause(); CustomLog noIssue = new CustomLog("type", "noIssue"); CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); @@ -144,68 +144,85 @@ public void sendLog_uses_retry_barrier() throws RemoteException { doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); - Truth.assertThat(customLogTransmitter.sendLog(noIssue)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_SUCCESS); - Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_SUCCESS); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED); + Truth.assertThat(customLogInstructionSerializer.sendLog(noIssue)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_SUCCESS); + Truth.assertThat(customLogInstructionSerializer.sendLog(successAfterRetries)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogInstructionSerializer.sendLog(successAfterRetries)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogInstructionSerializer.sendLog(successAfterRetries)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_SUCCESS); + Truth.assertThat(customLogInstructionSerializer.sendLog(retryExceeded)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogInstructionSerializer.sendLog(retryExceeded)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_FAILURE_RETRIABLE); + Truth.assertThat(customLogInstructionSerializer.sendLog(retryExceeded)).isEqualTo(CustomLogInstructionSerializer.SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED); } @Test(timeout = 3000L) - public void transmit_works_regardless_of_service() throws RemoteException { + public void requestSendingLog_works_regardless_of_service() throws RemoteException { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); - CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); - customLogTransmitter.connect(service); + CustomLogInstructionSerializer customLogInstructionSerializer = new CustomLogInstructionSerializer(PACKAGE_NAME, configuration); - CustomLog noIssue = new CustomLog("type", "noIssue"); - CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); - CustomLog retryExceeded = new CustomLog("type", "retryExceeded"); + // Don't connect a service - doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(noIssue.toExtras())); - doThrow(TransactionTooLargeException.class).doThrow(DeadObjectException.class).doNothing().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); - doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); + for (int i = 0; i < 30; i++) { + customLogInstructionSerializer.requestSendingLog("type", "body"); + } + + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idle(); + + Truth.assertThat(customLogInstructionSerializer.getPendingCount()).isEqualTo(30); + } + + @Test(timeout = 3000L) + public void requestSendingLog_does_nothing_if_disabled() throws RemoteException { + CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); + CustomLogInstructionSerializer customLogInstructionSerializer = new CustomLogInstructionSerializer(PACKAGE_NAME, configuration); + + customLogInstructionSerializer.setDisabled(true); + + for (int i = 0; i < 30; i++) { + customLogInstructionSerializer.requestSendingLog("type", "body"); + } + + Truth.assertThat(customLogInstructionSerializer.hasHandlerInitialized()).isFalse(); + Truth.assertThat(customLogInstructionSerializer.getPendingCount()).isEqualTo(0); + + // Even if a service connection is established, this does nothing. + customLogInstructionSerializer.connect(service); + + for (int i = 0; i < 30; i++) { + customLogInstructionSerializer.requestSendingLog("type", "body"); + } + + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idle(); - Truth.assertThat(customLogTransmitter.sendLog(noIssue)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_SUCCESS); - Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(successAfterRetries)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_SUCCESS); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRIABLE); - Truth.assertThat(customLogTransmitter.sendLog(retryExceeded)).isEqualTo(CustomLogTransmitter.SEND_LOG_RESULT_FAILURE_RETRY_EXCEEDED); + Truth.assertThat(customLogInstructionSerializer.getPendingCount()).isEqualTo(0); } @Test(timeout = 3000L) public void retry_barrier_can_prevent_holding_logs_that_always_fail() throws RemoteException, InterruptedException { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); - CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + CustomLogInstructionSerializer customLogInstructionSerializer = new CustomLogInstructionSerializer(PACKAGE_NAME, configuration); doThrow(RemoteException.class).when(service).sendEvent(any(), any(), any()); - customLogTransmitter.connect(service); + customLogInstructionSerializer.connect(service); for (int i = 0; i < 10; i++) { CustomLog log = new CustomLog("type", String.valueOf(i)); - customLogTransmitter.transmit(log.type, log.body); + customLogInstructionSerializer.requestSendingLog(log.type, log.body); } - Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idle(); - while (customLogTransmitter.getPendingCount() > 0) { - Shadows.shadowOf(customLogTransmitter.getLooper()).idleFor(100, TimeUnit.MILLISECONDS); + while (customLogInstructionSerializer.getPendingCount() > 0) { + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idleFor(100, TimeUnit.MILLISECONDS); } - Truth.assertThat(customLogTransmitter.getPendingCount()).isEqualTo(0); - Mockito.verify(service, times((CustomLogTransmitter.MAX_RETRY_COUNT + 1) * 10)).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), any()); + Truth.assertThat(customLogInstructionSerializer.getPendingCount()).isEqualTo(0); + Mockito.verify(service, times((CustomLogInstructionSerializer.MAX_RETRY_COUNT + 1) * 10)).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), any()); } @Test(timeout = 3000L) - public void transmit_works_as_expected_with_retry_barrier() throws RemoteException { + public void requestSendingLog_works_as_expected_with_retry_barrier() throws RemoteException { CustomLogConfiguration configuration = new CustomLogConfiguration.Builder().build(); - CustomLogTransmitter customLogTransmitter = new CustomLogTransmitter(PACKAGE_NAME, configuration); + CustomLogInstructionSerializer customLogInstructionSerializer = new CustomLogInstructionSerializer(PACKAGE_NAME, configuration); CustomLog noIssue = new CustomLog("type", "noIssue"); CustomLog successAfterRetries = new CustomLog("type", "successAfterRetries"); @@ -215,13 +232,13 @@ public void transmit_works_as_expected_with_retry_barrier() throws RemoteExcepti doThrow(TransactionTooLargeException.class, DeadObjectException.class).doCallRealMethod().when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(successAfterRetries.toExtras())); doThrow(RemoteException.class).when(service).sendEvent(eq(PACKAGE_NAME), eq(DeployGateEvent.ACTION_SEND_CUSTOM_LOG), eq(retryExceeded.toExtras())); - customLogTransmitter.connect(service); + customLogInstructionSerializer.connect(service); - customLogTransmitter.transmit(successAfterRetries.type, successAfterRetries.body); - customLogTransmitter.transmit(noIssue.type, noIssue.body); - customLogTransmitter.transmit(retryExceeded.type, retryExceeded.body); + customLogInstructionSerializer.requestSendingLog(successAfterRetries.type, successAfterRetries.body); + customLogInstructionSerializer.requestSendingLog(noIssue.type, noIssue.body); + customLogInstructionSerializer.requestSendingLog(retryExceeded.type, retryExceeded.body); - Shadows.shadowOf(customLogTransmitter.getLooper()).idle(); + Shadows.shadowOf(customLogInstructionSerializer.getLooper()).idle(); List extras = service.getEventExtraList(DeployGateEvent.ACTION_SEND_CUSTOM_LOG); From 64ea2291cb202226423d21078ecbda5b851680bf Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Tue, 12 Apr 2022 10:08:12 +0900 Subject: [PATCH 16/18] refactor: remove parcelable and eqiality impl from CustomLog it's not required for now --- .../java/com/deploygate/sdk/CustomLog.java | 61 +------------------ 1 file changed, 1 insertion(+), 60 deletions(-) diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLog.java b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java index a0d6bfc..d8313ad 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLog.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLog.java @@ -1,12 +1,10 @@ package com.deploygate.sdk; import android.os.Bundle; -import android.os.Parcel; -import android.os.Parcelable; import com.deploygate.service.DeployGateEvent; -class CustomLog implements Parcelable { +class CustomLog { public final String type; public final String body; private int retryCount; @@ -20,12 +18,6 @@ class CustomLog implements Parcelable { this.retryCount = 0; } - protected CustomLog(Parcel in) { - type = in.readString(); - body = in.readString(); - retryCount = in.readInt(); - } - /** * @return the number of current attempts */ @@ -42,55 +34,4 @@ Bundle toExtras() { extras.putSerializable(DeployGateEvent.EXTRA_LOG_TYPE, type); return extras; } - - public static final Creator CREATOR = new Creator() { - @Override - public CustomLog createFromParcel(Parcel in) { - return new CustomLog(in); - } - - @Override - public CustomLog[] newArray(int size) { - return new CustomLog[size]; - } - }; - - @Override - public int describeContents() { - return 0; - } - - @Override - public void writeToParcel( - Parcel dest, - int flags - ) { - dest.writeString(type); - dest.writeString(body); - dest.writeInt(retryCount); - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - - CustomLog customLog = (CustomLog) o; - - if (!type.equals(customLog.type)) { - return false; - } - return body.equals(customLog.body); - } - - @Override - public int hashCode() { - int result = type.hashCode(); - result = 31 * result + body.hashCode(); - return result; - } } From db3677725fc0ad765dbd73a302161afac6e52a26 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Tue, 12 Apr 2022 11:39:02 +0900 Subject: [PATCH 17/18] chore: exclude test tasks from build job --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 2d02ad3..ab3d7ab 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -36,7 +36,7 @@ jobs: steps: - checkout - restore_gradle_cache - - run: ./gradlew sdk:build sdkMock:build --continue + - run: ./gradlew sdk:build sdkMock:build -x sdk:test -x sdkMock:test --continue - save_gradle_cache test: executor: android From 21eca166e6873720e0691543cea3f25ad91ac088 Mon Sep 17 00:00:00 2001 From: Jumpei Matsuda Date: Tue, 12 Apr 2022 14:19:23 +0900 Subject: [PATCH 18/18] refactor: Remove push what offset and tweaked a typo --- .../sdk/CustomLogInstructionSerializer.java | 27 ++++--------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/sdk/src/main/java/com/deploygate/sdk/CustomLogInstructionSerializer.java b/sdk/src/main/java/com/deploygate/sdk/CustomLogInstructionSerializer.java index c754833..76455dd 100644 --- a/sdk/src/main/java/com/deploygate/sdk/CustomLogInstructionSerializer.java +++ b/sdk/src/main/java/com/deploygate/sdk/CustomLogInstructionSerializer.java @@ -15,7 +15,7 @@ /** * This class serialize the instructions for custom logs and process them in another thread in order of enqueued. - * The internal handler class creates a buffer pool and grantee the order. + * The internal handler class creates a buffer pool and guarantee the order. * *

* - Enqueue a new log to the buffer pool @@ -220,9 +220,7 @@ private static class CustomLogHandler extends Handler { private final CustomLogInstructionSerializer serializer; private final CustomLogConfiguration.Backpressure backpressure; private final int bufferSize; - private final int maxWhatOffset; private final LinkedList customLogs; - private int pushWhatOffset = 0; /** * @param looper @@ -244,7 +242,6 @@ private static class CustomLogHandler extends Handler { this.serializer = serializer; this.backpressure = backpressure; this.bufferSize = bufferSize; - this.maxWhatOffset = bufferSize * 2; this.customLogs = new LinkedList<>(); } @@ -271,7 +268,7 @@ void enqueueSendLogsInstruction() { * Enqueue new add-new-log instruction to the handler message queue. */ void enqueueAddNewLogInstruction(CustomLog log) { - Message msg = obtainMessage(WHAT_ADD_NEW_LOG + getAndIncrementPushWhatOffset(), log); + Message msg = obtainMessage(WHAT_ADD_NEW_LOG, log); sendMessage(msg); } @@ -342,27 +339,13 @@ public void handleMessage(Message msg) { break; } - default: { - if (msg.what >= WHAT_ADD_NEW_LOG) { - CustomLog log = (CustomLog) msg.obj; - addLogToLast(log); - } + case WHAT_ADD_NEW_LOG: { + CustomLog log = (CustomLog) msg.obj; + addLogToLast(log); break; } } } - - /** - * To avoid WHAT number conflicts, sdk uses a simple counter. - * - * @return positive number, which is greater than or equal to the value of {@link #WHAT_ADD_NEW_LOG} - */ - private int getAndIncrementPushWhatOffset() { - if (pushWhatOffset > maxWhatOffset) { - pushWhatOffset = 0; - } - return pushWhatOffset++; - } } }