Skip to content

Commit

Permalink
Tracked down the problem with the one crashing unit test (ForestDBCou…
Browse files Browse the repository at this point in the history 
…chStore.cs calling c4doc_selectRevision on a null document)

Closes #538
  • Loading branch information
@borrrden
borrrden committed Dec 17, 2015
1 parent ce0edfc commit f8ff89b
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 33 deletions.
132 changes: 109 additions & 23 deletions src/Couchbase.Lite.Shared/Database.cs
View file
Expand Up @@ -125,6 +125,11 @@ public sealed class Database : ICouchStoreDelegate, IDisposable
public CookieContainer PersistentCookieStore
{
get {
if (!IsOpen) {
Log.W(TAG, "PersistentCookeStore called on closed database");
return null;
}

if (_persistentCookieStore == null) {
_persistentCookieStore = new CookieStore(DbDirectory);
}
Expand Down Expand Up @@ -153,6 +158,11 @@ public CookieContainer PersistentCookieStore
public int DocumentCount
{
get {
if (!IsOpen) {
Log.W(TAG, "DocumentCount called on closed database");
return 0;
}

return Storage.DocumentCount;
}
}
Expand All @@ -166,6 +176,11 @@ public int DocumentCount
public long LastSequenceNumber
{
get {
if (!IsOpen) {
Log.W(TAG, "LastSequenceNumber called on closed database");
return 0;
}

return Storage.LastSequence;
}
}
Expand All @@ -175,6 +190,11 @@ public long LastSequenceNumber
/// </summary>
public long TotalDataSize {
get {
if (!IsOpen) {
Log.W(TAG, "TotalDataSize called on closed database");
return 0L;
}

string dir = DbDirectory;
var info = new DirectoryInfo(dir);

Expand All @@ -194,6 +214,11 @@ public long LastSequenceNumber
public IEnumerable<Replication> AllReplications
{
get {
if (!IsOpen) {
Log.W(TAG, "AllReplications called on closed database");
return null;
}

return AllReplicators.ToList();
}
}
Expand Down Expand Up @@ -310,6 +335,11 @@ internal Database(string directory, string name, Manager manager, bool readOnly)
/// compacting the <see cref="Couchbase.Lite.Database" /></exception>
public bool Compact()
{
if (!IsOpen) {
Log.W(TAG, "CreatePushReplication called on closed database");
return false;
}

try {
Storage.Compact();
} catch(CouchbaseLiteException) {
Expand All @@ -329,10 +359,7 @@ public bool Compact()
/// Thrown if an issue occurs while deleting the <see cref="Couchbase.Lite.Database" /></exception>
public void Delete()
{
if (IsOpen) {
Close();
}

Close();
if (!Exists()) {
return;
}
Expand Down Expand Up @@ -376,6 +403,10 @@ public Document CreateDocument()
/// <param name="id">Identifier.</param>
public IDictionary<String, Object> GetExistingLocalDocument(String id)
{
if (!IsOpen) {
throw new InvalidOperationException("Database is closed");
}

var gotRev = Storage.GetLocalDocument(MakeLocalDocumentId(id), null);
return gotRev != null ? gotRev.GetProperties() : null;
}
Expand All @@ -390,6 +421,10 @@ public Document CreateDocument()
/// while setting the contents of the local document.</exception>
public bool PutLocalDocument(string id, IDictionary<string, object> properties)
{
if (!IsOpen) {
throw new InvalidOperationException("Database is closed");
}

id = MakeLocalDocumentId(id);
var rev = new RevisionInternal(id, null, properties == null);
if (properties != null) {
Expand Down Expand Up @@ -417,6 +452,10 @@ public bool DeleteLocalDocument(string id)
/// <returns>Returns a <see cref="Couchbase.Lite.Query" /> that matches all <see cref="Couchbase.Lite.Document" />s in the <see cref="Couchbase.Lite.Database" />s.</returns>
public Query CreateAllDocumentsQuery()
{
if (!IsOpen) {
throw new InvalidOperationException("Database is closed");
}

return new Query(this, (View)null);
}

Expand All @@ -429,8 +468,12 @@ public Query CreateAllDocumentsQuery()
/// <param name="name">The name of the <see cref="Couchbase.Lite.View" /> to get or create.</param>
public View GetView(String name)
{
View view = null;
if (!IsOpen) {
Log.W(TAG, "GetView called on closed database");
return null;
}

View view = null;
if (_views != null) {
view = _views.Get(name);
}
Expand All @@ -449,6 +492,11 @@ public View GetView(String name)
/// <param name="name">The name of the View to get.</param>
public View GetExistingView(String name)
{
if (!IsOpen) {
Log.W(TAG, "GetExistingView called on closed database");
return null;
}

View view = null;
if (_views != null) {
_views.TryGetValue(name, out view);
Expand All @@ -470,6 +518,11 @@ public View GetExistingView(String name)
/// <param name="name">The name of the validation delegate to get.</param>
public ValidateDelegate GetValidation(String name)
{
if (!IsOpen) {
Log.W(TAG, "GetValidation called on closed database");
return null;
}

ValidateDelegate retVal = null;
if (!Shared.TryGetValue<ValidateDelegate>("validation", name, Name, out retVal)) {
return null;
Expand All @@ -487,8 +540,13 @@ public ValidateDelegate GetValidation(String name)
/// </summary>
/// <param name="name">The name of the validation delegate to set.</param>
/// <param name="validationDelegate">The validation delegate to set.</param>
public void SetValidation(String name, ValidateDelegate validationDelegate)
public void SetValidation(string name, ValidateDelegate validationDelegate)
{
if (!IsOpen) {
Log.W(TAG, "SetValidation called on closed database");
return;
}

Shared.SetValue("validation", name, Name, validationDelegate);
}

Expand All @@ -500,7 +558,11 @@ public void SetValidation(String name, ValidateDelegate validationDelegate)
/// <param name="status">The result of the operation</param>
public FilterDelegate GetFilter(String name, Status status = null)
{
if (name == null) {
if (!IsOpen || name == null) {
if (!IsOpen) {
Log.W(TAG, "GetFilter called on closed database");
}

return null;
}

Expand Down Expand Up @@ -551,6 +613,11 @@ public FilterDelegate GetFilter(String name, Status status = null)
/// <param name="filterDelegate">The filter delegate to set.</param>
public void SetFilter(String name, FilterDelegate filterDelegate)
{
if (!IsOpen) {
Log.W(TAG, "SetFilter called on closed database");
return;
}

Shared.SetValue("filter", name, Name, filterDelegate);
}

Expand All @@ -572,6 +639,11 @@ public Task RunAsync(RunAsyncDelegate runAsyncDelegate)
/// <param name="transactionDelegate">The delegate to run within a transaction.</param>
public bool RunInTransaction(RunInTransactionDelegate transactionDelegate)
{
if (!IsOpen) {
Log.W(TAG, "RunInTransaction called on closed database");
return false;
}

return Storage.RunInTransaction(transactionDelegate);
}

Expand All @@ -583,6 +655,11 @@ public bool RunInTransaction(RunInTransactionDelegate transactionDelegate)
/// <param name="url">The url of the target Database.</param>
public Replication CreatePushReplication(Uri url)
{
if (!IsOpen) {
Log.W(TAG, "CreatePushReplication called on closed database");
return null;
}

var scheduler = new SingleTaskThreadpoolScheduler();
return new Pusher(this, url, false, new TaskFactory(scheduler));
}
Expand All @@ -594,6 +671,11 @@ public Replication CreatePushReplication(Uri url)
/// <param name="url">The url of the source Database.</param>
public Replication CreatePullReplication(Uri url)
{
if (!IsOpen) {
Log.W(TAG, "CreatePullReplication called on closed database");
return null;
}

var scheduler = new SingleTaskThreadpoolScheduler();
return new Puller(this, url, false, new TaskFactory(scheduler));
}
Expand Down Expand Up @@ -800,8 +882,8 @@ internal static string MakeLocalDocumentId(string documentId)

internal void SetLastSequence(string lastSequence, string checkpointId)
{
if (Storage == null) {
Log.I(TAG, "Storage is null, so not attempting to set last sequence");
if (Storage == null || !Storage.IsOpen) {
Log.I(TAG, "Storage is null or closed, so not attempting to set last sequence");
return;
}

Expand All @@ -810,7 +892,7 @@ internal void SetLastSequence(string lastSequence, string checkpointId)

internal string LastSequenceWithCheckpointId(string checkpointId)
{
if (Storage == null) {
if (Storage == null || !Storage.IsOpen) {
return String.Empty;
}

Expand Down Expand Up @@ -1901,21 +1983,20 @@ internal void Close()
return;
}

IsOpen = false;
Log.D(TAG, "Closing database at {0}", DbDirectory);
if (_views != null) {
foreach (var view in _views) {
view.Value.Close();
}
}

if (ActiveReplicators != null) {
var activeReplicatorCopy = new Replication[ActiveReplicators.Count];
ActiveReplicators.CopyTo(activeReplicatorCopy, 0);
var activeReplicatorCopy = ActiveReplicators;
ActiveReplicators = null;
if(activeReplicatorCopy != null && activeReplicatorCopy.Count > 0) {
foreach (var repl in activeReplicatorCopy) {
repl.DatabaseClosing();
}

ActiveReplicators = null;
}

try {
Expand All @@ -1928,9 +2009,8 @@ internal void Close()
} finally {
Storage = null;

IsOpen = false;
UnsavedRevisionDocumentCache.Clear();
DocumentCache = new LruCache<string, Document>(DocumentCache.MaxSize);
DocumentCache = null;
Manager.ForgetDatabase(this);
}
}
Expand Down Expand Up @@ -2008,11 +2088,12 @@ internal void OpenWithOptions(DatabaseOptions options)
// Open the storage!
try {
Storage.Open(DbDirectory, Manager, _readonly);

// HACK: Needed to overcome the read connection not getting the write connection
// changes until after the schema is written
Storage.Close();
Storage.Open(DbDirectory, Manager, _readonly);
if(Storage is SqliteCouchStore) {
// HACK: Needed to overcome the read connection not getting the write connection
// changes until after the schema is written
Storage.Close();
Storage.Open(DbDirectory, Manager, _readonly);
}
} catch(CouchbaseLiteException) {
Storage.Close();
Log.E(TAG, "Failed to open storage for database");
Expand Down Expand Up @@ -2080,6 +2161,11 @@ internal void Open()

private void ChangeEncryptionKey(SymmetricKey newKey)
{
if (!IsOpen) {
Log.W(TAG, "ChangeEncryptionKey called on closed database");
return;
}

var action = Storage.ActionToChangeEncryptionKey(newKey);
action.AddLogic(Attachments.ActionToChangeEncryptionKey(newKey));
action.AddLogic(() => Shared.SetValue("encryptionKey", "", Name, newKey), null, null);
Expand Down
1 change: 1 addition & 0 deletions src/Couchbase.Lite.Shared/Manager.cs
View file
Expand Up @@ -570,6 +570,7 @@ internal Database GetDatabase(string name, bool mustExist)

db.Name = name;
databases.Put(name, db);
Shared.OpenedDatabase(db);
}

return db;
Expand Down
9 changes: 5 additions & 4 deletions src/Couchbase.Lite.Shared/Replication.cs
View file
Expand Up @@ -1761,7 +1761,6 @@ private static bool Is404(Exception e)

private void SaveLastSequence(SaveLastSequenceCompletionBlock completionHandler)
{

if (!lastSequenceChanged) {
if (completionHandler != null) {
completionHandler();
Expand All @@ -1773,6 +1772,7 @@ private void SaveLastSequence(SaveLastSequenceCompletionBlock completionHandler)
// If a save is already in progress, don't do anything. (The completion block will trigger
// another save after the first one finishes.)
Task.Delay(500).ContinueWith(t => SaveLastSequence(completionHandler));
return;
}

lastSequenceChanged = false;
Expand Down Expand Up @@ -1820,13 +1820,14 @@ private void SaveLastSequence(SaveLastSequenceCompletionBlock completionHandler)
body.Put ("_rev", response.Get ("rev"));
_remoteCheckpoint = body;
var localDb = LocalDatabase;
if(localDb == null || localDb.Storage == null) {
Log.W(TAG, "Database is null, ignoring remote checkpoint response");
if(localDb == null || !localDb.IsOpen) {
Log.W(TAG, "Database is null or closed, ignoring remote checkpoint response");
if(completionHandler != null) {
completionHandler();
}
return;
}
localDb.SetLastSequence(LastSequence, remoteCheckpointDocID);
}
Expand Down Expand Up @@ -1932,7 +1933,7 @@ private void ClearDbRef()
// response arrives _db will be nil, so there won't be any way to save the checkpoint locally.
// To avoid that, pre-emptively save the local checkpoint now.
var localDb = LocalDatabase;
if (localDb != null && _savingCheckpoint && LastSequence != null) {
if (localDb != null && localDb.IsOpen && _savingCheckpoint && LastSequence != null) {
localDb.SetLastSequence(LastSequence, RemoteCheckpointDocID());
}

Expand Down
4 changes: 4 additions & 0 deletions src/Couchbase.Lite.Shared/Store/BlobStore.cs
View file
Expand Up @@ -175,6 +175,10 @@ public bool GetKeyForFilename(BlobKey outKey, string filename)
public byte[] BlobForKey(BlobKey key)
{
using (var blobStream = BlobStreamForKey(key)) {
if (blobStream == null) {
return null;
}

return blobStream.ReadAllBytes();
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/Couchbase.Lite.Shared/Store/ForestDBCouchStore.cs
View file
Expand Up @@ -787,9 +787,10 @@ public int FindMissingRevisions(RevisionList revs)
lastDocId = rev.GetDocId();
Native.c4doc_free(doc);
doc = Native.c4doc_get(Forest, lastDocId, true, null);
if(doc == null) {
continue;
}
}

if(doc == null) {
continue;
}

if (Native.c4doc_selectRevision(doc, rev.GetRevId(), false, null)) {
Expand Down

0 comments on commit f8ff89b

Please sign in to comment.