-
Notifications
You must be signed in to change notification settings - Fork 96
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
Public activity streams #1090
Public activity streams #1090
Conversation
return callback({'code': 400, 'msg': 'Loggedin activity streams are only available for user or group resources'}); | ||
} | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, the design vision that was in mind for public / loggedin streams was a little more "under the hood" than this. Specifically, for a given activity stream type activity
, it could have public
, loggedin
and private
permutations (e.g., activity#public
, activity#loggedin
and activity
for the private one), without having to register a new activity stream type and put custom logic in the router for each one.
Once you can determine the "maximum visibility" of an activity based on its entities, you can determine which visibility bucket(s) to drop the activity into. If it is public, it would get dropped into all of public, loggedin and private streams. If it is loggedin, it goes into loggedin and private. If it is private, then private only.
Then when requesting the "activity" stream, the activity API would automatically determine which visibility you get based on your relation to the group/user/content, etc... This pattern is very similar to how library's work with their visibility buckets and then determining the target library visibility on request.
After this, there may actually be no concept of "access denied" for accessing an activity stream (then again, there could still be), because you could always just get the public stream.
There are performance implications. As a performance constraint, multi-bucket activities could potentially just be "enabled" for a stream type, in which case we'd only enable it for activity
streams, which should then be performance-wise equivalent to this solution.
fc92852
to
d04b80b
Compare
So initially I didn't go that way as I couldn't really figure out a way how to do all of that without significantly refactoring the activity router and/or activity stream registration pattern. Turns out that it wasn't really that bad. In the new approach, when you register an activity stream type (like I've re-used the library authz logic to determine when to retrieve which stream, this actually works really well as the authz scheme is completely the same. When you do a request to
Any feedback you would have on the new mechanisme would be greatly appreciated |
d04b80b
to
49c60db
Compare
@@ -130,6 +134,7 @@ OAE.registerPreShutdownHandler('oae-activity', null, function(callback) { | |||
* @param {Object} [options.push] A set of options for modifying the behaviour of push notifications on the stream | |||
* @param {Object} [options.push.delivery] Indicates at which activity delivery phase(s) activities should be pushed to the client | |||
* @param {String} [options.push.delivery.phase] Indicates when a single activity should be pushed to the client. One of `routing` or `aggregation`. Defaults to `routing` | |||
* @param {Boolean} [options.visibilityBucketing] If `true`, activities with only public entities will also be routed to `<activityStreamType>-public` and similarly for loggedin entities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly for logged in
isn't entirely true as the loggedin stream will contain items with both public and loggedin entities
Looks really good from a code and functional perspective. Added some minor comments. Back to @simong for follow-up |
Followed up. Back to @nicolaasmatthijs |
Changes Unknown when pulling 6e2be4c on simong:public-activity-streams into * on oaeproject:master*. |
Looks good. Merged |
No description provided.