From f60bb145cf00b335fd14f4d39d531771711149da Mon Sep 17 00:00:00 2001 From: Cory Slep Date: Sun, 13 May 2018 13:05:12 +0200 Subject: [PATCH] Deduplicate adding to like/following collections. We simply prevent adding a duplicate likes, liked, following, or follower based on the IRI of the element being added to the collection. Adds corresponding black box tests. Note that this deduplication does not cover the inbox. --- pub/fed.go | 2 - pub/fed_test.go | 216 ++++++++++++++++++++++++++++++++++++++++++++++++ pub/internal.go | 122 ++++++++++++++++++++++++--- 3 files changed, 326 insertions(+), 14 deletions(-) diff --git a/pub/fed.go b/pub/fed.go index 5ff4913..42bab66 100644 --- a/pub/fed.go +++ b/pub/fed.go @@ -887,7 +887,6 @@ func (f *federator) handleFollow(c context.Context) func(s *streams.Follow) erro } return false, fmt.Errorf("cannot determine type of object followers") } - // TODO: Deduplication detection. var err error if ownsAny, err = f.addAllActorsToObjectCollection(c, getter, raw); err != nil { return err @@ -944,7 +943,6 @@ func (f *federator) handleAccept(c context.Context) func(s *streams.Accept) erro } return false, fmt.Errorf("cannot determine type of actor following") } - // TODO: Deduplication detection. if err := f.addAllObjectsToActorCollection(c, getter, follow); err != nil { return err } diff --git a/pub/fed_test.go b/pub/fed_test.go index e6297f1..130afeb 100644 --- a/pub/fed_test.go +++ b/pub/fed_test.go @@ -2170,6 +2170,79 @@ func TestPostInbox_Follow_AutoAccept(t *testing.T) { } } +func TestPostInbox_Follow_DoesNotAddForAutoAcceptIfAlreadyPresent(t *testing.T) { + app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p := NewPubberTest(t) + PreparePostInboxTest(t, app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p) + resp := httptest.NewRecorder() + req := ActivityPubRequest(httptest.NewRequest("POST", testInboxURI, bytes.NewBuffer(MustSerialize(testFollow)))) + fedApp.onFollow = func(c context.Context, s *streams.Follow) FollowResponse { + return AutomaticAccept + } + fedCb.follow = func(c context.Context, s *streams.Follow) error { + return nil + } + gotHttpDo := 0 + httpClient.do = func(req *http.Request) (*http.Response, error) { + gotHttpDo++ + if gotHttpDo == 1 { + actorResp := &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewBuffer(sallyActorJSON)), + } + return actorResp, nil + } else if gotHttpDo == 2 { + okResp := &http.Response{ + StatusCode: http.StatusOK, + Body: ioutil.NopCloser(bytes.NewBuffer([]byte{})), + } + return okResp, nil + } + return nil, nil + } + d.do = func(b []byte, u url.URL, toDo func(b []byte, u url.URL) error) { + if err := toDo(b, u); err != nil { + t.Fatalf("Unexpected error in MockDeliverer.Do: %s", err) + } + } + app.owns = func(c context.Context, id url.URL) bool { + return true + } + app.get = func(c context.Context, id url.URL) (PubObject, error) { + followers := &vocab.Collection{} + followers.AddItemsIRI(*sallyIRI) + samActor := &vocab.Person{} + samActor.SetInboxAnyURI(*samIRIInbox) + samActor.SetId(*samIRI) + samActor.SetFollowersCollection(followers) + return samActor, nil + } + gotSet := 0 + var setObject PubObject + app.set = func(c context.Context, o PubObject) error { + gotSet++ + if gotSet == 1 { + setObject = o + } + return nil + } + expectedFollowers := &vocab.Collection{} + expectedFollowers.AddItemsIRI(*sallyIRI) + expectedActor := &vocab.Person{} + expectedActor.SetInboxAnyURI(*samIRIInbox) + expectedActor.SetId(*samIRI) + expectedActor.SetFollowersCollection(expectedFollowers) + handled, err := p.PostInbox(context.Background(), resp, req) + if err != nil { + t.Fatal(err) + } else if !handled { + t.Fatalf("expected handled, got !handled") + } else if gotSet != 2 { + t.Fatalf("expected %d, got %d", 2, gotSet) + } else if err := PubObjectEquals(setObject, expectedActor); err != nil { + t.Fatal(err) + } +} + func TestPostInbox_Follow_AutoAcceptFollowersIsOrderedCollection(t *testing.T) { app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p := NewPubberTest(t) PreparePostInboxTest(t, app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p) @@ -2458,6 +2531,53 @@ func TestPostInbox_Accept_AcceptFollowAddsToFollowersIfOwned(t *testing.T) { } } +func TestPostInbox_Accept_AcceptFollowDoesNotAddIfAlreadyInCollection(t *testing.T) { + app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p := NewPubberTest(t) + PreparePostInboxTest(t, app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p) + resp := httptest.NewRecorder() + req := ActivityPubRequest(httptest.NewRequest("POST", testInboxURI, bytes.NewBuffer(MustSerialize(testAcceptFollow)))) + app.owns = func(c context.Context, id url.URL) bool { + return true + } + app.get = func(c context.Context, id url.URL) (PubObject, error) { + following := &vocab.Collection{} + following.AddItemsIRI(*samIRI) + sallyActor := &vocab.Person{} + sallyActor.SetInboxAnyURI(*sallyIRIInbox) + sallyActor.SetId(*sallyIRI) + sallyActor.SetFollowingCollection(following) + return sallyActor, nil + } + gotSet := 0 + var setObject PubObject + app.set = func(c context.Context, o PubObject) error { + gotSet++ + if gotSet == 1 { + setObject = o + } + return nil + } + fedCb.accept = func(c context.Context, s *streams.Accept) error { + return nil + } + expectedFollowing := &vocab.Collection{} + expectedFollowing.AddItemsIRI(*samIRI) + expectedActor := &vocab.Person{} + expectedActor.SetInboxAnyURI(*sallyIRIInbox) + expectedActor.SetId(*sallyIRI) + expectedActor.SetFollowingCollection(expectedFollowing) + handled, err := p.PostInbox(context.Background(), resp, req) + if err != nil { + t.Fatal(err) + } else if !handled { + t.Fatalf("expected handled, got !handled") + } else if gotSet != 2 { + t.Fatalf("expected %d, got %d", 2, gotSet) + } else if err := PubObjectEquals(setObject, expectedActor); err != nil { + t.Fatal(err) + } +} + func TestPostInbox_Accept_AcceptFollowAddsToFollowersOrderedCollection(t *testing.T) { app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p := NewPubberTest(t) PreparePostInboxTest(t, app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p) @@ -3066,6 +3186,55 @@ func TestPostInbox_Like_AddsToLikeCollection(t *testing.T) { } } +func TestPostInbox_Like_DoesNotAddLikeToCollectionIfAlreadyPresent(t *testing.T) { + app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p := NewPubberTest(t) + PreparePostInboxTest(t, app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p) + resp := httptest.NewRecorder() + req := ActivityPubRequest(httptest.NewRequest("POST", testInboxURI, bytes.NewBuffer(MustSerialize(testLikeNote)))) + app.owns = func(c context.Context, id url.URL) bool { + return true + } + app.get = func(c context.Context, id url.URL) (PubObject, error) { + likes := &vocab.Collection{} + likes.AddItemsIRI(*sallyIRI) + v := &vocab.Note{} + v.SetId(*noteIRI) + v.AddNameString(noteName) + v.AddContentString("This is a simple note") + v.SetLikesCollection(likes) + return v, nil + } + gotSet := 0 + var gotSetObject PubObject + app.set = func(c context.Context, target PubObject) error { + gotSet++ + if gotSet == 1 { + gotSetObject = target + } + return nil + } + fedCb.like = func(c context.Context, s *streams.Like) error { + return nil + } + handled, err := p.PostInbox(context.Background(), resp, req) + expected := &vocab.Collection{} + expected.AddItemsIRI(*sallyIRI) + expectedNote := &vocab.Note{} + expectedNote.SetId(*noteIRI) + expectedNote.AddNameString(noteName) + expectedNote.AddContentString("This is a simple note") + expectedNote.SetLikesCollection(expected) + if err != nil { + t.Fatal(err) + } else if !handled { + t.Fatalf("expected handled, got !handled") + } else if gotSet != 2 { + t.Fatalf("expected %d, got %d", 2, gotSet) + } else if err := PubObjectEquals(gotSetObject, expectedNote); err != nil { + t.Fatalf("unexpected callback object: %s", err) + } +} + func TestPostInbox_Like_AddsToLikeOrderedCollection(t *testing.T) { app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p := NewPubberTest(t) PreparePostInboxTest(t, app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p) @@ -4817,6 +4986,53 @@ func TestPostOutbox_Like_AddsToLikedCollection(t *testing.T) { } } +func TestPostOutbox_Like_DoesNotAddIfAlreadyLiked(t *testing.T) { + app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p := NewPubberTest(t) + PreparePostOutboxTest(t, app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p) + resp := httptest.NewRecorder() + req := ActivityPubRequest(httptest.NewRequest("POST", testOutboxURI, bytes.NewBuffer(MustSerialize(testLikeNote)))) + app.owns = func(c context.Context, iri url.URL) bool { + return true + } + app.get = func(c context.Context, iri url.URL) (PubObject, error) { + liked := &vocab.Collection{} + liked.AddItemsIRI(*noteIRI) + v := &vocab.Person{} + v.AddNameString("Sally") + v.SetId(*sallyIRI) + v.SetLikedCollection(liked) + return v, nil + } + gotSet := 0 + var gotSetObj PubObject + app.set = func(c context.Context, o PubObject) error { + gotSet++ + if gotSet == 1 { + gotSetObj = o + } + return nil + } + socialCb.like = func(c context.Context, s *streams.Like) error { + return nil + } + handled, err := p.PostOutbox(context.Background(), resp, req) + expectedLikes := &vocab.Collection{} + expectedLikes.AddItemsIRI(*noteIRI) + expectedActor := &vocab.Person{} + expectedActor.AddNameString("Sally") + expectedActor.SetId(*sallyIRI) + expectedActor.SetLikedCollection(expectedLikes) + if err != nil { + t.Fatal(err) + } else if !handled { + t.Fatalf("expected handled, got !handled") + } else if gotSet != 2 { + t.Fatalf("expected %d, got %d", 2, gotSet) + } else if err := PubObjectEquals(gotSetObj, expectedActor); err != nil { + t.Fatalf("set obj: %s", err) + } +} + func TestPostOutbox_Like_AddsToLikedOrderedCollection(t *testing.T) { app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p := NewPubberTest(t) PreparePostOutboxTest(t, app, socialApp, fedApp, socialCb, fedCb, d, httpClient, p) diff --git a/pub/internal.go b/pub/internal.go index 0faeb43..03dd5b6 100644 --- a/pub/internal.go +++ b/pub/internal.go @@ -1218,27 +1218,50 @@ func (f *federator) addAllObjectsToActorCollection(ctx context.Context, getter g ok := false actor, ok = pObj.(vocab.ObjectType) if !ok { - // TODO: Handle links, too return fmt.Errorf("actor is not vocab.ObjectType") } + // Obtain ordered/unordered collection var lc vocab.CollectionType var loc vocab.OrderedCollectionType isIRI := false if isIRI, err = getter(actor, &lc, &loc); err != nil { return err } + // Duplication detection + var iriSet map[url.URL]bool + if lc != nil { + iriSet, err = getIRISetFromItems(lc) + } else if loc != nil { + iriSet, err = getIRISetFromOrderedItems(loc) + } + if err != nil { + return err + } + // Add object to collection if not a duplicate for i := 0; i < c.ObjectLen(); i++ { if c.IsObjectIRI(i) { + iri := c.GetObjectIRI(i) + if iriSet[iri] { + continue + } if lc != nil { - lc.AddItemsIRI(c.GetObjectIRI(i)) + lc.AddItemsIRI(iri) } else if loc != nil { - loc.AddOrderedItemsIRI(c.GetObjectIRI(i)) + loc.AddOrderedItemsIRI(iri) } } else if c.IsObject(i) { + obj := c.GetObject(i) + if !obj.HasId() { + return fmt.Errorf("object at index %d has no id", i) + } + iri := obj.GetId() + if iriSet[iri] { + continue + } if lc != nil { - lc.AddItemsObject(c.GetObject(i)) + lc.AddItemsObject(obj) } else if loc != nil { - loc.AddOrderedItemsObject(c.GetObject(i)) + loc.AddOrderedItemsObject(obj) } } } @@ -1285,33 +1308,64 @@ func (f *federator) addAllActorsToObjectCollection(ctx context.Context, getter g ok := false object, ok = pObj.(vocab.ObjectType) if !ok { - // TODO: Handle links, too return ownsAny, fmt.Errorf("object is not vocab.ObjectType") } + // Obtain ordered/unordered collection var lc vocab.CollectionType var loc vocab.OrderedCollectionType isIRI := false if isIRI, err = getter(object, &lc, &loc); err != nil { return ownsAny, err } + // Duplication detection + var iriSet map[url.URL]bool + if lc != nil { + iriSet, err = getIRISetFromItems(lc) + } else if loc != nil { + iriSet, err = getIRISetFromOrderedItems(loc) + } + if err != nil { + return ownsAny, err + } + // Add actor to collection if not a duplicate for i := 0; i < c.ActorLen(); i++ { if c.IsActorIRI(i) { + iri := c.GetActorIRI(i) + if iriSet[iri] { + continue + } if lc != nil { - lc.AddItemsIRI(c.GetActorIRI(i)) + lc.AddItemsIRI(iri) } else if loc != nil { - loc.AddOrderedItemsIRI(c.GetActorIRI(i)) + loc.AddOrderedItemsIRI(iri) } } else if c.IsActorObject(i) { + obj := c.GetActorObject(i) + if !obj.HasId() { + return ownsAny, fmt.Errorf("actor object at index %d has no id", i) + } + iri := obj.GetId() + if iriSet[iri] { + continue + } if lc != nil { - lc.AddItemsObject(c.GetActorObject(i)) + lc.AddItemsObject(obj) } else if loc != nil { - loc.AddOrderedItemsObject(c.GetActorObject(i)) + loc.AddOrderedItemsObject(obj) } } else if c.IsActorLink(i) { + l := c.GetActorLink(i) + if !l.HasHref() { + return ownsAny, fmt.Errorf("actor link at index %d has no id", i) + } + iri := l.GetHref() + if iriSet[iri] { + continue + } if lc != nil { - lc.AddItemsLink(c.GetActorLink(i)) + lc.AddItemsLink(l) } else if loc != nil { - loc.AddOrderedItemsLink(c.GetActorLink(i)) + loc.AddOrderedItemsLink(l) } } } @@ -1626,6 +1680,50 @@ func recursivelyApplyDeletes(m, hasNils map[string]interface{}) { } } +func getIRISetFromItems(c vocab.CollectionType) (map[url.URL]bool, error) { + r := make(map[url.URL]bool, c.ItemsLen()) + for i := 0; i < c.ItemsLen(); i++ { + if c.IsItemsObject(i) { + obj := c.GetItemsObject(i) + if !obj.HasId() { + return r, fmt.Errorf("items object at index %d has no id", i) + } + r[obj.GetId()] = true + } else if c.IsItemsLink(i) { + l := c.GetItemsLink(i) + if !l.HasHref() { + return r, fmt.Errorf("items link at index %d has no href", i) + } + r[l.GetHref()] = true + } else if c.IsItemsIRI(i) { + r[c.GetItemsIRI(i)] = true + } + } + return r, nil +} + +func getIRISetFromOrderedItems(c vocab.OrderedCollectionType) (map[url.URL]bool, error) { + r := make(map[url.URL]bool, c.OrderedItemsLen()) + for i := 0; i < c.OrderedItemsLen(); i++ { + if c.IsOrderedItemsObject(i) { + obj := c.GetOrderedItemsObject(i) + if !obj.HasId() { + return r, fmt.Errorf("items object at index %d has no id", i) + } + r[obj.GetId()] = true + } else if c.IsOrderedItemsLink(i) { + l := c.GetOrderedItemsLink(i) + if !l.HasHref() { + return r, fmt.Errorf("items link at index %d has no href", i) + } + r[l.GetHref()] = true + } else if c.IsOrderedItemsIRI(i) { + r[c.GetOrderedItemsIRI(i)] = true + } + } + return r, nil +} + // TODO: Move this to vocab package. var activityTypes = []string{"Accept", "Add", "Announce", "Arrive", "Block", "Create", "Delete", "Dislike", "Flag", "Follow", "Ignore", "Invite", "Join", "Leave", "Like", "Listen", "Move", "Offer", "Question", "Reject", "Read", "Remove", "TentativeReject", "TentativeAccept", "Travel", "Undo", "Update", "View"}