diff --git a/TODO b/TODO index 614db143b..b206a20c1 100644 --- a/TODO +++ b/TODO @@ -4,15 +4,6 @@ There are two TODO lists. This file (good for airplanes) and the online bug trac Offline list: --- does a permanode's modtime include the max claim date of a delete - claim affecting a permanode attribute claim? Yes, it changed, but - it was more of a undo (didn't mean to do that), and not a real - modification in the log. That is: after GC, if you were to delete - all the claims, what would the eventually consistent modtime be? I - argue for only using the -attribute claim dates, and not the - dates of delete claims. TODO: see if we all agree, and then - document/test. - -- genconfig needs to be updated to route incoming blobs to sync handlers; or should sync handlers subscribe to their source's blobhub? No, see latest email to Bill Thiede: sync handlers already subscribe diff --git a/doc/schema/claims/delete.txt b/doc/schema/claims/delete.txt index fbbfdc7d2..f313a1040 100644 --- a/doc/schema/claims/delete.txt +++ b/doc/schema/claims/delete.txt @@ -1,4 +1,6 @@ A claim can delete a permanode or another claim. +(Un)Deletions are not considered as modifications, so the claimDate of a delete claim +is never considered as a modtime in the context of time constrained searches. ----- {"camliVersion": 1," diff --git a/pkg/index/index.go b/pkg/index/index.go index d992cf321..16b44d9a1 100644 --- a/pkg/index/index.go +++ b/pkg/index/index.go @@ -224,65 +224,6 @@ func kvDeleted(k string) (c camtypes.Claim, ok bool) { }, true } -// TODO(mpl): it looks like we won't be needing DeletedAt after all since we're probably -// not going to consider (un)deletions as modifications as far as modtime is concerned. -// Remove once getRecentPerms is done and we're sure about that. - -// DeletedAt returns whether br (a blobref or a claim) should be considered deleted, -// and if yes, at what time the latest deletion occured. If it was never deleted, -// it returns false, time.Time{}. -func (x *Index) DeletedAt(br blob.Ref) (bool, time.Time) { - if x.deletes == nil { - // We still allow the slow path, in case someone creates - // their own Index without a deletes cache. - return x.deletedAtNoCache(br) - } - x.deletes.RLock() - defer x.deletes.RUnlock() - return x.deletedAt(br) -} - -// The caller must hold x.deletes.mu for read. -func (x *Index) deletedAt(br blob.Ref) (bool, time.Time) { - deletes, ok := x.deletes.m[br] - if !ok { - return false, time.Time{} - } - for _, v := range deletes { - if deleterIsDeleted, _ := x.deletedAt(v.deleter); !deleterIsDeleted { - // We can exit early because the deletes are time sorted - return true, v.when - } - } - return false, time.Time{} -} - -// Used when the Index has no deletes cache (x.deletes is nil). -func (x *Index) deletedAtNoCache(br blob.Ref) (bool, time.Time) { - var err error - it := x.queryPrefix(keyDeleted, br) - deleted := false - var mostRecentDeletion time.Time - for it.Next() { - cl, ok := kvDeleted(it.Key()) - if !ok { - panic(fmt.Sprintf("Bogus keyDeleted entry key: want |\"deleted\"||||, got %q", it.Key())) - } - if deleterIsDeleted, _ := x.deletedAtNoCache(cl.BlobRef); !deleterIsDeleted { - deleted = true - mostRecentDeletion = cl.Date - // we can exit early because the iterator gives use time sorted entries for keyDeleted - break - } - } - closeIterator(it, &err) - if err != nil { - // TODO: Do better? - panic(fmt.Sprintf("Could not close iterator on keyDeleted: %v", err)) - } - return deleted, mostRecentDeletion -} - // IsDeleted reports whether the provided blobref (of a permanode or // claim) should be considered deleted. func (x *Index) IsDeleted(br blob.Ref) bool { diff --git a/pkg/index/indextest/tests.go b/pkg/index/indextest/tests.go index a1127b541..d911d9013 100644 --- a/pkg/index/indextest/tests.go +++ b/pkg/index/indextest/tests.go @@ -900,27 +900,14 @@ func Delete(t *testing.T, initIdx func() *index.Index) { cl1Time := id.lastTime() t.Logf("set attribute %q", cl1) - // Test the never, ever, deleted case - deleted, when := idx.DeletedAt(pn1) - if deleted || !when.IsZero() { - t.Fatal("pn1 should never have been deleted") - } - // delete pn1 delpn1 := id.Delete(pn1) - delTime := id.lastTime() t.Logf("del claim %q deletes %q", delpn1, pn1) - deleted = idx.IsDeleted(pn1) + deleted := idx.IsDeleted(pn1) if !deleted { t.Fatal("pn1 should be deleted") } - deleted, when = idx.DeletedAt(pn1) - if !deleted { - t.Fatal("pn1 should be deleted") - } - if !when.Equal(delTime) { - t.Fatalf("pn1 should have been deleted at %v, not %v", delTime, when) - } + // and try to find it with SearchPermanodesWithAttr (which should not work) { ch := make(chan blob.Ref, 10) @@ -944,32 +931,19 @@ func Delete(t *testing.T, initIdx func() *index.Index) { // delete pn1 again with another claim delpn1bis := id.Delete(pn1) - delTime = id.lastTime() t.Logf("del claim %q deletes %q a second time", delpn1bis, pn1) deleted = idx.IsDeleted(pn1) if !deleted { t.Fatal("pn1 should be deleted") } - deleted, when = idx.DeletedAt(pn1) - if !deleted { - t.Fatal("pn1 should be deleted") - } - if !when.Equal(delTime) { - t.Fatalf("pn1 should have been deleted at %v, not %v", delTime, when) - } // verify that deleting delpn1 is not enough to make pn1 undeleted del2 := id.Delete(delpn1) - delTime = id.lastTime() t.Logf("delete claim %q deletes %q, which should not yet revive %q", del2, delpn1, pn1) deleted = idx.IsDeleted(pn1) if !deleted { t.Fatal("pn1 should not yet be undeleted") } - deleted, when = idx.DeletedAt(pn1) - if !deleted { - t.Fatal("pn1 should not yet be undeleted") - } // we should not yet be able to find it again with SearchPermanodesWithAttr { ch := make(chan blob.Ref, 10) @@ -993,16 +967,11 @@ func Delete(t *testing.T, initIdx func() *index.Index) { // delete delpn1bis as well -> should undelete pn1 del2bis := id.Delete(delpn1bis) - delTime = id.lastTime() t.Logf("delete claim %q deletes %q, which should revive %q", del2bis, delpn1bis, pn1) deleted = idx.IsDeleted(pn1) if deleted { t.Fatal("pn1 should be undeleted") } - deleted, when = idx.DeletedAt(pn1) - if deleted { - t.Fatal("pn1 should be undeleted") - } // we should now be able to find it again with SearchPermanodesWithAttr { ch := make(chan blob.Ref, 10) diff --git a/pkg/index/keys.go b/pkg/index/keys.go index 3d51b4b8c..e23a6bc21 100644 --- a/pkg/index/keys.go +++ b/pkg/index/keys.go @@ -269,17 +269,6 @@ var ( nil, } - // keyDeletes indexes a claim that deletes an entity. It ties the deleter - // claim to the deleted entity. - keyDeletes = &keyType{ - "deletes", - []part{ - {"deleter", typeBlobRef}, // the deleter claim blobref - {"deleted", typeBlobRef}, // the deleted entity (a permanode or another claim) - }, - nil, - } - // Given a blobref (permanode or static file or directory), provide a mapping // to potential parents (they may no longer be parents, in the case of permanodes). // In the case of permanodes, camliMember or camliContent constitutes a forward diff --git a/pkg/index/receive.go b/pkg/index/receive.go index 392b1f115..9a6a53b55 100644 --- a/pkg/index/receive.go +++ b/pkg/index/receive.go @@ -364,7 +364,6 @@ func (ix *Index) populateDeleteClaim(cl schema.Claim, vr *jsonsign.VerifyRequest return } mm.Set(keyDeleted.Key(target, cl.ClaimDateString(), br), "") - mm.Set(keyDeletes.Key(br, target), "") if meta.CamliType == "claim" { return } diff --git a/pkg/search/search.go b/pkg/search/search.go index fa842c7e8..da31e0bad 100644 --- a/pkg/search/search.go +++ b/pkg/search/search.go @@ -1,5 +1,5 @@ /* -Copyright 2011 Google Inc. +Copyright 2011 The Camlistore Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -15,4 +15,10 @@ limitations under the License. */ // Package search describes and answers Camlistore search queries. +// +// Many of the search methods or functions provide results that are +// ordered by modification time, or at least depend on modification +// times. In that context, (un)deletions (of permanodes, or attributes) +// are not considered modifications and therefore the time at which they +// occured does not affect the result. package search