Merge pull request #63 from pettermahlen/composite-logger-symmetry

ensure that CompositeLogger uses LIFO order for 'after's

mobius-extras/build.gradle

@@ -19,6 +19,7 @@ dependencies {
     testImplementation "com.google.guava:guava:${versions.guava}"
     testImplementation "org.awaitility:awaitility:${versions.awaitility}"
     testImplementation "com.google.auto.value:auto-value:${versions.autoValue}"
+    testImplementation "org.assertj:assertj-core:${versions.assertjcore}"
     testImplementation "org.hamcrest:hamcrest-library:${versions.hamcrestLibrary}"
 }
 

mobius-extras/src/main/java/com/spotify/mobius/extras/CompositeLogger.java

@@ -25,6 +25,8 @@
 import com.spotify.mobius.MobiusLoop.Logger;
 import com.spotify.mobius.Next;
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 
 /**
@@ -49,9 +51,12 @@ public static <M, E, F> Logger<M, E, F> from(Logger<M, E, F> logger, Logger<M, E
   }
 
   private final List<Logger<M, E, F>> loggers;
+  private final List<Logger<M, E, F>> loggersReversed;
 
   private CompositeLogger(List<Logger<M, E, F>> loggers) {
     this.loggers = loggers;
+    this.loggersReversed = new LinkedList<>(loggers);
+    Collections.reverse(loggersReversed);
   }
 
   @Override
@@ -63,14 +68,14 @@ public void beforeInit(M model) {
 
   @Override
   public void afterInit(M model, First<M, F> result) {
-    for (Logger<M, E, F> logger : loggers) {
+    for (Logger<M, E, F> logger : loggersReversed) {
       logger.afterInit(model, result);
     }
   }
 
   @Override
   public void exceptionDuringInit(M model, Throwable exception) {
-    for (Logger<M, E, F> logger : loggers) {
+    for (Logger<M, E, F> logger : loggersReversed) {
       logger.exceptionDuringInit(model, exception);
     }
   }
@@ -84,14 +89,14 @@ public void beforeUpdate(M model, E event) {
 
   @Override
   public void afterUpdate(M model, E event, Next<M, F> result) {
-    for (Logger<M, E, F> logger : loggers) {
+    for (Logger<M, E, F> logger : loggersReversed) {
       logger.afterUpdate(model, event, result);
     }
   }
 
   @Override
   public void exceptionDuringUpdate(M model, E event, Throwable exception) {
-    for (Logger<M, E, F> logger : loggers) {
+    for (Logger<M, E, F> logger : loggersReversed) {
       logger.exceptionDuringUpdate(model, event, exception);
     }
   }

mobius-extras/src/test/java/com/spotify/mobius/extras/CompositeLoggerTest.java

@@ -21,16 +21,16 @@
 
 import static com.spotify.mobius.Next.next;
 import static java.util.Collections.singleton;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import com.google.auto.value.AutoValue;
 import com.spotify.mobius.First;
 import com.spotify.mobius.MobiusLoop;
+import com.spotify.mobius.MobiusLoop.Logger;
 import com.spotify.mobius.Next;
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
 import org.junit.Before;
 import org.junit.Test;
@@ -41,13 +41,19 @@ public class CompositeLoggerTest {
   private RecordingLogger<String, Integer, String> logger2;
   private RecordingLogger<String, Integer, String> logger3;
   private MobiusLoop.Logger<String, Integer, String> underTest;
+  private List<String> logEntries;
+  private TaggingLogger<String, Integer, String> taggingLogger1;
+  private TaggingLogger<String, Integer, String> taggingLogger2;
 
   @Before
   public void setUp() {
     logger1 = new RecordingLogger<>();
     logger2 = new RecordingLogger<>();
     logger3 = new RecordingLogger<>();
     underTest = CompositeLogger.from(logger1, logger2, logger3);
+    logEntries = Collections.synchronizedList(new LinkedList<>());
+    taggingLogger1 = new TaggingLogger<>("1", logEntries);
+    taggingLogger2 = new TaggingLogger<>("2", logEntries);
   }
 
   @Test
@@ -97,6 +103,60 @@ public void delegatesExceptionDuringUpdateToAllLoggers()
     assertTestCaseLogged(testCase);
   }
 
+  @Test
+  public void callsLoggersInFifoOrderForBeforeInit() throws Exception {
+    underTest = CompositeLogger.from(taggingLogger1, taggingLogger2);
+
+    underTest.beforeInit("moo");
+
+    assertThat(logEntries).containsExactly("1: beforeInit", "2: beforeInit");
+  }
+
+  @Test
+  public void callsLoggersInLifoOrderForAfterInit() throws Exception {
+    underTest = CompositeLogger.from(taggingLogger1, taggingLogger2);
+
+    underTest.afterInit("moo", First.first("!!"));
+
+    assertThat(logEntries).containsExactly("2: afterInit", "1: afterInit");
+  }
+
+  @Test
+  public void callsLoggersInLifoOrderForExceptionDuringInit() throws Exception {
+    underTest = CompositeLogger.from(taggingLogger1, taggingLogger2);
+
+    underTest.exceptionDuringInit("moo", new RuntimeException("bark"));
+
+    assertThat(logEntries).containsExactly("2: exceptionDuringInit", "1: exceptionDuringInit");
+  }
+
+  @Test
+  public void callsLoggersInFifoOrderForBeforeUpdate() throws Exception {
+    underTest = CompositeLogger.from(taggingLogger1, taggingLogger2);
+
+    underTest.beforeUpdate("moo", 1);
+
+    assertThat(logEntries).containsExactly("1: beforeUpdate", "2: beforeUpdate");
+  }
+
+  @Test
+  public void callsLoggersInLifoOrderForAfterUpdate() throws Exception {
+    underTest = CompositeLogger.from(taggingLogger1, taggingLogger2);
+
+    underTest.afterUpdate("moo", 1, Next.next("!!"));
+
+    assertThat(logEntries).containsExactly("2: afterUpdate", "1: afterUpdate");
+  }
+
+  @Test
+  public void callsLoggersInLifoOrderForExceptionDuringUpdate() throws Exception {
+    underTest = CompositeLogger.from(taggingLogger1, taggingLogger2);
+
+    underTest.exceptionDuringUpdate("moo", 1, new RuntimeException("bark"));
+
+    assertThat(logEntries).containsExactly("2: exceptionDuringUpdate", "1: exceptionDuringUpdate");
+  }
+
   private void assertTestCaseLogged(LogEvent testCase) {
     logger1.assertLogEvents(testCase);
     logger2.assertLogEvents(testCase);
@@ -107,8 +167,8 @@ private static class RecordingLogger<M, E, F> implements MobiusLoop.Logger<M, E,
 
     private final List<LogEvent> events = new ArrayList<>();
 
-    public void assertLogEvents(LogEvent... events) {
-      assertThat(this.events, is(equalTo(Arrays.asList(events))));
+    void assertLogEvents(LogEvent... events) {
+      assertThat(this.events).containsExactly(events);
     }
 
     @Override
@@ -142,6 +202,46 @@ public void exceptionDuringUpdate(M model, E event, Throwable exception) {
     }
   }
 
+  private static class TaggingLogger<M, E, F> implements MobiusLoop.Logger<M, E, F> {
+    private final String tag;
+    private final List<String> logEntries;
+
+    private TaggingLogger(String tag, List<String> logEntries) {
+      this.tag = tag;
+      this.logEntries = logEntries;
+    }
+
+    @Override
+    public void beforeInit(M model) {
+      logEntries.add(String.format("%s: beforeInit", tag));
+    }
+
+    @Override
+    public void afterInit(M model, First<M, F> result) {
+      logEntries.add(String.format("%s: afterInit", tag));
+    }
+
+    @Override
+    public void exceptionDuringInit(M model, Throwable exception) {
+      logEntries.add(String.format("%s: exceptionDuringInit", tag));
+    }
+
+    @Override
+    public void beforeUpdate(M model, E event) {
+      logEntries.add(String.format("%s: beforeUpdate", tag));
+    }
+
+    @Override
+    public void afterUpdate(M model, E event, Next<M, F> result) {
+      logEntries.add(String.format("%s: afterUpdate", tag));
+    }
+
+    @Override
+    public void exceptionDuringUpdate(M model, E event, Throwable exception) {
+      logEntries.add(String.format("%s: exceptionDuringUpdate", tag));
+    }
+  }
+
   private interface LogEvent {}
 
   @AutoValue