Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBL-5684: Database.getDefaultCollection() should be non-null #286

Merged
merged 1 commit into from
Apr 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
CBL-5684: Database.getDefaultCollection() should be non-null
Co-authored-by: Jeff Lockhart <[email protected]>
  • Loading branch information
bmeike and jeffdgr8 committed Apr 26, 2024
commit a5f4a7206251560aeb6655b639c1fee8c47ae504
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ private CouchbaseLiteInternal() { }

private static final String LITECORE_JNI_LIBRARY = "LiteCoreJNI";

private static final Object LOCK = new Object();

private static final AtomicReference<SoftReference<Context>> CONTEXT = new AtomicReference<>();
private static final AtomicReference<ExecutionService> EXECUTION_SERVICE = new AtomicReference<>();
private static final AtomicReference<NetworkConnectivityManager> CONNECTIVITY_MANAGER = new AtomicReference<>();

private static final AtomicBoolean INITIALIZED = new AtomicBoolean(false);

private static final Object LOCK = new Object();

private static volatile boolean debugging;

private static volatile File defaultDbDir;
Expand Down Expand Up @@ -153,7 +153,16 @@ public static String getDefaultDbDirPath() {
}

@VisibleForTesting
public static void reset(boolean state) { INITIALIZED.set(state); }
public static void reset() {
debugging = false;
defaultDbDir = null;

CONTEXT.set(null);
EXECUTION_SERVICE.set(null);
CONNECTIVITY_MANAGER.set(null);

INITIALIZED.set(false);
}

