devnull:I’m happy to roll back the changeI think, on balance, this may make sense.
-
to: @devnullcc: @angus
---
Any idea what's going on here? It seems to be a different bug than the one where Discourse generates duplicate posts for a Create that later gets Announced back at it.
-
Thank you all for taking care of this!
As you can see I'm not super responsive here. Maybe we should figure a quick line for sysadmins.
-
@devnull @how @trwnh
Ok, I finally had the time to recreate this case in a spec.
discourse-activity-pub/spec/integration/integration_cases_spec.rb at 057e1ce983822bfdd433cc6d8c1fc4fb55a2de71 · angusmcleod/discourse-activity-pub
Adds ActivityPub support to Discourse. Contribute to angusmcleod/discourse-activity-pub development by creating an account on GitHub.
GitHub (github.com)
The reason this happened is because:
- Discourse was unable to resolve a
Conversation
type forcontext
. - The error thrown in
1
was too nested to properly roll back the relevant transactions.
This will address the immediate issue
Fix handling of failure to resolve context by angusmcleod · Pull Request #157 · discourse/discourse-activity-pub
Context: https://socialhub.activitypub.rocks/t/a-single-post-is-somehow-creating-multiple-topics-with-no-replies/4899 @pmusaraj I think certain cases are sufficiently complex that they are better s...
GitHub (github.com)
@devnull I haven't been keeping up with the latest on context / collections etc. Could you briefly explain the thinking behind using "Conversation" instead of "OrderedCollection" in
context
. Is it meant to be a type of collection..or? - Discourse was unable to resolve a
-
devnull:
Implementing the
Conversation
-type object allowed me to further my own agenda of being able to explicitly pull in topics by the context URL (which is what people would undoubtedly try to do) instead of by individual post URLs.That makes your implementation incompatible with others, because
context
is expected to be a collection.If you need to create a link between post and a topic, you can use a different property (
topic
?). -
angus:
@devnull I haven’t been keeping up with the latest on context / collections etc. Could you briefly explain the thinking behind using “Conversation” instead of “OrderedCollection” in
context
. Is it meant to be a type of collection…or?sort of... indirectly... basically here's what led up to this:
- about 2 weeks ago, i realized a quirk of ActivityPub would make it impossible to Follow a Collection without potentially undesirable and unavoidable side effects, because giving a Collection an inbox and addressing it directly would not only attempt delivery to the Collection inbox, it would also attempt delivery to any inboxes of every single item in the collection! this happens only for Collection and its subtypes, and is a MUST in the spec. of course, a Follow on a Collection is not necessarily relevant to every item in that collection... but there's no way to address only the collection or only its items. the way addressing and delivery are defined, both will necessarily happen. the full discussion of the issue is ongoing, but it can be found here: https://github.com/w3c/activitypub/issues/486
- in passing, i mentioned the above issue to @devnull as a reason that context maybe shouldn't be directly a Collection, but instead be its own Object that has a defined relationship with a canonical Collection. we could call this contextual object a
Conversation
and we could refer to the canonical collection via aposts
relation/property. this mirrors some other data models and is one potential solution for the weirdness of following a conversation represented by a Collection -- basically make it something other than a Collection (so it can be addressed as an individual without also addressing its members), or otherwise extend Follow to be sent to theattributedTo
actor if any (which is not a well defined pattern, unnecessarily introduces more indirection and more complexity in what an actor might receive, and could be recursive). in general, i am inclined to say that it's easier to just change the type of the context object rather than to change the way follows work and introduce following non-actor objects. i am not sure about anything yet, and this whole issue requires a bit more thought. - based on the things i said in that private discussion, @devnull went ahead and committed a change that introduced Conversation and Conversation.posts along the lines of one of the potential solutions. i consider this premature, but hey, it led to this bug being discovered in the Discourse plugin, so i guess it wasn't all bad!
-
Implementing the
Conversation
-type object allowed me to further my own agenda of being able to explicitly pull in topics by the context URL (which is what people would undoubtedly try to do) instead of by individual post URLs.It was not a large change, and should the spec shift it should be fairly easy to keep up — at least now you know what it looks like in practice as well as in theory!
-
Thanks for the explanations guys. I'll add
Conversation
support to the Discourse plugin when I get a spare mo this week. -
It seems to be getting worse, about ~15 identical topics have been created today.
And I still don't understand why this change was made. Why break our past agreement about context being a collection, @devnull ?
Collection
worked for everyone.Object
adds an unnecessary layer of indirection, and doesn't work for blogging platforms where "topics" may not exist.I think
context
should remain a collection. Other features like "topics" and subscribing to conversations should be implemented independently. I mentioned a possible solution for the latter problem on GitHub, and will describe it in more detail here:context
property points to a collection- top-level post and/or context collection may also have an
observer
property, which points to an actor - this actor can be followed to receive conversation updates
- it can also represent a topic
Example:
{ "type": "Note" "context": { "type": "Collection", "items": [] }, "observer": { "type": "Application", "inbox": "..." }}
What do you think?
There's no intermediary between post and conversation collection. All components can be implemented independently. Only one new property. Compatible with ActivityPub spec.
-
@angus @silverpill I'm happy to roll back the change, especially if it's causing issues with other software or expectations with where we'd eventually end up.
It's in a v4.1 branch that is as yet unreleased.
-
Switching to my NodeBB account to continue the discussion.
The change has been rolled back now.
@[email protected] when you say one shouldn't rely on
context.type
, why is that?When an object reports a
context
, I resolve it and check its' type to ensure it is Collection-ish.When someone on NodeBB tries to pull a context directly into NodeBB, I check for
context.type
(again, that it is Collection-ish) before trying to load posts.Both those checks seem ok. Admittedly someone could try pulling a followers collection in, but NodeBB would just see that they're not posts and find nothing to pull in.
So what's the problem? Ideally we have some sort of explicit declaration, of course.