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

Add _:atomUri property for deduplicating OStatus/ActivityPub legacy records #4593

Merged
merged 1 commit into from Aug 17, 2017

Conversation

@Gargron
Copy link
Member

Gargron commented Aug 14, 2017

  • For remote records, we can't know the ActivityPub URI post-hoc. The URI is the URI, so the id property for such notes will already be OStatus - which is "bad data" as it's unresolvable, but at least it's easy to cross-reference with existing DB entries
  • But for local records, we generate URIs on the fly, so we can generate both. So we can add an _:atomUri property as an alternative ID. If the status is already in someone's DB under that URI, this allows them to work with the existing DB record without creating a duplicate
@Gargron Gargron added the activitypub label Aug 14, 2017
@unarist

This comment has been minimized.

Copy link
Collaborator

unarist commented Aug 14, 2017

If there are A and C on new masto, and B on old masto...

  1. A toots "foo"
  2. B receives it from A via OStatus (uri:... style id)
  3. C receives "foo" from A via ActivityPub, and boosts it
  4. B receives the boost from C via OStatus (https:... style id)

In this case, still B on old masto will get two instances of same toot, right?

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Aug 14, 2017

@unarist Yes, I think you are right. I don't know what could be done about that...

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Aug 14, 2017

@unarist I don't think we can do anything reasonable but urge people to upgrade. I also consider bumping the major version to 2.0 to signify the upgrade.

@nightpool

This comment has been minimized.

Copy link
Collaborator

nightpool commented Aug 17, 2017

#4623 solves @unarist's concern, right? Can we also use it to solve the ActivityPub first, OStatus second duplication problem?

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Aug 17, 2017

@nightpool Yes the two PRs are complementary to each other.

Copy link
Collaborator

nightpool left a comment

okay LGTM

@Gargron Gargron merged commit ad892db into master Aug 17, 2017
3 checks passed
3 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@Gargron Gargron deleted the feature-activitypub-atom-uri branch Aug 17, 2017
@akihikodaki

This comment has been minimized.

Copy link
Collaborator

akihikodaki commented Aug 26, 2017

Sorry for a super late reply, but I am against this change. That would prevent from dealing with old statuses when OStatus compatibility was abolished. Even in such future, there would be statuses which are known only with OStatus id. That is why I decided to write this comment now.

I suggest a different approach: converting idmade by Mastodon in OStatus into id compatible with ActivityPub, and using id made by other entities in OStatus as is.

Here I describe more details about my approach. There are some cases which _:atomUri and my alternative would solve:
a) Distributing statuses from OStatus in ActivityPub
b) Deduplication

My approach would have the following solutions:
a) Converting id made by Mastodon for OStatus into id compatible with ActivityPub, and using made by other entities in OStatus as is.
b) Converting id made by Mastodon for ActivityPub into id compatible with OStatus, and look up the database with it.

That would let Mastodon look like a seamless ActivityPub implementation for other ActivityPub implementations.

My solution is, however, have a disadvantage which _:atomUri does not have: that would not allow other implementations which support both of OStatus and ActivityPub to take advantage of the feature.

tl; dr: my argument is about which to choose, allowing other ActivityPub implementations without extensions (including Mastodon in future) to interact with Mastodon, or allowing other OStatus applications to have ActivityPub support at the same time. In my opinion, it is really unlikely that there would be other softwares with both of OStatus and ActivityPub, and thus we should pursue a seamless ActivityPub implementation.

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Aug 27, 2017

I suggest a different approach: converting id made by Mastodon in OStatus into id compatible with ActivityPub, and using id made by other entities in OStatus as is.

So basically putting ActivityPub::TagManager.instance.uri_for(object) into <id> in Atom feeds? That's all well and good but because local URIs are generated on the fly this would mean all old IDs change, this could be problematic?

@akihikodaki

This comment has been minimized.

Copy link
Collaborator

akihikodaki commented Aug 27, 2017

TagManager.instance.unique_tag_to_local_id is proving old IDs can be parsed (otherwise even existing code woul be broken.) That means they should be converted into ActivityPub IDs without problems.

@Gargron

This comment has been minimized.

Copy link
Member Author

Gargron commented Aug 27, 2017

@akihikodaki Could you provide an example of which lines you would change?

@akihikodaki

This comment has been minimized.

Copy link
Collaborator

akihikodaki commented Aug 27, 2017

That would be a big change as _:atomUri approach does.

First, we need a model to describe whether domains are hosting Mastodon or not. A service to fill the information by accessing to domains is also necessary.

ActivityPub::TagManager.instance.uri_for would use the service and model to check whether the owner of the object is a Mastodon instance (note that it would introduce remote I/O in serializers. Maybe there is a better option.)
If it is Mastodon and the object is made in OStatus, then the id should be converted.

Queries with id should also be updated. (e.g. ActivityPub::Activity::Create.find_existing_status) If there are no matches with the received id, it should convert the id to its OStatus version and look up the status with it again. If a status is found, the service mentioned above should be used to test whether the status was mafe by Mastodon.

In future, those changes can be dropped after translating all existing IDs.

@nightpool

This comment has been minimized.

Copy link
Collaborator

nightpool commented Aug 27, 2017

@akihikodaki even if you know a domain is hosting mastodon, how do you know if they've upgraded to activitypub?

@akihikodaki

This comment has been minimized.

Copy link
Collaborator

akihikodaki commented Aug 27, 2017

I forgot to mention the case of interacting with old Mastodon using OStatus. If an object was made by Mastodon on ActivutyPub, its id should be converted into an OStatus representation before delivering it to OStatus federation.

The operations I described are all done when dealing with those protocols, so you do not have to know which protocol should be used beforehand.

YaQ00 added a commit to YaQ00/mastodon that referenced this pull request Sep 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.