Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Permit properties currently in endpoints to within the Actor itself #291

Closed
cjslep opened this issue Jan 17, 2018 · 9 comments
Closed

Permit properties currently in endpoints to within the Actor itself #291

cjslep opened this issue Jan 17, 2018 · 9 comments

Comments

@cjslep
Copy link

cjslep commented Jan 17, 2018

The endpoints property is currently an ActivityStreams Object with a grab-bag of custom properties pertinent to a specific Actor. However, the specification does not permit these properties to be directly populated directly on the Actor. The properties are:

  • proxyUrl
  • oauthAuthorizationEndpoint
  • oauthTokenEndpoint
  • provideClientKey
  • signClientKey
  • sharedInbox

Since all are URI values, there should be no data-readability issue with populating them directly on an actor. Furthermore, since this endpoint Object is not required to reference the actor it pertains to it alone has no semantic meaning and implementations must process it with a larger context.

It is a case of Accidental Complexity within the specification: the alternate is to just give an implementation the Actor with the above properties populated. Currently, implementations must be given an Actor for context and the resolved endpoints URI object and incur the extra HTTPS call to do the resolution.

Once both are permitted, I would like to consider deprecating endpoints as a property of the Actor and favor directly setting the properties on the Actor.

@nightpool
Copy link
Collaborator

endpoints should generally be static within a server, or grouping of actors. this is in contrast to actor properties, which have no such relationship.

@cjslep
Copy link
Author

cjslep commented Jan 17, 2018

The spec does not say SHOULD which is understood with regards to RFC2119. The spec says "(typically server/domain-wide) endpoints" which as far as an implementor is concerned is gentle guidance.

Looking at the actual relation within the spec, endpoint is designed to be accessible on individual Actors (which may or may not represent groups, individuals, non-Actor ActivityStreams, etc) so the implied relation is that endpoint is dependent on the Actor. I have an aggressive imagination, and a fancy sharded specialized-server setup could fit this bill and still try to federate nicely.

Since Actors can be anything, and endpoints have effectively no bounds to their value, the endpoints grouping is effectively semantically arbitrary to an implementor. I am just asking for consistency due to the lack of effective meaning.

@nightpool
Copy link
Collaborator

nightpool commented Jan 17, 2018

yes, I understand. that's why I said generally static within a server. while the distinction in theoretically arbitrary, in practice the separation allows for caching and deduplication of these static values across very large groups of actors, by having a separate document.

@cjslep
Copy link
Author

cjslep commented Jan 17, 2018

I fail to see how having these URIs whose resolutions are being cached requires them to be bundled within an endpoints JSON object. If an implementation is given a sharedInbox value of example.com/abc and is caching that response, the caching mechanism should not care whether sharedInbox came from an Actor object directly or an endpoints object. This sounds like a specific implementation's details leaking into the spec.

In fact, if the goal is to optimize for caching and deduplication, then I would modify my proposal to something along the lines of:

Remove embedding the endpoints JSON object at all and have it be a functional anyURI value. That way the server can aggressively fan-out-cache the endpoint URI's response and also that response's individual URIs. This would also save some amount of bytes on the wire since the full endpoints object is no longer being sent for every Actor object, just its URI reference.

@cjslep
Copy link
Author

cjslep commented Jan 17, 2018

Also, while I greatly appreciate the practicality, implementors that want to create libraries with the maximum chance of federating correctly with neighbors have to consider all the theoretical use-cases permitted by the specification or else risk being accused of not being compliant to the spec when a special-purpose application uses extreme edge cases permitted by the spec and tries to federate.

@nightpool
Copy link
Collaborator

nightpool commented Jan 22, 2018

I still think you're misunderstanding what i'm trying to say. from the spec:

This mapping may be nested inside the actor document as the value or may be a link to a JSON-LD document with these properties

that means that the Range of endpoints is Object | Link. Implementations that wish to have a shared endpoints value can use a URI, and implementations that don't can embed it. Implementations that care about payload size can fan-out-cache the link, and implementations that don't can embed the value.

