Merge pull request #284 from scireum/fwe/event-bug-fixes

Fix two small bugs in EventRecorder

src/main/java/sirius/biz/analytics/events/EventRecorder.java

@@ -134,13 +134,14 @@ public void record(@Nonnull Event event) {
             return;
         }
 
-        if (bufferedEvents.incrementAndGet() > MAX_BUFFER_SIZE) {
+        if (bufferedEvents.get() >= MAX_BUFFER_SIZE) {
             return;
         }
 
         try {
             event.getDescriptor().beforeSave(event);
             buffer.offer(event);
+            bufferedEvents.incrementAndGet();
         } catch (HandledException e) {
             Exceptions.ignore(e);
         } catch (Exception e) {
@@ -176,14 +177,14 @@ protected int process() {
         int processedEvents = 0;
         try (BatchContext ctx = new BatchContext(() -> "Process recorded events.", Duration.ofMinutes(1))) {
             Map<Class<? extends Event>, InsertQuery<Event>> queries = new HashMap<>();
-            Event nextEvent = buffer.poll();
+            Event nextEvent = fetchBufferedEvent();
             while (nextEvent != null) {
                 processEvent(ctx, queries, nextEvent);
                 if (++processedEvents >= MAX_EVENTS_PER_PROCESS) {
                     return processedEvents;
                 }
 
-                nextEvent = buffer.poll();
+                nextEvent = fetchBufferedEvent();
             }
         } catch (HandledException e) {
             Exceptions.ignore(e);

src/test/java/sirius/biz/analytics/events/EventRecorderSpec.groovy

@@ -39,4 +39,45 @@ class EventRecorderSpec extends BaseSpecification {
         dbs.get("clickhouse").createQuery("SELECT * FROM testevent2").queryList().size() == 4
     }
 
+    def "bufferedEvents-field is reset after processing"() {
+        when:
+        recorder.record(new TestEvent1())
+        recorder.record(new TestEvent1())
+        recorder.record(new TestEvent1())
+        recorder.record(new TestEvent1())
+        and:
+        recorder.record(new TestEvent2())
+        recorder.record(new TestEvent2())
+        recorder.record(new TestEvent2())
+        recorder.record(new TestEvent2())
+        then:
+        recorder.bufferedEvents.get() == 8
+        and:
+        recorder.process() == 8
+        and:
+        recorder.bufferedEvents.get() == 0
+    }
+
+    def "bufferedEvents-field does not get larger as MAX_BUFFER_SIZE"() {
+        when:
+        for (int i = 0; i < EventRecorder.MAX_BUFFER_SIZE + 20; i++) {
+            recorder.record(new TestEvent1())
+        }
+        then:
+        recorder.bufferedEvents.get() == EventRecorder.MAX_BUFFER_SIZE
+        and:
+        recorder.process()
+        then:
+        recorder.bufferedEvents.get() == 0
+    }
+
+    def "bufferedEvents-field is not incremented if the Event throws exception on save"() {
+        when:
+        recorder.record(new TestEvent3ThrowsExceptionOnSave())
+        then:
+        recorder.buffer.poll() == null
+        recorder.bufferedEvents.get() == 0
+        and:
+        recorder.process() == 0
+    }
 }

src/test/java/sirius/biz/analytics/events/TestEvent3ThrowsExceptionOnSave.java

@@ -0,0 +1,20 @@
+package sirius.biz.analytics.events;
+
+import sirius.db.mixing.annotations.BeforeSave;
+import sirius.kernel.health.Exceptions;
+
+import java.io.UncheckedIOException;
+
+public class TestEvent3ThrowsExceptionOnSave extends Event {
+
+    private final UserData user = new UserData();
+
+    public UserData getUser() {
+        return user;
+    }
+
+    @BeforeSave
+    public void throwErrorOnSave() {
+        throw Exceptions.createHandled().handle();
+    }
+}