BoxStore: route all native handle access through method with open check.

Author: greenrobot-team

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

@@ -574,7 +574,7 @@ public long panicModeRemoveAll() {
      * Returns a builder to create queries for Object matching supplied criteria.
      */
     public QueryBuilder<T> query() {
-        return new QueryBuilder<>(this, store.internalHandle(), store.getDbName(entityClass));
+        return new QueryBuilder<>(this, store.getNativeStore(), store.getDbName(entityClass));
     }
 
     /**

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

@@ -220,6 +220,7 @@ public static boolean isSyncServerAvailable() {
 
     private final File directory;
     private final String canonicalPath;
+    /** Reference to the native store. Should probably get through {@link #getNativeStore()} instead. */
     private final long handle;
     private final Map<Class<?>, String> dbNameByClass = new HashMap<>();
     private final Map<Class<?>, Integer> entityTypeIdByClass = new HashMap<>();
@@ -467,11 +468,12 @@ public static long sysProcStatusKb(String key) {
      * @return 0 if the size could not be determined (does not throw unless this store was already closed)
      */
     public long sizeOnDisk() {
-        checkOpen();
-        return nativeSizeOnDisk(handle);
+        return nativeSizeOnDisk(getNativeStore());
     }
 
     /**
+     * Closes this if this is finalized.
+     * <p>
      * Explicitly call {@link #close()} instead to avoid expensive finalization.
      */
     @SuppressWarnings("deprecation") // finalize()
@@ -481,8 +483,11 @@ protected void finalize() throws Throwable {
         super.finalize();
     }
 
+    /**
+     * Verifies this has not been {@link #close() closed}.
+     */
     private void checkOpen() {
-        if (closed) {
+        if (isClosed()) {
             throw new IllegalStateException("Store is closed");
         }
     }
@@ -533,13 +538,12 @@ <T> EntityInfo<T> getEntityInfo(Class<T> entityClass) {
      */
     @Internal
     public Transaction beginTx() {
-        checkOpen();
         // Because write TXs are typically not cached, initialCommitCount is not as relevant than for read TXs.
         int initialCommitCount = commitCount;
         if (debugTxWrite) {
             System.out.println("Begin TX with commit count " + initialCommitCount);
         }
-        long nativeTx = nativeBeginTx(handle);
+        long nativeTx = nativeBeginTx(getNativeStore());
         if (nativeTx == 0) throw new DbException("Could not create native transaction");
 
         Transaction tx = new Transaction(this, nativeTx, initialCommitCount);
@@ -555,7 +559,6 @@ public Transaction beginTx() {
      */
     @Internal
     public Transaction beginReadTx() {
-        checkOpen();
         // initialCommitCount should be acquired before starting the tx. In race conditions, there is a chance the
         // commitCount is already outdated. That's OK because it only gives a false positive for an TX being obsolete.
         // In contrast, a false negative would make a TX falsely not considered obsolete, and thus readers would not be
@@ -565,7 +568,7 @@ public Transaction beginReadTx() {
         if (debugTxRead) {
             System.out.println("Begin read TX with commit count " + initialCommitCount);
         }
-        long nativeTx = nativeBeginReadTx(handle);
+        long nativeTx = nativeBeginReadTx(getNativeStore());
         if (nativeTx == 0) throw new DbException("Could not create native read transaction");
 
         Transaction tx = new Transaction(this, nativeTx, initialCommitCount);
@@ -575,6 +578,9 @@ public Transaction beginReadTx() {
         return tx;
     }
 
+    /**
+     * If this was {@link #close() closed}.
+     */
     public boolean isClosed() {
         return closed;
     }
@@ -584,8 +590,7 @@ public boolean isClosed() {
      * If true the schema is not updated and write transactions are not possible.
      */
     public boolean isReadOnly() {
-        checkOpen();
-        return nativeIsReadOnly(handle);
+        return nativeIsReadOnly(getNativeStore());
     }
 
     /**
@@ -665,7 +670,7 @@ private void checkThreadTermination() {
      * Note: If false is returned, any number of files may have been deleted before the failure happened.
      */
     public boolean deleteAllFiles() {
-        if (!closed) {
+        if (!isClosed()) {
             throw new IllegalStateException("Store must be closed");
         }
         return deleteAllFiles(directory);
@@ -765,8 +770,7 @@ public static boolean deleteAllFiles(@Nullable File baseDirectoryOrNull, @Nullab
      * </ul>
      */
     public void removeAllObjects() {
-        checkOpen();
-        nativeDropAllData(handle);
+        nativeDropAllData(getNativeStore());
     }
 
     @Internal
@@ -1049,8 +1053,7 @@ public <R> void callInTxAsync(final Callable<R> callable, @Nullable final TxCall
      * @return String that is typically logged by the application.
      */
     public String diagnose() {
-        checkOpen();
-        return nativeDiagnose(handle);
+        return nativeDiagnose(getNativeStore());
     }
 
     /**
@@ -1069,13 +1072,11 @@ public long validate(long pageLimit, boolean checkLeafLevel) {
         if (pageLimit < 0) {
             throw new IllegalArgumentException("pageLimit must be zero or positive");
         }
-        checkOpen();
-        return nativeValidate(handle, pageLimit, checkLeafLevel);
+        return nativeValidate(getNativeStore(), pageLimit, checkLeafLevel);
     }
 
     public int cleanStaleReadTransactions() {
-        checkOpen();
-        return nativeCleanStaleReadTransactions(handle);
+        return nativeCleanStaleReadTransactions(getNativeStore());
     }
 
     /**
@@ -1090,11 +1091,6 @@ public void closeThreadResources() {
         // activeTx is cleaned up in finally blocks, so do not free them here
     }
 
-    @Internal
-    long internalHandle() {
-        return handle;
-    }
-
     /**
      * A {@link io.objectbox.reactive.DataObserver} can be subscribed to data changes using the returned builder.
      * The observer is supplied via {@link SubscriptionBuilder#observer(DataObserver)} and will be notified once a
@@ -1146,8 +1142,7 @@ public String startObjectBrowser() {
     @Nullable
     public String startObjectBrowser(int port) {
         verifyObjectBrowserNotRunning();
-        checkOpen();
-        String url = nativeStartObjectBrowser(handle, null, port);
+        String url = nativeStartObjectBrowser(getNativeStore(), null, port);
         if (url != null) {
             objectBrowserPort = port;
         }
@@ -1158,14 +1153,13 @@ public String startObjectBrowser(int port) {
     @Nullable
     public String startObjectBrowser(String urlToBindTo) {
         verifyObjectBrowserNotRunning();
-        checkOpen();
         int port;
         try {
             port = new URL(urlToBindTo).getPort(); // Gives -1 if not available
         } catch (MalformedURLException e) {
             throw new RuntimeException("Can not start Object Browser at " + urlToBindTo, e);
         }
-        String url = nativeStartObjectBrowser(handle, urlToBindTo, 0);
+        String url = nativeStartObjectBrowser(getNativeStore(), urlToBindTo, 0);
         if (url != null) {
             objectBrowserPort = port;
         }
@@ -1178,8 +1172,7 @@ public synchronized boolean stopObjectBrowser() {
             throw new IllegalStateException("ObjectBrowser has not been started before");
         }
         objectBrowserPort = 0;
-        checkOpen();
-        return nativeStopObjectBrowser(handle);
+        return nativeStopObjectBrowser(getNativeStore());
     }
 
     @Experimental
@@ -1204,8 +1197,7 @@ private void verifyObjectBrowserNotRunning() {
      * This for example allows central error handling or special logging for database-related exceptions.
      */
     public void setDbExceptionListener(@Nullable DbExceptionListener dbExceptionListener) {
-        checkOpen();
-        nativeSetDbExceptionListener(handle, dbExceptionListener);
+        nativeSetDbExceptionListener(getNativeStore(), dbExceptionListener);
     }
 
     @Internal
@@ -1234,18 +1226,19 @@ public TxCallback<?> internalFailedReadTxAttemptCallback() {
     }
 
     void setDebugFlags(int debugFlags) {
-        checkOpen();
-        nativeSetDebugFlags(handle, debugFlags);
+        nativeSetDebugFlags(getNativeStore(), debugFlags);
     }
 
     long panicModeRemoveAllObjects(int entityId) {
-        checkOpen();
-        return nativePanicModeRemoveAllObjects(handle, entityId);
+        return nativePanicModeRemoveAllObjects(getNativeStore(), entityId);
     }
 
     /**
-     * If you want to use the same ObjectBox store using the C API, e.g. via JNI, this gives the required pointer,
-     * which you have to pass on to obx_store_wrap().
+     * Gets the reference to the native store. Can be used with the C API to use the same store, e.g. via JNI, by
+     * passing it on to {@code obx_store_wrap()}.
+     * <p>
+     * Throws if the store is closed.
+     * <p>
      * The procedure is like this:<br>
      * 1) you create a BoxStore on the Java side<br>
      * 2) you call this method to get the native store pointer<br>

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

@@ -24,10 +24,6 @@
 @Internal
 public class InternalAccess {
 
-    public static long getHandle(BoxStore boxStore) {
-        return boxStore.internalHandle();
-    }
-
     public static Transaction getActiveTx(BoxStore boxStore) {
         Transaction tx = boxStore.activeTx.get();
         if (tx == null) {

objectbox-java/src/main/java/io/objectbox/sync/SyncClientImpl.java

@@ -48,7 +48,7 @@ public class SyncClientImpl implements SyncClient {
         this.serverUrl = builder.url;
         this.connectivityMonitor = builder.platform.getConnectivityMonitor();
 
-        long boxStoreHandle = InternalAccess.getHandle(builder.boxStore);
+        long boxStoreHandle = builder.boxStore.getNativeStore();
         long handle = nativeCreate(boxStoreHandle, serverUrl, builder.trustedCertPaths);
         if (handle == 0) {
             throw new RuntimeException("Failed to create sync client: handle is zero.");

objectbox-java/src/main/java/io/objectbox/sync/server/SyncServerImpl.java

@@ -2,7 +2,6 @@
 
 import javax.annotation.Nullable;
 
-import io.objectbox.InternalAccess;
 import io.objectbox.annotation.apihint.Internal;
 import io.objectbox.sync.SyncCredentials;
 import io.objectbox.sync.SyncCredentialsToken;
@@ -24,7 +23,7 @@ public class SyncServerImpl implements SyncServer {
     SyncServerImpl(SyncServerBuilder builder) {
         this.url = builder.url;
 
-        long storeHandle = InternalAccess.getHandle(builder.boxStore);
+        long storeHandle = builder.boxStore.getNativeStore();
         long handle = nativeCreate(storeHandle, url, builder.certificatePath);
         if (handle == 0) {
             throw new RuntimeException("Failed to create sync server: handle is zero.");

tests/objectbox-java-test/src/test/java/io/objectbox/BoxStoreTest.java

@@ -136,7 +136,7 @@ public void testClose() {
         assertThrowsStoreIsClosed(() -> store.subscribe(TestEntity.class));
         assertThrowsStoreIsClosed(store::startObjectBrowser);
         assertThrowsStoreIsClosed(() -> store.startObjectBrowser(12345));
-        assertThrowsStoreIsClosed(() -> store.startObjectBrowser(""));
+        assertThrowsStoreIsClosed(() -> store.startObjectBrowser("http://127.0.0.1"));
         // assertThrowsStoreIsClosed(store::stopObjectBrowser); // Requires mocking, not testing for now.
         assertThrowsStoreIsClosed(() -> store.setDbExceptionListener(null));
         // Internal thread pool is shut down as part of closing store, should no longer accept new work.

tests/objectbox-java-test/src/test/java/io/objectbox/query/QueryTest.java

@@ -62,9 +62,11 @@ public class QueryTest extends AbstractQueryTest {
     public void createIfStoreClosed_throws() {
         store.close();
 
-        IllegalStateException ex = assertThrows(IllegalStateException.class, () -> box.query());
-        // FIXME Replace with actual error message
-        assertEquals("No schema set on store", ex.getMessage());
+        IllegalStateException ex = assertThrows(
+                IllegalStateException.class,
+                () -> box.query()
+        );
+        assertEquals("Store is closed", ex.getMessage());
     }
 
     @Test