Skip to content

Commit

Permalink
CBL-5684: Database.getDefaultCollection() should be non-null
Browse files Browse the repository at this point in the history
Co-authored-by: Jeff Lockhart <[email protected]>
  • Loading branch information
bmeike and jeffdgr8 committed Apr 26, 2024
1 parent e73d92b commit 6890a50
Show file tree
Hide file tree
Showing 17 changed files with 135 additions and 110 deletions.
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
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 @@ -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

0 comments on commit 6890a50

Please sign in to comment.