@VisibleForTesting
@NonNull
Expand Down
50 changes: 18 additions & 32 deletions common/main/java/com/couchbase/lite/AbstractDatabase.java
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ static Database getDbForCollections(@Nullable Set<Collection> collections) {

@GuardedBy("dbLock")
private Collection defaultCollection;
@GuardedBy("dbLock")
private boolean noDefaultCollection;

private volatile CountDownLatch closeLatch;

Expand Down Expand Up @@ -484,11 +482,11 @@ public final Collection getCollection(@NonNull String collectionName, @Nullable
}

/**
* Get the default collection. If the default collection has been deleted the function will return null.
* Get the default collection.
*
* @return the default collection or null if it does not exist.
* @return the default collection.
*/
@Nullable
@NonNull
public final Collection getDefaultCollection() throws CouchbaseLiteException {
synchronized (getDbLock()) {
assertOpenChecked();
Expand All @@ -497,8 +495,8 @@ public final Collection getDefaultCollection() throws CouchbaseLiteException {
}

/**
* Delete a collection by name in the default scope. If the collection doesn't exist, the operation
* will be no-ops. Note: the default collection can be deleted but cannot be recreated.
* Delete a collection by name in the default scope. If the collection doesn't exist, the operation
* will do nothing. Note: the default collection cannot be deleted.
*
* @param name the collection to be deleted
* @throws CouchbaseLiteException on failure
Expand All @@ -508,8 +506,8 @@ public final void deleteCollection(@NonNull String name) throws CouchbaseLiteExc
}

/**
* Delete a collection by name in the specified scope. If the collection doesn't exist, the operation
* will be no-ops. Note: the default collection can be deleted but cannot be recreated.
* Delete a collection by name in the specified scope. If the collection doesn't exist, the operation
* will do nothing. Note: the default collection cannot be deleted.
*
* @param collectionName the collection to be deleted
* @param scopeName the scope from which to delete the collection
Expand Down Expand Up @@ -634,15 +632,10 @@ public boolean equals(Object o) {
*/
@Deprecated
public long getCount() {
final Collection defaultCollection;
try {
synchronized (getDbLock()) { defaultCollection = getDefaultCollectionLocked(); }
synchronized (getDbLock()) { return getDefaultCollectionLocked().getCount(); }
}
catch (CouchbaseLiteException e) {
throw new CouchbaseLiteError("Failed getting default collection", e);
}

return (defaultCollection == null) ? 0 : defaultCollection.getCount();
catch (CouchbaseLiteException e) { throw new CouchbaseLiteError("Failed getting default collection", e); }
}

/**
Expand Down Expand Up @@ -1024,9 +1017,6 @@ Set<Collection> getDefaultCollectionAsSet() {
final Collection defaultCollection;
try { defaultCollection = Collection.getDefaultCollection(this.getDatabase()); }
catch (CouchbaseLiteException e) { throw new CouchbaseLiteError("Can't get default collection", e); }
if (defaultCollection == null) {
throw new IllegalArgumentException("Database " + getName() + "has no default collection");
}
final HashSet<Collection> collections = new HashSet<>();
collections.add(defaultCollection);
return collections;
Expand All @@ -1044,7 +1034,7 @@ C4Collection getC4Collection(@NonNull String scopeName, @NonNull String collecti
synchronized (getDbLock()) { return getOpenC4DbLocked().getCollection(scopeName, collectionName); }
}

@Nullable
@NonNull
C4Collection getDefaultC4Collection() throws LiteCoreException {
synchronized (getDbLock()) { return getOpenC4DbLocked().getDefaultCollection(); }
}
Expand Down Expand Up @@ -1298,27 +1288,23 @@ FLEncoder getSharedFleeceEncoder() {

@NonNull
private Collection getDefaultCollectionOrThrow() {
Exception err = null;
try {
synchronized (getDbLock()) {
assertOpenUnchecked();
final Collection collection = getDefaultCollectionLocked();
if (collection != null) { return collection; }
return getDefaultCollectionLocked();
}
}
catch (CouchbaseLiteException e) { err = e; }

throw new CouchbaseLiteError(Log.lookupStandardMessage("DBClosedOrCollectionDeleted"), err);
catch (CouchbaseLiteException e) {
throw new CouchbaseLiteError(Log.lookupStandardMessage("DBClosedOrCollectionDeleted"), e);
}
}

@Nullable
@NonNull
private Collection getDefaultCollectionLocked() throws CouchbaseLiteException {
if (noDefaultCollection) { return null; }

if (defaultCollection == null) { defaultCollection = Collection.getDefaultCollection(getDatabase()); }
else if (!defaultCollection.isValid()) { defaultCollection = null; }

noDefaultCollection = defaultCollection == null;
else if (!defaultCollection.isValid()) {
throw new CouchbaseLiteException("Database " + getName() + " default collection is invalid");
}

return defaultCollection;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ protected static Map<Collection, CollectionConfiguration> configureDefaultCollec
throw new CouchbaseLiteError(Log.lookupStandardMessage("NoDefaultCollectionInConfig"), e);
}

if (defaultCollection == null) {
throw new CouchbaseLiteError(Log.lookupStandardMessage("NoDefaultCollectionInConfig"));
}

final Map<Collection, CollectionConfiguration> collections = new HashMap<>();
collections.put(defaultCollection, new CollectionConfiguration());

Expand Down
9 changes: 3 additions & 6 deletions common/main/java/com/couchbase/lite/Collection.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,9 @@ static Collection getCollection(
catch (LiteCoreException e) { throw CouchbaseLiteException.convertException(e); }
}

@Nullable
@NonNull
static Collection getDefaultCollection(@NonNull Database db) throws CouchbaseLiteException {
try {
final C4Collection c4Coll = db.getDefaultC4Collection();
return (c4Coll == null) ? null : new Collection(db, c4Coll);
}
try { return new Collection(db, db.getDefaultC4Collection()); }
catch (LiteCoreException e) { throw CouchbaseLiteException.convertException(e); }
}

Expand Down Expand Up @@ -429,7 +426,7 @@ public ListenerToken addDocumentChangeListener(
/**
* Get a list of the names of indices in the collection.
*
* @return the list of index names
* @return the set of index names
* @throws CouchbaseLiteException on failure
*/
@NonNull
Expand Down
2 changes: 1 addition & 1 deletion common/main/java/com/couchbase/lite/DataSource.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static As database(@NonNull Database database) {
Preconditions.assertNotNull(database, "database");

final Collection defaultCollection;
try { defaultCollection = Preconditions.assertNotNull(database.getDefaultCollection(), "default collection"); }
try { defaultCollection = database.getDefaultCollection(); }
catch (CouchbaseLiteException e) { throw new IllegalArgumentException("Database not open", e); }

final As source = new As(defaultCollection);
Expand Down
9 changes: 3 additions & 6 deletions common/main/java/com/couchbase/lite/DatabaseChange.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,10 @@
public class DatabaseChange {
@NonNull
private static Collection getDefaultCollection(@NonNull Database database) {
CouchbaseLiteException fail = null;
try {
final Collection defaultCollection = database.getDefaultCollection();
if (defaultCollection != null) { return defaultCollection; }
try { return database.getDefaultCollection(); }
catch (CouchbaseLiteException e) {
throw new CouchbaseLiteError("Failed retrieving default collection for database: " + database.getName(), e);
}
catch (CouchbaseLiteException e) { fail = e; }
throw new CouchbaseLiteError("Database " + database.getName() + " has no default collection", fail);
}

@NonNull
Expand Down
2 changes: 1 addition & 1 deletion common/main/java/com/couchbase/lite/ResultSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public void close() {
if (qEnum == null) { return; }

final AbstractDatabase db = context.getDatabase();
if (db == null) { throw new IllegalStateException("Could not obtain db lock"); }
if (db == null) { throw new CouchbaseLiteError("Could not obtain db lock"); }

synchronized (db.getDbLock()) { qEnum.close(); }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public static C4Collection get(@NonNull C4Database c4db, @NonNull String scope,
return get(NATIVE_IMPL, c4db, scope, collection);
}

@Nullable
@NonNull
public static C4Collection getDefault(@NonNull C4Database c4db) throws LiteCoreException {
return getDefault(NATIVE_IMPL, c4db);
}
Expand Down Expand Up @@ -126,12 +126,13 @@ static C4Collection get(
}

@VisibleForTesting
@Nullable
@NonNull
static C4Collection getDefault(@NonNull NativeImpl impl, @NonNull C4Database c4db) throws LiteCoreException {
final long c4collection = impl.nGetDefaultCollection(c4db.getPeer());
return (c4collection == 0)
? null
: new C4Collection(impl, c4collection, c4db, Scope.DEFAULT_NAME, Collection.DEFAULT_NAME);
return new C4Collection(
impl,
impl.nGetDefaultCollection(c4db.getPeer()),
c4db, Scope.DEFAULT_NAME,
Collection.DEFAULT_NAME);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ public C4Collection getCollection(@NonNull String scopeName, @NonNull String col
return C4Collection.get(this, scopeName, collectionName);
}

@Nullable
@NonNull
public final C4Collection getDefaultCollection() throws LiteCoreException {
return C4Collection.getDefault(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;

import com.couchbase.lite.CouchbaseLiteError;


/**
* Please see the comments in MValue
Expand Down Expand Up @@ -92,7 +94,7 @@ private MCollection(

protected void assertOpen() {
if ((context != null) && context.isClosed()) {
throw new IllegalStateException("Cannot use a Fleece object after its parent has been closed");
throw new CouchbaseLiteError("Cannot use a Fleece object after its parent has been closed");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,7 @@ fun LogFileConfiguration?.create(
// database default collection, we are about to lose information.
internal fun checkDbCollections(db: Database, collections: Set<Collection>?) {
val colls = collections ?: emptySet()
val defaultCollection = db.defaultCollection
?: throw IllegalArgumentException(Log.lookupStandardMessage("NoDefaultCollectionInConfig"))
if ((colls.size != 1) || (!colls.contains(defaultCollection))) {
if ((colls.size != 1) || (!colls.contains(db.defaultCollection))) {
Log.d(LogDomain.LISTENER, "Copy to deprecated config loses collection information")
}
}
Expand Down
13 changes: 0 additions & 13 deletions common/test/java/com/couchbase/lite/CollectionQueryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testQueryDefaultCollectionA() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)
loadJSONResourceIntoCollection("names_100.json", collection = defaultCollection)

// create with _
Expand All @@ -54,7 +53,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testQueryDefaultCollectionB() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)
loadJSONResourceIntoCollection("names_100.json", collection = defaultCollection)
verifyQuery(
QueryBuilder.createQuery("SELECT name.first FROM _default ORDER BY name.first LIMIT 1", testDatabase),
Expand All @@ -75,7 +73,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testQueryDefaultCollectionC() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)
loadJSONResourceIntoCollection("names_100.json", collection = defaultCollection)
val query = QueryBuilder.createQuery(
"SELECT name.first FROM " + testDatabase.name + " ORDER BY name.first LIMIT 1",
Expand Down Expand Up @@ -303,7 +300,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testFTSQueryWithFullTextIndexInDefaultCollectionA() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

loadJSONResourceIntoCollection("sentences.json", collection = defaultCollection)

Expand Down Expand Up @@ -331,7 +327,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testFTSQueryWithFullTextIndexInDefaultCollectionB() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

loadJSONResourceIntoCollection("sentences.json", collection = defaultCollection)

Expand Down Expand Up @@ -359,7 +354,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testFTSQueryWithFullTextIndexInDefaultCollectionC() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

loadJSONResourceIntoCollection("sentences.json", collection = defaultCollection)

Expand Down Expand Up @@ -387,7 +381,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testFTSQueryWithFullTextIndexInDefaultCollectionD() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

loadJSONResourceIntoCollection("sentences.json", collection = defaultCollection)

Expand Down Expand Up @@ -415,7 +408,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testFTSQueryWithFullTextIndexInDefaultCollectionE() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

loadJSONResourceIntoCollection("sentences.json", collection = defaultCollection)

Expand Down Expand Up @@ -617,7 +609,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testSelectAllResultKeyA() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

var doc = MutableDocument()
doc.setValue("name", "rose")
Expand All @@ -643,7 +634,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testSelectAllResultKeyB() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

var doc = MutableDocument()
doc.setValue("name", "rose")
Expand All @@ -669,7 +659,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testSelectAllResultKeyC() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

var doc = MutableDocument()
doc.setValue("name", "rose")
Expand Down Expand Up @@ -750,7 +739,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testQueryBuilderWithDefaultCollectionAsDataSource() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)
loadJSONResourceIntoCollection("names_100.json", collection = defaultCollection)

// create with default collection
Expand Down Expand Up @@ -906,7 +894,6 @@ class CollectionQueryTest : BaseQueryTest() {
@Test
fun testBuilderSelectAllResultKeyC() {
val defaultCollection = testDatabase.defaultCollection
Assert.assertNotNull(defaultCollection!!)

var doc = MutableDocument()
doc.setValue("name", "rose")
Expand Down
Loading