-
Notifications
You must be signed in to change notification settings - Fork 26
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
Actions with explicit target #154
base: master
Are you sure you want to change the base?
Conversation
I think of operations as descriptions of (HTTP) requests and actions as the abstract, semantic meaning they have. I think we discussed that before. I think mixing the two (as in sub-classing which means a single resource is both an action and an operation at the same time) will be confusing and lead to all sort of unintended side effects. Reviewed 2 of 2 files at r1. drafts/use-cases/5.1.creating-event-with-put.md, line 33 at r1 (raw file):
At some point we should discuss how to get rid of this. The client shouldn't need to care where such information goes. It should just be concerned with providing all the necessary data. In this case here, providing the event would be enough. In some cases, however, the relationship between the payload and IRI parameters isn't as straightforward though drafts/use-cases/5.1.creating-event-with-put.md, line 67 at r1 (raw file):
Hmm... we loose important information here. Would you see the two mechanisms to co-exist or, as this change suggests, that drafts/use-cases/7.searching-events.md, line 19 at r1 (raw file):
Do you suggest that clients treat values of drafts/use-cases/7.searching-events.md, line 22 at r1 (raw file):
drafts/use-cases/7.searching-events.md, line 25 at r1 (raw file):
You seem to assume that drafts/use-cases/7.searching-events.md, line 102 at r1 (raw file):
Let's make a better example by describing a real event. We could, e.g., describe a Hydra conference call here. Comments from Reviewable |
Review status: all files reviewed at latest revision, 6 unresolved discussions. drafts/use-cases/5.1.creating-event-with-put.md, line 33 at r1 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Makes sense. As I mentioned in comment of this PR, we might even consider to remove that pseudo-code here and gradually re-add it following Herakles.ts features development. drafts/use-cases/5.1.creating-event-with-put.md, line 67 at r1 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Do you mean that we would use named node with drafts/use-cases/7.searching-events.md, line 19 at r1 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
I think client could provide single interface for action with explicit target and operations with target implied by drafts/use-cases/7.searching-events.md, line 25 at r1 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
good catch! I'll add drafts/use-cases/7.searching-events.md, line 102 at r1 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
👍 Comments from Reviewable |
Review status: 1 of 2 files reviewed at latest revision, 6 unresolved discussions. drafts/use-cases/7.searching-events.md, line 22 at r1 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Done. drafts/use-cases/7.searching-events.md, line 25 at r1 (raw file): Previously, elf-pavlik (elf Pavlik) wrote…
Done. drafts/use-cases/7.searching-events.md, line 102 at r1 (raw file): Previously, elf-pavlik (elf Pavlik) wrote…
Done. Comments from Reviewable |
Here's a rough sketch of what I mentioned on the call earlier today: {
"@context": "/api/context.jsonld",
"@id": "/api/events",
"@type": "Collection",
...
"**action**": {
"@type": "schema:AddAction",
"title": "Add an event to this collection",
"**request**": {
"@type": "Operation",
"method": "PUT",
"expects": "schema:Event",
"target": {
"@type": "IriTemplate",
...
}
}
}
} I introduced What do you think about the general structure of such an approach (ignoring the naming of those new terms for the time being)? Review status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions. drafts/use-cases/5.1.creating-event-with-put.md, line 67 at r1 (raw file): Previously, elf-pavlik (elf Pavlik) wrote…
No, I meant that you removed Comments from Reviewable |
How would a similar example look like for target not being an IriTemplate? Simple I was also thinking from an API point of view. Currently, an operation is associated with owning resource (a resource that is a subject of the relation with operation) and the target URL of the operation is taken from that owning resource. It is somehow invoking an operation on that owning resource. Here we have an owning resource that has an action that points somewhere else. This somewhere else sometimes will be that owning resource, but it may be completely different. In an API, we would then invoke an action on an owning resource but actually it may be untrue (as a the target may be completely different one). This may lead to misunderstandings. Review status: 1 of 2 files reviewed at latest revision, 3 unresolved discussions. drafts/use-cases/5.1.creating-event-with-put.md, line 33 at r1 (raw file): Previously, elf-pavlik (elf Pavlik) wrote…
Yep, let's leave it for now as it seems unrelated to the clue of this PR. drafts/use-cases/7.searching-events.md, line 19 at r1 (raw file): Previously, elf-pavlik (elf Pavlik) wrote…
I'd introduce separate method, i.e. Comments from Reviewable |
Almost exactly the same:
The important part here is that the value of
Sure. That's how pretty much every HTML form out there works too. Why do you think this would lead to misunderstandings? Reviewed 1 of 1 files at r2. Comments from Reviewable |
Well, yes and no - page has an intermediate object that actually does that - the form. From the API point of view you don't submit the page, but the form - you invoke an action on the form and there is no confusion here. In our case an operation is owned by the resource and points to it. In case of the action - the action may not point to the owning resource. client.invoke(events.operations.first());
// vs
client.invoke(events.actions.first()); // <- collection owns the action, but the action may point to the collection Both looks very similar, but may have drastically different behaviors. Review status: all files reviewed at latest revision, 3 unresolved discussions. Comments from Reviewable |
What the difference between a HTML form and a Hydra Operation in your opinion?
s/page/resource/ Review status: all files reviewed at latest revision, 3 unresolved discussions. Comments from Reviewable |
If I understand the object within Review status: all files reviewed at latest revision, 3 unresolved discussions. Comments from Reviewable |
I like your snippets @lanthaler, I could update this PR to reflect it! If we would merge it at some point, the we would have to update all the other snippets to separate actions from handlers (operations).
I recall an old Activity Streams 2.0 related draft which never even made it to Workin Draft stage, I have a copy of it on http://elf-pavlik.github.io/w3c-socialwg-activitystreams/activitystreams2-actions.html it uses concept of actions and handlers. Since property names should describe meaning of the relation we could even consider names like handledWith (instead of request) and affords (instead of action). Also if we separate actions from handlers (operations). Resources would most likely reference actions directly via action / affords and possibly not reference handlers directly via operation / Review status: all files reviewed at latest revision, 3 unresolved discussions. drafts/use-cases/5.1.creating-event-with-put.md, line 67 at r1 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
I don't think any of current use cases exemplifies requirement for such 'direct lookups'. Maybe until someone proposes (via PR) a clear use case for such 'direct lookups', we will not try to deal with that requirement? Comments from Reviewable |
I quite like handledWith. I'm not a native speaker but handledBy sounds slightly better to me. Perhaps executedBy / executableBy or something similar would be even more intuitive.
I know that the term affordance is en vogue in API circles but I have yet to find a person outside this small community to understand it :-)
That would require developers to spell out the same URL twice, right? I guess having a few examples (perhaps in a separate markdown file for now) would help to move this discussion forward. Could you please amend the PR with what you had in mind? Thanks Review status: all files reviewed at latest revision, 3 unresolved discussions. Comments from Reviewable |
I think terms ending with Review status: all files reviewed at latest revision, 3 unresolved discussions. Comments from Reviewable |
If we take https://github.com/HydraCG/Specifications/blob/master/drafts/use-cases/6.updating-event.md#details it would look something like: {
"@context": "/api/context.jsonld",
"@id": "/api/events/1",
"@type": ["schema:Event"],
"action": [
{
"@type": "schema:ReplaceAction",
"title": "Update an existing event",
"handledWith": {
"@type": "Operation",
"method": "PUT",
"expects": "schema:Event",
"target": "/api/events/1"
}
}
]
}
One could frame it differently not to repeat Review status: all files reviewed at latest revision, 2 unresolved discussions. Comments from Reviewable |
I've separated action and operation in almost all snippets. I'll add commit for search via POST and use case 9 soon. Review status: 0 of 8 files reviewed at latest revision, 2 unresolved discussions. Comments from Reviewable |
Review status: 0 of 8 files reviewed at latest revision, 3 unresolved discussions. drafts/use-cases/1.1.security-considerations.md, line 35 at r3 (raw file):
I though it is the Comments from Reviewable |
@elf-pavlik any update on this? |
Review status: 0 of 8 files reviewed at latest revision, 6 unresolved discussions. drafts/use-cases/1.entry-point.md, line 94 at r3 (raw file):
Isn't it weird to have this id repeated here where it's the subject of drafts/use-cases/5.1.creating-event-with-put.md, line 67 at r1 (raw file): Previously, elf-pavlik (elf Pavlik) wrote…
Meaning we actually want to remove drafts/use-cases/5.1.creating-event-with-put.md, line 28 at r3 (raw file):
So this should be called drafts/use-cases/5.1.creating-event-with-put.md, line 33 at r3 (raw file):
So the expansion uses the variable name or the property? drafts/use-cases/7.searching-events.md, line 19 at r1 (raw file): Previously, alien-mcl (Karol Szczepański) wrote…
Ah yes, what I commented above. Would see it more like
Comments from Reviewable |
Reviewed 8 of 8 files at r3. drafts/use-cases/1.1.security-considerations.md, line 35 at r3 (raw file): Previously, alien-mcl (Karol Szczepański) wrote…
Adding it to the action wouldn't work as there might be multiple operations allowing you to perform the action. The use case at hand would be using different protocols like either sending an HTTP request or a GRPC request. drafts/use-cases/1.entry-point.md, line 94 at r3 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
For embedded actions/operations it is, yeah. But it may be fine as they would only be used rarely. @elf-pavlik could you please add an example illustrating how this would work in a drafts/use-cases/5.1.creating-event-with-put.md, line 67 at r1 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Yes. Pavlik already replaced it's need for PUTs here. We could do the same for GETs by adding the following action
drafts/use-cases/5.1.creating-event-with-put.md, line 28 at r3 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Or simply something like drafts/use-cases/5.1.creating-event-with-put.md, line 33 at r3 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
I agree with Pavlik that the application using Heracles should provide the property and not know anything about the variable name as it is an implementation detail that may change at any time. Comments from Reviewable |
Review status: all files reviewed at latest revision, 6 unresolved discussions. drafts/use-cases/1.1.security-considerations.md, line 35 at r3 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Hmm. This just makes the benefits from action event less tempting. If it just to add some semantic to an operation, I think I preferred typing an operation - this way you could have single operation marked with multiple semantics (i.e. same operation adds and creates collection items). BTW. How would you pick the operation matching a desired protocol? Comments from Reviewable |
My apologies everyone for staying gone offline for so long. We had real mess with changing to faster and more reliable broadband in our pretty remote home. I hope from now on we should enjoy stable connection. I'd like to point a major problem this PR tries to solve, if we look at pseudo code of current use cases: Currently client needs to have prior knowledge of which one of those server uses and write different code based on that prior knowledge. IMO client shouldn't have to do that and same code should handle both of those cases. Separating the semantic action which client intends to perform, from operation which provides server specific details of how to perform (handle) that action looks like promising approach. I would propose to focus first on the part of this pull request related to just those two use cases UC5 & UC5.1 While I wanted to avoid getting stuck in details of the pseudo code, now I think that if we want to have same code handling both cases, it might make sense to take a moment to agree on that pseudo code or even take a small detour to try implement in in Heracles.ts currently common part of pseud code in UC5 & UC5.1 var event = {
"@context": "/api/context.jsonld",
"@type": "schema:Event",
"eventName": "My brand new event",
"eventDescription": "Hope it will work",
"startDate": "2017-04-19",
"endDate": "2017-04-19"
};
var client = new HydraClient(); UC5 specific var operation = client.get("http://example.com")
.getApiDocumentation()
.getEntryPoint()
.getCollection({
property: "http://www.w3.org/1999/02/22-rdf-syntax-ns#type",
object: "http://schema.org/Event"
})
.getOperationOfType('http://schema.org/CreateAction');
client.invoke(operation, event); UC5.1 specific var templateVariables = {
slug: [
'meeting',
'with-will'
]
};
var memberTemplate = client.get('http://example.com/events').memberTemplate;
var operation = memberTemplate
.getOperationsOfType('http://schema.org/CreateAction')
.thatExpect('http://schema.org/Event')
.first();
const operationWithTargetExpanded = operation.expandTarget(templateVariables);
client.invoke(operationWithTargetExpanded, event); Since in UC5.1 mapping for IriTemplate includes {
"@type": "IriTemplateMapping",
"variable": "slug",
"property": "schema:name",
"required": true
} and event itself includes value for eventName property (mapped in JSON-LD context to schema:name), i think pseudo code above doesn't need that Given above we could combine snippets specific to UC5 and UC5.1 into something like const collection = client.get("http://example.com")
.getApiDocumentation()
.getEntryPoint()
.getCollection({
property: "http://www.w3.org/1999/02/22-rdf-syntax-ns#type",
object: "http://schema.org/Event"
})
const operation = collection.getOperationForAction('http://schema.org/CreateAction')
.thatExpect('http://schema.org/Event')
.first();
client.invoke(operationWithTargetExpanded, event); Generic Hydra client (like Herakles.ts) can most likely implement that relying internally on memberTemplate, doing some magic under the hood to streamline discovery of operations directly referenced from the collection and those referenced from template which collection references with memberTemplate. In my opinion this would take the raw JSON structure of the collection further apart from the interface provided by the client. Taking approach proposed in this PR for me keeps the JSON representation and interface provided by generic hydra client more aligned. In next days I will regularly respond to all discussions on this PR to have it moving forward before our next telecon! Review status: all files reviewed at latest revision, 6 unresolved discussions. Comments from Reviewable |
Well - in my opinion actions won't resolve that issue you've pointed. Action does not tell the client on how to build a resulting request - which details should go into the IRI template and which into the body. Indeed, client shouldn't need prior knowledge telling him to expand the operation with details or not, but I'm worried that there will be situations when this snipped: client.invoke(operation, event); will be enough - operation would have an IRI template that expects variables provided from outside of the I believe we've touched an issue we had a very brief chat on before - operation (or action, doesn't matter) is neither only it's body nor it's URL(template) - it's both (or even more for other protocols). I aknowledge actions to give some semantic hints to the client - in that scenario operation would be a mere request description, but both still lack a complete knowledge of how the request needs to be created. Review status: all files reviewed at latest revision, 6 unresolved discussions. Comments from Reviewable |
I do not agree. With a URI template I don't think we should assume that the mappings are derived from the actual request representation. They could and the actual event would be used to fill in the template var newEvent = {};
/* ... */
const operationWithTargetExpanded = operation.expandTarget(newEvent);
client.invoke(operationWithTargetExpanded, newEvent); But from the client (library) perspective I think these should be kept separate. Review status: all files reviewed at latest revision, 8 unresolved discussions. drafts/use-cases/0.intro.md, line 61 at r3 (raw file):
So we want this to be drafts/use-cases/1.1.security-considerations.md, line 35 at r3 (raw file):
There is one good benefit. Instead of minting properties like
I'd say this is outside of the scope of the API description. drafts/use-cases/1.entry-point.md, line 94 at r3 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Oh yeah, this was bugging me. In general we have symmetry between ApiDocumentation and inline affordances
Actions looked to me like an inline-only feature... drafts/use-cases/5.1.creating-event-with-put.md, line 67 at r1 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Good one! Another reason to like drafts/use-cases/5.1.creating-event-with-put.md, line 28 at r3 (raw file):
👍 drafts/use-cases/5.1.creating-event-with-put.md, line 33 at r3 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Well, both can change 🙂. But neither is out-of-bounds so there is not issue with allowing either. After all this information comes from the template mappings. Still, I don't understand why two value concatenated into one string. What if one was to have spaces? :) And full URL should be used (or possibly proper JSON-LD) {
"http://schema.org/name": [
{ "@value": "meeting" },[
{ "@value": "with will" }
]
} drafts/use-cases/7.searching-events.md, line 19 at r1 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Actually, I like Markus' suggestion above better. drafts/use-cases/7.searching-events.md, line 72 at r3 (raw file):
Yes! Kill it, kill it 🔥 Why not just remove Comments from Reviewable |
Review status: all files reviewed at latest revision, 8 unresolved discussions. drafts/use-cases/0.intro.md, line 61 at r3 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Also, I notice that there is a schema:target too. This can be confusing drafts/use-cases/7.searching-events.md, line 19 at r1 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Sorry about going back and forth but it occurred to me that maybe the client may be interested in properties of the action (a human description?). Going straight for the operation can be unintuitive Comments from Reviewable |
I agree with everything Pavlik said. The design might still need a bit of work though (some of which may be out of scope for this PR like a more expressive description of what data an action/operation client.usingApi("http://example.com/")
.findCollectionWith({
property: "http://www.w3.org/1999/02/22-rdf-syntax-ns#type",
object: "http://schema.org/Event"
})
.invoke('http://schema.org/CreateAction')
.whichExpects('http://schema.org/Event')
.withData(event)
.getResult(); We should keep an eye on the verbosity the split of operations into actions and operations entails. Review status: all files reviewed at latest revision, 8 unresolved discussions. Comments from Reviewable |
API changes, even drastic are not the issue here. These are relatively easy to implement, including the example above. .withData(event) Neither suggested actions nor current operations are not oriented in that direction. Review status: all files reviewed at latest revision, 8 unresolved discussions. Comments from Reviewable |
Review status: all files reviewed at latest revision, 8 unresolved discussions. drafts/use-cases/0.intro.md, line 61 at r3 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
In latest version of this PR drafts/use-cases/5.1.creating-event-with-put.md, line 33 at r3 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
If server uses some value to compose IRI it probably has to URL encode that value drafts/use-cases/7.searching-events.md, line 72 at r3 (raw file): Previously, tpluscode (Tomasz Pluskiewicz) wrote…
to remove Comments from Reviewable |
Review status: all files reviewed at latest revision, 8 unresolved discussions. drafts/use-cases/7.searching-events.md, line 72 at r3 (raw file): Previously, elf-pavlik (elf Pavlik) wrote…
In Heracles.ts there is already an integration test that uses Comments from Reviewable |
Yeah, we would need to define that. @elf-pavlik do you have time do another pass over this, address the feedback and add the examples that have been asked for? Review status: all files reviewed at latest revision, 6 unresolved discussions. Comments from Reviewable |
Review status: all files reviewed at latest revision, 5 unresolved discussions. drafts/use-cases/5.1.creating-event-with-put.md, line 33 at r3 (raw file): Previously, elf-pavlik (elf Pavlik) wrote…
This is beside the point and defined in the Comments from Reviewable |
fa23516
to
9e3402c
Compare
I haven't found time in last two weeks to properly consider UC9 Adding existing resources to collections, and other cases where we just want to create relation (link) two resources. Review status: 7 of 9 files reviewed at latest revision, 5 unresolved discussions. Comments from Reviewable |
Last commit provides new take on UC9, renaming it to "Creating relations between existing resources". It introduces Review status: 7 of 11 files reviewed at latest revision, 5 unresolved discussions. Comments from Reviewable |
Reviewed 2 of 2 files at r4, 2 of 2 files at r5. drafts/use-cases/5.1.creating-event-with-put.md, line 30 at r5 (raw file):
Indentation is slightly off. That being said, I love this interface. drafts/use-cases/9.creating-relations-between-existing-resources.md, line 130 at r5 (raw file):
Please make this valid JSON-LD, i.e., wrap it in a curly brackets Comments from Reviewable |
Review status: all files reviewed at latest revision, 7 unresolved discussions. drafts/use-cases/5.1.creating-event-with-put.md, line 30 at r5 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Indentation fixed! drafts/use-cases/9.creating-relations-between-existing-resources.md, line 130 at r5 (raw file): Previously, lanthaler (Markus Lanthaler) wrote…
Done. Comments from Reviewable |
Reviewed 7 of 8 files at r3, 1 of 2 files at r5, 3 of 3 files at r6. Comments from Reviewable |
This change now. Please raise any remaining concerns you may have. We'll then discuss it in our telecon in two weeks (Pavlik and I are out tomorrow) and decide how to proceed. Review status: all files reviewed at latest revision, 4 unresolved discussions. Comments from Reviewable |
Do we have a consensu here? This PR updates existing use cases with new features. We'll have to update the reference client implementation and spec sooner or later to make everything fit altogether. Review status: all files reviewed at latest revision, 4 unresolved discussions. Comments from Reviewable |
Have we progressed on concensus on this? |
To revive this PR, let me attach a diagram of how do I see this matter. In general, I feel that additional predicate
|
sorry everyone for staying silent in last months! @alien-mcl I will study your diagram in next days. I also have another use case for 'append only' dataset where updating a resource will require creating new one using specified iri template. As long as we have explicit target it should work. |
It's great to have you back @elf-pavlik! As for the use cases, we're going to create a gitbook with use cases and examples so have that one you mention prepared somewhere |
obviously NOT READY TO MERGE
This PR serves as starting point for considering actions/operations with explicit target, I reuse terms from
schema:
namespace - schema:potentialAction & schema:target just because they seem to have same purpose and we already use other terms from that namespace.I would also suggest not to focus on the client pseudo code and update it with Heracles.ts based code as the reference implementation of the client progresses.
As discussed in on of the previous issues, it seems that we have something like:
so more specific
hydra:operation
implies the target of referencedhydra:Operation
, whileschema:potentialAction
leaves it toschema:Action
to make the target explicit viaschema:target
.This change is