go-fed / activity

ActivityStreams & ActivityPub in golang, oh my!
BSD 3-Clause "New" or "Revised" License
711 stars 111 forks source link

Extending Callbacker #71

Closed ghost closed 6 years ago

ghost commented 6 years ago

The Callbacker interface is very useful. It allows separating application specific logic. Unfortunately, not all valid activity types trigger Callbacker. An example would be a Join activity written to the Outbox.

It would be nice if Callbacker could support additional activity types, namely all the types listed in https://www.w3.org/TR/activitystreams-vocabulary/#activity-types

cjslep commented 6 years ago

Hah, Callbacker was one of the first interfaces I tried to abstract out. At the time, I wasn't sure of all the Activity types that would have had default behaviors provided in pub, so I wanted to capture the absolute bare minimum without forcing apps to write things like:

// Ignore all these ...
func (*myCallbacker) Offer(c context.Context, s *streams.Offer) error { return nil }
func (*myCallbacker) Listen(c context.Context, s *streams.Offer) error { return nil }
// etc for all types ignored...

Two ways to extend this further that I've thought of so far (could be more out there):

  1. Take in a user-defined Resolver for the rest of the callbacks, and use it.
  2. Define a ton of callbackerExtensionOffer, callbackerExtensionListen, etc and see if a user's Callbacker has additional methods to call. This would need to leverage additional code generation.

Pros of the first option: May scale with new vocabularies easier (yay for me). Cons of the first option: App logic is split between Callbacker and Resolver. Resolver may not be as familiar due to being omitted from tutorial, it's going to change a lot in 1.0.0, etc (app writers pay heavy price).

Pros of the second option: App just implements another similar method on their Callbacker type (yay for app writers). Cons of the second option: May require more thought on how to scale to multiple vocabularies in 1.0.0 (woe is me).

Between the two, the second one sounds like the winner in the long haul.

ghost commented 6 years ago

I think its reasonable to force apps to write things like

// Ignore all these ...
func (*myCallbacker) Offer(c context.Context, s *streams.Offer) error { return nil }
func (*myCallbacker) Listen(c context.Context, s *streams.Offer) error { return nil }
// etc for all types ignored...

It is a one-time setup cost for app writers.

cjslep commented 6 years ago

I definitely value knowing where you stand, It's a great signal for me as to whether it is reasonable or not. At the time, alone in a snowed-in apartment room, I had no idea how (un)reasonable I was being.

However, I want to tread especially carefully before drawing the line of reasonable-ness here, especially knowing that multi-vocabulary support is on the horizon. I want to be cognizant of any additional "throwaway" migration work I add in 0.3.0 (Ideally, I'm thinking 1.0.0 would follow 0.3.X). Because I feel like these empty definitions would quickly become obsolete in proper/clean support of extended vocabularies, and I value saving X number of people this level of cognitive overhead if it means a slight increased burden just for myself. Although there would need to be some discoverability aids and clear documentation if it goes the implied/typecasted interface route, for the Y number of people (<=X) who need it.

cjslep commented 6 years ago

1252727ca8f324a560e4a65683563538b5b2e7b5 and 797b2f79a848ed215572db8bc426a668f1b3f0f0

See CHANGELOG for description on how to use this, until I can update the go-fed site.