Merge pull request ably#599 from ably/feature/598-discard-persisted-events

Discard persisted events with non-nullary constructors

Author: tcard

android/src/androidTest/java/io/ably/lib/test/android/AndroidPushTest.java

@@ -1385,6 +1385,33 @@ public Void apply(TestActivation.Options options) throws AblyException {
 		assertInstanceOf(AfterRegistrationSyncFailed.class, activation.machine.current);
 	}
 
+	// https://github.com/ably/ably-java/issues/598
+	public void test_restore_non_nullary_event() {
+		TestActivation activation = new TestActivation();
+		assertInstanceOf(NotActivated.class, activation.machine.current);
+
+		SyncRegistrationFailed event = new SyncRegistrationFailed(new ErrorInfo());
+
+		activation.machine.handleEvent(event);
+
+		// NotActivated can't handle SyncRegistrationFailed, so it should be pending.
+		assertTrue(activation.machine.pendingEvents.contains(event));
+
+		// Now recover the persisted state and events.
+
+		activation = new TestActivation(new Helpers.AblyFunction<TestActivation.Options, Void>() {
+			@Override
+			public Void apply(TestActivation.Options options) throws AblyException {
+				options.clearPersisted = false;
+				return null;
+			}
+		});
+
+		// Since the event doesn't have a nullary constructor, it should be dropped.
+		assertInstanceOf(NotActivated.class, activation.machine.current);
+		assertSize(0, activation.machine.pendingEvents);
+	}
+
 	// This is all copied and pasted from ParameterizedTest, since I can't inherit from it.
 	// I need to inherit from AndroidPushTest, and Java doesn't have multiple inheritance
 	// or mixins or something like that.

android/src/main/java/io/ably/lib/push/ActivationStateMachine.java

@@ -573,6 +573,9 @@ public synchronized boolean handleEvent(ActivationStateMachine.Event event) {
 			// An event's side effects may end up synchronously calling handleEvent while it's
 			// itself being handled. In that case, enqueue it so it's handled next (and still
 			// synchronously).
+			//
+			// We don't need to persist here, as the handleEvent call up the stack will eventually
+			// persist when done with the synchronous transitions.
 			enqueueEvent(event);
 			return true;
 		}
@@ -584,7 +587,7 @@ public synchronized boolean handleEvent(ActivationStateMachine.Event event) {
 			ActivationStateMachine.State maybeNext = current.transition(event);
 			if (maybeNext == null) {
 				enqueueEvent(event);
-				return true;
+				return persist();
 			}
 
 			Log.d(TAG, String.format("transition: %s -(%s)-> %s", current.getClass().getSimpleName(), event.getClass().getSimpleName(), maybeNext.getClass().getSimpleName()));
@@ -675,7 +678,18 @@ private ArrayDeque<ActivationStateMachine.Event> getPersistedPendingEvents() {
 		for (int i = 0; i < length; i++) {
 			try {
 				String className = activationContext.getPreferences().getString(String.format("%s[%d]", ActivationStateMachine.PersistKeys.PENDING_EVENTS_PREFIX, i), "");
-				ActivationStateMachine.Event event = ((Class<ActivationStateMachine.Event>) Class.forName(className)).newInstance();
+				Class<ActivationStateMachine.Event> eventClass = (Class<ActivationStateMachine.Event>) Class.forName(className);
+				ActivationStateMachine.Event event;
+				try {
+					event = eventClass.newInstance();
+				} catch(InstantiationException e) {
+					// We aren't properly persisting events with a non-nullary constructor. Those events
+					// are supposed to be handled by states that aren't persisted (until
+					// https://github.com/ably/ably-java/issues/546 is fixed), so it should be safe to
+					// just drop them.
+					Log.d(TAG, String.format("discarding improperly persisted event: %s", className));
+					continue;
+				}
 				deque.add(event);
 			} catch(Exception e) {
 				throw new RuntimeException(e);