Anyway, endpoints is a separate object entirely to provide that kind of caching you proposed. that is already allowed for in the spec and is the whole entire purpose of having it as a separate object.

@cjslep
Copy link
Author

cjslep commented Jan 23, 2018

As an aside, to address the typing point:

I don't read it as endpoints as having Object | Link range on an ActivityStream type. And I don't believe they are an ActivityPub Object either, since the specification takes care to be extra detailed (emphasis mine):

endpoint
A json object which maps additional (typically server/domain-wide) endpoints which may be useful either for this actor or someone referencing this actor. This mapping may be nested inside the actor document as the value or may be a link to a JSON-LD document with these properties.

However, I realize my interpretation may not the intended one. And this is an aside, anyhow.

Back to the meat of your point:

endpoints is a separate object entirely to provide that kind of caching you proposed. that is already allowed for in the spec and is the whole entire purpose of having it as a separate object.

Gotcha. Thanks for pointing that out. Unfortunately, re-reading past-me's comment, present-me is still not convinced this caching reasoning is sufficient to have endpoints be a separate object.

My core complaint that is unaddressed is that as written in the spec and as organized with its own properties, endpoints is not meaningful. I know caching was mentioned earlier so the following also tries to take that into account:

endpoints is nothing more than an arbitrary grouping of endpoints. Why not also move inbox, outbox and so on also into the endpoints object? Those "may be useful either for this actor or someone referencing this actor". In fact, why not just move everything that's an IRI on the actor into endpoints? That's what the word "endpoints" means, so why not back up its name with proper meaning? If "endpoints" isn't a good name, what name should it be? cacheableEndpoints? Or instead of trying to make 'endpoints' a meaningful property name, let's cut out the middle man and have actors have these properties as an obvious direct relationship and note in the spec that they are specially cacheable in some way beyond typical HTTP headers.

Or, an alternative option is to ditch the requirement for actors to need to reference endpoints at all, save all those bytes per actor, and have a well-known URI that federating servers can call on each other. (Can use HTTP headers or other mechanisms, I'm not trying to get lost in those details right now). This also has the added benefit of not requiring peers to obtain actors to cache these values. Which may be beneficial to discovery, I haven't thought through those implications yet. This option would promote endpoints to really mean something special besides its current "arbitrary collection of URIs that may change per actor".

As it is now, the "endpoints" object is incredibly unclear and problematic to me as an implementor:

  • It only has some arbitrary endpoints.
  • It is still related to an actor in some unspecified way, but
  • It is distinct enough from an actor that they're not properties of the actor itself.
  • It bloats every actor object's wire size, which partially defeats the purpose of caching.
  • It is not clear in the spec that its purpose is to make caching easy.
  • It is not clear in the spec if its sole purpose is to make caching easy.
  • It is not clear what type it is (JSON object, IRI to JSON-LD document, ActivityPub object).

Any resolution that address these above concerns, I am all ears for. Hopefully it is clearer now why I am still unconvinced despite my own past comments on caching being satisfied.

@gobengo
Copy link

gobengo commented May 3, 2023

However, the specification does not permit these properties to be directly populated directly on the Actor.

My interpretation is that the specification does permit these properties anywhere. i.e. there is no limit to their domain in the owl. But it is probably not very useful to put them on the actor, as other consumers likely won't look for it there.

@evanp
Copy link
Collaborator

evanp commented May 3, 2023

Although I think we appreciate the effort that went into this work, changing these properties from being expected in the endpoints property, to being expected in the actor itself, will be a difficult, backwards-incompatible effort for dozens of implementers.

The work required, and potential bugs and interoperability issues for a live protocol like AP, is not justified by the conceptual mismatch @cjslep has called out.

This would have been an interesting conversation to hold before the spec was published and implemented, but at this point, the effort of changing it is too high.

@evanp evanp closed this as completed May 3, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants