Merge branch '82-null-db-exception-listener' into 'dev'

BoxStore: do not warn on (Java)/allow null (Kotlin) DbExceptionListener.

See merge request objectbox/objectbox-java!73

Author: greenrobot-team

objectbox-java/src/main/java/io/objectbox/BoxStore.java

@@ -16,7 +16,6 @@
 
 package io.objectbox;
 
-import io.objectbox.annotation.apihint.Beta;
 import org.greenrobot.essentials.collections.LongHashMap;
 
 import java.io.Closeable;
@@ -40,6 +39,7 @@
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.ThreadSafe;
 
+import io.objectbox.annotation.apihint.Beta;
 import io.objectbox.annotation.apihint.Experimental;
 import io.objectbox.annotation.apihint.Internal;
 import io.objectbox.converter.PropertyConverter;
@@ -165,7 +165,7 @@ static native void nativeRegisterCustomType(long store, int entityId, int proper
 
     static native int nativeCleanStaleReadTransactions(long store);
 
-    static native void nativeSetDbExceptionListener(long store, DbExceptionListener dbExceptionListener);
+    static native void nativeSetDbExceptionListener(long store, @Nullable DbExceptionListener dbExceptionListener);
 
     static native void nativeSetDebugFlags(long store, int debugFlags);
 
@@ -1027,10 +1027,12 @@ private void verifyObjectBrowserNotRunning() {
     }
 
     /**
-     * The given listener will be called when an exception is thrown.
-     * This for example allows a central error handling, e.g. a special logging for DB related exceptions.
+     * Sets a listener that will be called when an exception is thrown. Replaces a previously set listener.
+     * Set to {@code null} to remove the listener.
+     * <p>
+     * This for example allows central error handling or special logging for database-related exceptions.
      */
-    public void setDbExceptionListener(DbExceptionListener dbExceptionListener) {
+    public void setDbExceptionListener(@Nullable DbExceptionListener dbExceptionListener) {
         nativeSetDbExceptionListener(handle, dbExceptionListener);
     }
 

tests/objectbox-java-test/src/test/java/io/objectbox/exception/ExceptionTest.java

@@ -16,16 +16,136 @@
 
 package io.objectbox.exception;
 
-import io.objectbox.AbstractObjectBoxTest;
 import org.junit.Test;
 
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import io.objectbox.AbstractObjectBoxTest;
+
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
 
+/**
+ * Tests related to {@link DbExceptionListener}.
+ *
+ * Note: this test has an equivalent in test-java-android integration tests.
+ */
 public class ExceptionTest extends AbstractObjectBoxTest {
 
+    @Test
+    public void exceptionListener_null_works() {
+        store.setDbExceptionListener(null);
+    }
+
+    @Test
+    public void exceptionListener_closedStore_works() {
+        store.close();
+        store.setDbExceptionListener(e -> System.out.println("This is never called"));
+    }
+
+    private static boolean weakRefListenerCalled = false;
+
+    @Test
+    public void exceptionListener_noLocalRef_works() throws InterruptedException {
+        // Note: do not use lambda, it would keep a reference to this class
+        // and prevent garbage collection of the listener.
+        //noinspection Convert2Lambda
+        DbExceptionListener listenerNoRef = new DbExceptionListener() {
+            @Override
+            public void onDbException(Exception e) {
+                System.out.println("Listener without strong reference is called");
+                weakRefListenerCalled = true;
+            }
+        };
+        WeakReference<DbExceptionListener> weakReference = new WeakReference<>(listenerNoRef);
+
+        // Set and clear local reference.
+        store.setDbExceptionListener(listenerNoRef);
+        //noinspection UnusedAssignment
+        listenerNoRef = null;
+
+        // Try and fail to release weak reference.
+        int triesClearWeakRef = 5;
+        while (weakReference.get() != null) {
+            if (--triesClearWeakRef == 0) break;
+            System.out.println("Suggesting GC");
+            System.gc();
+            //noinspection BusyWait
+            Thread.sleep(300);
+        }
+        assertEquals("Failed to keep weak reference to listener", 0, triesClearWeakRef);
+
+        // Throw, listener should be called.
+        weakRefListenerCalled = false;
+        assertThrows(
+                DbException.class,
+                () -> DbExceptionListenerJni.nativeThrowException(store.getNativeStore(), 0)
+        );
+        assertTrue(weakRefListenerCalled);
+
+        // Remove reference from native side.
+        store.setDbExceptionListener(null);
+
+        // Try and succeed to release weak reference.
+        triesClearWeakRef = 5;
+        while (weakReference.get() != null) {
+            if (--triesClearWeakRef == 0) break;
+            System.out.println("Suggesting GC");
+            System.gc();
+            //noinspection BusyWait
+            Thread.sleep(300);
+        }
+        assertTrue("Failed to release weak reference to listener", triesClearWeakRef > 0);
+
+        // Throw, listener should not be called.
+        weakRefListenerCalled = false;
+        assertThrows(
+                DbException.class,
+                () -> DbExceptionListenerJni.nativeThrowException(store.getNativeStore(), 0)
+        );
+        assertFalse(weakRefListenerCalled);
+    }
+
+    @Test
+    public void exceptionListener_removing_works() {
+        AtomicBoolean replacedListenerCalled = new AtomicBoolean(false);
+        DbExceptionListener listenerRemoved = e -> replacedListenerCalled.set(true);
+
+        store.setDbExceptionListener(listenerRemoved);
+        store.setDbExceptionListener(null);
+
+        assertThrows(
+                DbException.class,
+                () -> DbExceptionListenerJni.nativeThrowException(store.getNativeStore(), 0)
+        );
+        assertFalse("Should not have called removed DbExceptionListener.", replacedListenerCalled.get());
+    }
+
+    @Test
+    public void exceptionListener_replacing_works() {
+        AtomicBoolean replacedListenerCalled = new AtomicBoolean(false);
+        DbExceptionListener listenerReplaced = e -> replacedListenerCalled.set(true);
+
+        AtomicBoolean newListenerCalled = new AtomicBoolean(false);
+        DbExceptionListener listenerNew = e -> newListenerCalled.set(true);
+
+        store.setDbExceptionListener(listenerReplaced);
+        store.setDbExceptionListener(listenerNew);
+
+        assertThrows(
+                DbException.class,
+                () -> DbExceptionListenerJni.nativeThrowException(store.getNativeStore(), 0)
+        );
+        assertFalse("Should not have called replaced DbExceptionListener.", replacedListenerCalled.get());
+        assertTrue("Failed to call new DbExceptionListener.", newListenerCalled.get());
+    }
+
     @Test
     public void testThrowExceptions() {
         final List<Exception> exs = new ArrayList<>();