search

HydraCG / Specifications

Specifications created by the Hydra W3C Community Group
Other
139 stars 25 forks source link

Add Use Case: Creating events indirectly (ie. with PUT) #143

Closed tpluscode closed 6 years ago

tpluscode commented 6 years ago

In this PR I followed the structure we discussed on last call and in issues #3, #118 and #141

I also took the liberty of defining some additional client API elements which allow me to propose mixing an IriTemplate and Operation in a single request. Please see PR comments.

This change is Reviewable

elf-pavlik commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

    "totalItems": 0,
    "members": [ ],
    "http://example.com/vocab/createEvent": {

the manages block already signals that this collection 'contains' instances of schema:Event, and to create we already rely on schema:CreateAction, if we take approach similar to hydra:search I would prefer to define very generic hydra:add which would link to a hydra:Resource or 'hydra:IriTemplate`

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 116 at r1 (raw file): I would myself include an @id even when creating a new resource, since client decides the IRI. To recognize Create vs. Update one should probably do conditional request and use If-Match HTTP header https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Match

The If-Match HTTP request header makes the request conditional. For GET and HEAD methods, the server will send back the requested resource only if it matches one of the listed ETags. For PUT and other non-safe methods, it will only upload the resource in this case.

Comments from Reviewable

elf-pavlik commented 6 years ago

@tpluscode you have to resolve IPR issue with W3C system https://labs.w3.org/hatchery/repo-manager/pr/id/HydraCG/Specifications/143

tpluscode commented 6 years ago

@elf-pavlik I know, but how do I create the w3c account to link to GitHub?

tpluscode commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 58 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…
I somehow have some doubts about this `manages` construct. What does it say? Are all resources of type `schema:Event` members of this collection? Or should I add each member explicitly? I believe both cases are pretty legal - server could expose on-the-fly views acting as collections just by using some of the resource's attributes (i.e. collection of wines where each resource is of type `foo:Wine`), but it also could expose explicit collections (i.e. my likes that I've added manually).

Interesting points, but I think they are outside of the scope of this PR. Best let's continue this discussion in a new issue or where we discuss the manages block. I merely copied the collection from Use Case 5.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
the `manages` block already signals that this collection 'contains' instances of `schema:Event`, and to create we already rely on `schema:CreateAction`, if we take approach similar to `hydra:search` I would prefer to define very generic `hydra:add` which would link to a `hydra:Resource` or 'hydra:IriTemplate`

Sure, I can agree with either. I used an API-specific property here because where I previously minted something in the hydra namespace and I thought it caused confusion :).

On the other hand, similar method can be employed to create resource outside the context of collections. I can think of an optional linked resource which can be created once but is not part of any collection. Would you also propose a generic hydra term for such uses?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 78 at r1 (raw file):

that just adds a resource that already exists

I think that's in scope of Use Case 9. Let's discuss there. Here I wanted to add an alternative to creating new resources (see UC 5).

In fact I'll just add in the considerations section a note that this method can be used to create resource outside of a collection context.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 116 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
I would myself include an `@id` even when creating a new resource, since client decides the IRI. To recognize Create vs. Update one should probably do conditional request and use `If-Match` HTTP header https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Match > The `If-Match` HTTP request header makes the request conditional. For GET and HEAD methods, the server will send back the requested resource only if it matches one of the listed ETags. For PUT and other non-safe methods, it will only upload the resource in this case.

Sounds good. @asbjornu, any thoughts about that? You are more knowledgeable in the nuances of HTTP.

Comments from Reviewable

tpluscode commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

var operation = client.get("http://example.com/events")
    .getOperationOfType('http://schema.org/CreateAction', /* recursive */ true)
    .filter(operation => operation.expects === 'http://schema.org/Event');

@alien-mcl See how I added the second recursive parameter. The indention is to search of operations in any linked resources.

Also, is the native filter something you would do? Also, I realised now that it would return an Array. Should add indexer here...

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 36 at r1 (raw file):

    .filter(operation => operation.expects === 'http://schema.org/Event');

client.invoke(operation, event, templateVariables);

@alien-mcl Here you see the second change where I added an optional parameter with template variables. Initially I was considering to have only one model - event and template var merged but I concluded it would break Single Responsibility in a way.

Comments from Reviewable

elf-pavlik commented 6 years ago

I know, but how do I create the w3c account to link to GitHub?

You should follow this option on https://labs.w3.org/hatchery/repo-manager/pr/id/HydraCG/Specifications/143

if the said contributor is a member of Hydra Community Group, they should link their W3C and github accounts together.

And login with your W3C account credentials (same ones that you use to log in on https://www.w3.org/community/hydra/wp-login.php

elf-pavlik commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 58 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Interesting points, but I think they are outside of the scope of this PR. Best let's continue this discussion in a new issue or where we discuss the manages block. I merely copied the collection from Use Case 5.

I agree that this PR does NOT introduce manages block here, I added it via https://github.com/HydraCG/Specifications/pull/132 and if we want to change it we should start dedicated Issue or PR for it

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Sure, I can agree with either. I used an API-specific property here because where I previously minted something in the `hydra` namespace and I thought it caused confusion :). On the other hand, similar method can be employed to create resource outside the context of collections. I can think of an optional linked resource which can be created once but is not part of any collection. Would you also propose a generic `hydra` term for such uses?

If you have some other specific Use Case, maybe you could describe it in depth, either in this PR, one of the open Issues or new PR. As I understand this Use Case we want to create a new resource of type schema:Event and have it appear as a member in this collection.

Comments from Reviewable

lanthaler commented 6 years ago

I connected your W3C account to your GitHub account for you.

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 1 at r1 (raw file):

## Creating new event indirectly

Why do you call it "indirectly"? What's indirect about it?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 6 at r1 (raw file):


As an API publisher
I want allow consumers to construct event's identifier

event's identifier -> event identifiers?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 7 at r1 (raw file):

As an API publisher
I want allow consumers to construct event's identifier
So that they can control the .

control what? The identifier? The URL?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 10 at r1 (raw file):


As an API consumer
I want to be able to create new resources of a given type

... controlling parts of the URL (I think that's the point here, isn't it)?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
@alien-mcl See how I added the second `recursive` parameter. The indention is to search of operations in any linked resources. Also, is the native `filter` something you would do? Also, I realised now that it would return an Array. Should add indexer here...

Hmm... that recursive parameter seems to open a can of worms. When should a client stop searching for the operation? It's easy to come up with an example that would crawl all of Wiki-/DBpedia

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 36 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
@alien-mcl Here you see the second change where I added an optional parameter with template variables. Initially I was considering to have only one model - event and template var merged but I concluded it would break Single Responsibility in a way.

An alternative that might be cleaner would be to first resolve the target of the operation to transform an abstract operation into a concrete one. Expand is the name the URL Template spec uses for that process.

operation.expandTarget(templateVariables);
client.invoke(operation, event);

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 42 at r1 (raw file):


To create an `Event` using `PUT`, the collection resource has to be linked to the actual target of the operation.
This linked object can be a concrete resource or an `IriTemplate` as presented in the above snippet. In this case

If it would be a concrete resource, it wouldn't create an Event, but replace that resource.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 58 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
I agree that this PR does NOT introduce `manages` block here, I added it via https://github.com/HydraCG/Specifications/pull/132 and if we want to change it we should start dedicated Issue or PR for it

+1.. but to answer the question: manages as currently defined only says that all members of the collection in this case are of type Event (nothing more and nothing less). I agree that we should find a better name for this.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
If you have some other specific Use Case, maybe you could describe it in depth, either in this PR, one of the open Issues or new PR. As I understand this Use Case we want to *create* a new resource of type *schema:Event* and have it appear as a member in this collection.

What are the semantics that you want to express here? Can the client expect the created resource to be added to the collection? In other words, what's the relationship of the collection to the IriTemplate and the Operation? If the resource will become a member, something like a hydra:memberTemplate might be preferable (see also https://github.com/HydraCG/Specifications/issues/16)

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 72 at r1 (raw file):

                "@type": "hydra:IriTemplateMapping",
                "variable": "slug",
                "property": "xsd:String",

This is a datatype, not a property. It should be something like schema:name or example:slug

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 97 at r1 (raw file):

``` json
{
    "@context": "/api/context.jsonld",

This is problematic. What is this URL relative to? How should the server receiving it interpret it (and thus the rest of the document)?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 116 at r1 (raw file): Not this specification but it would be a violation of the HTTP specification:

The PUT method requests that the state of the target resource be created or replaced with the state defined by the representation enclosed in the request message payload. A successful PUT of a given representation would suggest that a subsequent GET on that same target resource will result in an equivalent representation being sent in a 200 (OK) response. However, there is no guarantee that such a state change will be observable, since the target resource might be acted upon by other user agents in parallel, or might be subject to dynamic processing by the origin server, before any subsequent GET is received. A successful response only implies that the user agent's intent was achieved at the time of its processing by the origin server. [...] The fundamental difference between the POST and PUT methods is highlighted by the different intent for the enclosed representation. The target resource in a POST request is intended to handle the enclosed representation according to the resource's own semantics, whereas the enclosed representation in a PUT request is defined as replacing the state of the target resource. Hence, the intent of PUT is idempotent and visible to intermediaries, even though the exact effect is only known by the origin server.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 124 at r1 (raw file):


Even though the `schema:CreateAction` operation exists within the collection's representation it is not right to assume that
the newly created event will become part of the collection, ie. it's `"hydra:member"` property. What's more, a single

So, why is it then associated to the Collection at all?

Comments from Reviewable

lanthaler commented 6 years ago

Reviewed 1 of 1 files at r1. Review status: all files reviewed at latest revision, 15 unresolved discussions.

Comments from Reviewable

elf-pavlik commented 6 years ago

Review status: all files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 42 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
If it would be a concrete resource, it wouldn't create an Event, but replace that resource.

In one of the issues, we discussed that server could provide to client concrete IRI for new resource, so the resource doesn't exist yet and GET on that IRI provided by the server would result in 404 https://github.com/HydraCG/Specifications/issues/134#issuecomment-335028831

Comments from Reviewable

tpluscode commented 6 years ago

Review status: all files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 1 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
Why do you call it "indirectly"? What's indirect about it?

Good point. Actually now that I think of it, it's really the opposite: event is created directly (rather then by posting it to the collection).

Or should I just call this "Creating with PUT" and not be fancy?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 6 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
`event's identifier` -> event identifiers?

Done.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 7 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
control what? The identifier? The URL?

I guess I wasn't sure myself and forgot about it. The URL sounds good?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 10 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
... controlling parts of the URL (I think that's the point here, isn't it)?

Yes, part of. It's that and actually using doing PUT to the new resource itself. After all the server could link to concrete resource to PUT. Such as by minting a different URL for every GET. This is actually a good practice because let's the client take advantage of idempotency.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
Hmm... that recursive parameter seems to open a can of worms. When should a client stop searching for the operation? It's easy to come up with an example that would crawl all of Wiki-/DBpedia

Well, it would only traverse the representation. Not go over the wire and dereference resources.

But your point is still valid, but, what would you propose as alternative?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 36 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
An alternative that might be cleaner would be to first resolve the target of the operation to transform an abstract operation into a concrete one. [Expand is the name the URL Template spec uses for that process](https://tools.ietf.org/html/rfc6570#section-3). ```js operation.expandTarget(templateVariables); client.invoke(operation, event); ```

I like that. Only I would make this more functional instead of mutating the operation

const operationTargetExpanded = operation.expandTarget(templateVariables);
client.invoke(operationTargetExpanded, event);

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 42 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
In one of the issues, we discussed that server could provide to client concrete IRI for new resource, so the resource doesn't exist yet and GET on that IRI provided by the server would result in 404 https://github.com/HydraCG/Specifications/issues/134#issuecomment-335028831

As an example think of a discussion thread where the server offers random URIs for replies. This way the client can safely PUT multiple times without actually posting multiple comments.

So like @elf-pavlik says, if a resource doesn't exist yet is the client technically replacing or creating it? I think it's still creating. You can create resources but there is no representation.

I rephrased this sentence by the way.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
What are the semantics that you want to express here? Can the client expect the created resource to be added to the collection? In other words, what's the relationship of the collection to the IriTemplate and the Operation? If the resource will become a member, something like a `hydra:memberTemplate` might be preferable (see also https://github.com/HydraCG/Specifications/issues/16)

hydra:memberTemplate almost works but again, the object of this property doesn't have to be an IriTemplate at all.

@elf-pavlik here's an example:

  1. You register on a website
  2. The server creates a private profile for you, say, /user/some-guid
  3. You get a link to /user/{public-user-name} (or /user/some-guid/public if it wasn't to be a template)
  4. You PUT to that resource to create a publicly visible profile resource

So you link to a resource which doesn't initially exist and it's not part of any collection. But the general structure of your hypermedia is the same. I think that Stack Overflow follows a similar pattern.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 72 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
This is a datatype, not a property. It should be something like `schema:name` or `example:slug`

Of course, my bad.

And what if property was removed from a template mapping? Is that legal?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 97 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
This is problematic. What is this URL relative to? How should the server receiving it interpret it (and thus the rest of the document)?

I actually copied this from UC5 and probably similar in other use cases. Let's discuss separately? @alien-mcl?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 124 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
So, why is it then associated to the Collection at all?

I'm just saying that there isn't enough information to go on until we come up with more meta data for operations. We also haven't defined any concrete semantics here AFAIK and thus the client cannot assume anything which is not explicitly stated in the above representation.

Comments from Reviewable

elf-pavlik commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Well, it would only traverse the representation. Not go over the wire and dereference resources. But your point is still valid, but, what would you propose as alternative?

If client doesn't use anywhere proposed by you http://example.com/vocab/createEvent why even bother with introducing it? If you would just like to traverse all links, you could just use rdfs:seeAlso and it would give you the same result. IMO if you would like to introduce in the example http://example.com/vocab/createEvent client should use it, I would prefer something generic like hydra:add (or 'hydra:memberTemplate`) but still client would have to make use of it when finding intended operation.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
`hydra:memberTemplate` almost works but again, the object of this property doesn't have to be an `IriTemplate` at all. @elf-pavlik here's an example: 1. You register on a website 1. The server creates a private profile for you, say, `/user/some-guid` 1. You get a link to `/user/{public-user-name}` (or `/user/some-guid/public` if it wasn't to be a template) 1. You `PUT` to that resource to create a publicly visible profile resource So you link to a resource which doesn't initially exist and it's not part of any collection. But the general structure of your hypermedia is the same. I think that Stack Overflow follows a similar pattern.

In your example we don't have a collection where we want to create a member resource. I can't really think of situation where for collection we would have a resource and not a template for purpose of creating new members. If one wanted to use POST, would most likely use IRI of the collection, for PUT we need a template otherwise it wouldn't make sense for creating new members.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 72 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Of course, my bad. And what if `property` was removed from a template mapping? Is that legal?

I think we shouldn't require property from mapping of IriTemplate. Still we may want to provide data type somehow, also I would see a need later to inform client that it shoudl generate universal identifier UUID, CUID etc.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 97 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
I actually copied this from UC5 and probably similar in other use cases. Let's discuss separately? @alien-mcl?

I think we can make and Issue for that since UC5 also has it and don't block this PR if it doesn't introduce that issue

Comments from Reviewable

tpluscode commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

I can't really think of situation where for collection we would have a resource and not a template for purpose of creating new members

Like I described in another comment -> server can mint id's for new resources so that creating new members is idempotent

{
  "@id": "/comment/aaa-bbb-ccc",
  "hydra:add": {
    "@id": "/comment/xxx-yyy-zzz",
    "hydra:operation": [{
      "method": "PUT"
    }]
  }
}

The xxx-yyy-zzz would be random GUID every time. Transient unless PUT.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 72 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
I think we shouldn't require `property` from `mapping` of `IriTemplate`. Still we may want to provide data type somehow, also I would see a need later to inform client that it shoudl generate universal identifier UUID, CUID etc.

@lanthaler this must have been discussed already. do you recall any GitHub issue touching mapping property?

Comments from Reviewable

elf-pavlik commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
> I can't really think of situation where for collection we would have a resource and not a template for purpose of creating new members Like I described in another comment -> server can mint id's for new resources so that creating new members is idempotent ``` json { "@id": "/comment/aaa-bbb-ccc", "hydra:add": { "@id": "/comment/xxx-yyy-zzz", "hydra:operation": [{ "method": "PUT" }] } } ``` The `xxx-yyy-zzz` would be random GUID every time. Transient unless PUT.

This may work for something like 'Like' a movie etc. where movie would link to a not yet existing resource which once created would represent like by authenticated person (Vimeo and many other APIs style). For a collection to specify a way just add a one single resource to it doesn't seem practical at all. I can't imagine any service doing that since it would require client to re-request the collection every time before adding a new resource to it just to get a new IRI.

Comments from Reviewable

alien-mcl commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
If client doesn't use anywhere proposed by you `http://example.com/vocab/createEvent` why even bother with introducing it? If you would just like to traverse all links, you could just use `rdfs:seeAlso` and it would give you the same result. IMO if you would like to introduce in the example `http://example.com/vocab/createEvent` client should use it, I would prefer something generic like `hydra:add` (or 'hydra:memberTemplate`) but still client would have to make use of it when finding intended operation.

I feel like 'hydra:add' would be redundant to 'schema:AddAction', which is the convention I believe we agreed on. Still, we need to link the collection with the IriTemplate'd resource - I think the link's address is irrelevant from the client point of view. I also feel that this recursive flag is unneeded - all look-ups should be recursive within current resource. It should also use API documentation obtained at the very beginning.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 36 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
I like that. Only I would make this more functional instead of mutating the operation ``` js const operationTargetExpanded = operation.expandTarget(templateVariables); client.invoke(operationTargetExpanded, event); ```

I agree - we shouldn't mutate the original operation.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 58 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
+1.. but to answer the question: `manages` as currently defined only says that all members of the collection in this case are of type Event (nothing more and nothing less). I agree that we should find a better name for this.

Assuming the semantics given by @lanthaler, let's leave it for now.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
This may work for something like 'Like' a movie etc. where movie would link to a not yet existing resource which once created would represent like by authenticated person (Vimeo and many other APIs style). For a collection to specify a way just add a one single resource to it doesn't seem practical at all. I can't imagine any service doing that since it would require client to re-request the collection every time before adding a new resource to it just to get a new IRI.

I don't have any specific likes or dislikes for what was written above - just don't let us forget we shouldn't neither forbid nor enforce anything specific. Examples by @tpluscode could happen

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 97 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
I think we can make and Issue for that since UC5 also has it and don't block this PR if it doesn't introduce that issue

I'm not sure what is the problem - what does the JSON-LD spec says about relative URL's? How does it differ from the payload obtained with GET?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 116 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
Not this specification but it would be a violation of the [HTTP specification](https://tools.ietf.org/html/rfc7231#section-4.3.4): > The PUT method requests that the state of the target resource be > created or replaced with the state defined by the representation > enclosed in the request message payload. A successful PUT of a given > representation would suggest that a subsequent GET on that same > target resource will result in an equivalent representation being > sent in a 200 (OK) response. However, there is no guarantee that > such a state change will be observable, since the target resource > might be acted upon by other user agents in parallel, or might be > subject to dynamic processing by the origin server, before any > subsequent GET is received. A successful response only implies that > the user agent's intent was achieved at the time of its processing by > the origin server. > [...] > The fundamental difference between the POST and PUT methods is > highlighted by the different intent for the enclosed representation. > The target resource in a POST request is intended to handle the > enclosed representation according to the resource's own semantics, > whereas the enclosed representation in a PUT request is defined as > replacing the state of the target resource. Hence, the intent of PUT > is idempotent and visible to intermediaries, even though the exact > effect is only known by the origin server.

Good point

Comments from Reviewable

tpluscode commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

all look-ups should be recursive within current resource. It should also use API documentation obtained at the very beginning.

that's a twist :smile:

Comments from Reviewable

elf-pavlik commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
> all look-ups should be recursive within current resource. It should also use API documentation obtained at the very beginning. that's a twist :smile:

If the http://example.com/vocab/createEvent property doesn't matter, can we replace it with rdfs:seeAlso not to cause false impression that it does matter? If we decide that we need to use some meaningful property, then we can look at how client would rely on it and what it should mean and how do we name it. I don't think we can require server to include the template and all operations this template supports inline, IMO representation can just include IRI to the template and client should have capability to "follow its nose" and retrieve that template and operations its support. In that case even recursive traversal of particular representation will not work.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…
I don't have any specific likes or dislikes for what was written above - just don't let us forget we shouldn't neither forbid nor enforce anything specific. Examples by @tpluscode could happen

If we change method in that snippet from PUT to POST, how does the client know if it can use that resource just one and needs to refetch the collection to get new IRI before each time it wants to add to it (PUT) or it can continue using that resource (POST)

Comments from Reviewable

tpluscode commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
If the `http://example.com/vocab/createEvent` property doesn't matter, can we replace it with `rdfs:seeAlso` not to cause false impression that it does matter? If we decide that we need to use some meaningful property, then we can look at how client would rely on it and what it should mean and how do we name it. I don't think we can require server to include the template and all operations this template supports inline, IMO representation can just include IRI to the template and client should have capability to "follow its nose" and retrieve that template and operations its support. In that case even recursive traversal of particular representation will not work.

That's a new feature I think. But including those operations in ApiDocumentation should already be possible with SupportedProperties

Comments from Reviewable

tpluscode commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 15 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 115 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…
I agree with @elf-pavlik - HTTP `If-Match` header seems to fit better.

FYI, actually it's If-None-Match: *, which prevents an update if any representation exists.

Comments from Reviewable

lanthaler commented 6 years ago

Reviewed 1 of 1 files at r3. Review status: all files reviewed at latest revision, 14 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 1 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Good point. Actually now that I think of it, it's really the opposite: event is created directly (rather then by posting it to the collection). Or should I just call this "Creating with PUT" and not be fancy?

Creating event with PUT is fine. Let's also update the file name before submitting this.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 7 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
I guess I wasn't sure myself and forgot about it. **The URL** sounds good?

Sounds good.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 10 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Yes, part of. It's that and actually using doing PUT to the new resource itself. After all the server could link to concrete resource to PUT. Such as by minting a different URL for every GET. This is actually a good practice because let's the client take advantage of idempotency.

PUT is not just create and it is not the only way to create a resource so we should spell out here why PUT might be preferable.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
That's a new feature I think. But including those operations in ApiDocumentation should already be possible with SupportedProperties

Looks good. I'd only change getOperationOfType to getOperationsOfType as it always returns an array. You will also need to take the first result of the filter operation (currently operation is still an array).

I would probably also go a step further and also create a method to get rid of that filter statement to have something like

memberTemplate
    .getOperationsOfType('http://schema.org/CreateAction')
    .thatExpect('http://schema.org/Event')
    .first();

@alien-mcl what do you think about this?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 36 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…
I agree - we shouldn't mutate the original operation.

Great idea!

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 42 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
As an example think of a discussion thread where the server offers random URIs for replies. This way the client can safely PUT multiple times without actually posting multiple comments. So like @elf-pavlik says, if a resource doesn't exist yet is the client technically replacing or creating it? I think it's still creating. You can create resources but there is no representation. I rephrased this sentence by the way.

On second thought, I think we should split this off into a separate use case.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
If we change method in that snippet from `PUT` to `POST`, how does the client know if it can use that resource just one and needs to refetch the collection to get new IRI before each time it wants to add to it (`PUT`) or it can continue using that resource (`POST`)

Let's discuss PUT to a "placeholder URL" in the other comment or, likely better, in a separate use case.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 72 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
@lanthaler this must have been discussed already. do you recall any GitHub issue touching mapping property?

No. I also don't understand how a client would know how to replace the variable without property as it has no semantics then apart from being required.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 78 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
> that just adds a resource that already exists I think that's in scope of Use Case 9. Let's discuss there. Here I wanted to add an alternative to creating new resources (see UC 5). In fact I'll just add in the considerations section a note that this method can be used to create resource outside of a collection context.

I think it would be very surprising if it wouldn't be added to the collection as the IriTemplate is the memberTemplate of the collection.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 97 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…
I'm not sure what is the problem - what does the JSON-LD spec says about relative URL's? How does it differ from the payload obtained with `GET`?

The problem is that a POST/PUT don't have a base URL and so the relative URL can't be resolved. Using the target of the POST/PUT as base is not the right thing to do.

I filed https://github.com/HydraCG/Specifications/issues/144 to fix this later.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 115 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
FYI, actually it's `If-None-Match: *`, which prevents an update if any representation exists.

Thanks for looking it up Tomasz.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 124 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
I'm just saying that there isn't enough information to go on until we come up with more meta data for operations. We also haven't defined any concrete semantics here AFAIK and thus the client cannot assume anything which is not explicitly stated in the above representation.

Well, we can define the semantics however we want. IMO if the create operation is associated to a collection, it is reasonable to expect that new resource will become part of it. If the client doesn't want that, it should look for a create operation that isn't tight to a collection but, for instance, the class (Event in this case) instead.

Comments from Reviewable

tpluscode commented 6 years ago

Review status: all files reviewed at latest revision, 13 unresolved discussions.

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 10 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
PUT is not just create and it is not the only way to create a resource so we should spell out here why PUT might be preferable.

Do you have any suggestions?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 34 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
Looks good. I'd only change getOperationOfType to getOperation**s**OfType as it always returns an array. You will also need to take the first result of the filter operation (currently operation is still an array). I would probably also go a step further and also create a method to get rid of that filter statement to have something like ``` memberTemplate .getOperationsOfType('http://schema.org/CreateAction') .thatExpect('http://schema.org/Event') .first(); ``` @alien-mcl what do you think about this?

LGTM

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 42 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
On second thought, I think we should split this off into a separate use case.

If you have some details in mind please create an issue for that use case so that it doesn't escape our attention

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 64 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
Let's discuss PUT to a "placeholder URL" in the other comment or, likely better, in a separate use case.

Is that the same use case you mention in a comment above?

drafts/use-cases/5.1.creating-new-event-indirectly.md, line 78 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
I think it would be very surprising if it wouldn't be added to the collection as the IriTemplate is the memberTemplate of the collection.

There wasn't a memberTemplate when Karol asked his question, mind you.

Comments from Reviewable

elf-pavlik commented 6 years ago

Review status: 0 of 1 files reviewed at latest revision, 13 unresolved discussions, some commit checks broke.

drafts/use-cases/5.1.creating-event-with-put.md, line 10 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Do you have any suggestions?

For the client, one of the benefits comes with enabling it to create content while online, establishing relationships and then syncing it with a server once online, maybe even with https://github.com/WICG/BackgroundSync To keep it simple I think we can just mention service (server) - take advantage of Idempotence app (client) - control the URL (but here HTTP slug heder might also do)

drafts/use-cases/5.1.creating-event-with-put.md, line 42 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
If you have some details in mind please create an issue for that use case so that it doesn't escape our attention

I don't think that any use cases which we really care about will escape our attention, it may come up again when we continue with adding existing resources as members to collections

drafts/use-cases/5.1.creating-event-with-put.md, line 64 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Is that the same use case you mention in a comment above?

I think so... once again I suggest to wait until a really strong use case for such scenario shows up

drafts/use-cases/5.1.creating-event-with-put.md, line 72 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
No. I also don't understand how a client would know how to replace the variable without property as it has no semantics then apart from being required.

In cases where one uses opaquer IRI, we shouldn't expect that variable in template maps to any value relevant for the payload. How about we leave this one without property and follow up in this issue - https://github.com/HydraCG/Specifications/issues/145

Comments from Reviewable

lanthaler commented 6 years ago

Reviewed 2 of 2 files at r4. Review status: all files reviewed at latest revision, 12 unresolved discussions, some commit checks broke.

drafts/use-cases/5.1.creating-event-with-put.md, line 10 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
For the client, one of the benefits comes with enabling it to create content while online, establishing relationships and then syncing it with a server once online, maybe even with https://github.com/WICG/BackgroundSync To keep it simple I think we can just mention service (server) - take advantage of Idempotence app (client) - control the URL (but here HTTP slug heder might also do)

+1 to what Pavlik said. Mentioning idempotency seems like a great idea.

drafts/use-cases/5.1.creating-event-with-put.md, line 42 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
I don't think that any use cases which we really care about will escape our attention, it may come up again when we continue with adding existing resources as members to collections

Yeah, let's just drop it for now or copy it over into a separate document/PR. I don't think creating issues for these things is very efficient.

drafts/use-cases/5.1.creating-event-with-put.md, line 64 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
I think so... once again I suggest to wait until a really strong use case for such scenario shows up

Yeah, it's the same use case I mentioned above.

drafts/use-cases/5.1.creating-event-with-put.md, line 72 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
In cases where one uses opaquer IRI, we shouldn't expect that variable in template maps to any value relevant for the payload. How about we leave this one without property and follow up in this issue - https://github.com/HydraCG/Specifications/issues/145

If you have a IRI template, the IRI isn't opaque anymore. You describe it's structure with the template... and thus it is also necessary to describe what the variables stand for.

drafts/use-cases/5.1.creating-event-with-put.md, line 78 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
There wasn't a `memberTemplate` when Karol asked his question, mind you.

OK, we can close this comment then I guess if @alien-mcl agrees (please mark it as resolved here in Reviewable if you are satisfied with it so that it can close these threads; I see quite a few of the comments are still blocked)

drafts/use-cases/5.1.creating-event-with-put.md, line 135 at r4 (raw file):

ie. it's

Should be "i.e., its"

Comments from Reviewable

elf-pavlik commented 6 years ago

Review status: all files reviewed at latest revision, 12 unresolved discussions, some commit checks broke.

drafts/use-cases/5.1.creating-event-with-put.md, line 72 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
If you have a IRI template, the IRI isn't opaque anymore. You describe it's structure with the template... and thus it is also necessary to describe what the variables stand for.

I agree with your point about opacity, I mostly meant that variable in the IRI doesn't have to match any value of 'property' in a payload. It looks like latest revision has schema:namewhich I think can work for 'slug'. I will still like to eventually like example with unique identifier after brainstorming it in #145

Comments from Reviewable

tpluscode commented 6 years ago

Review status: all files reviewed at latest revision, 12 unresolved discussions, some commit checks broke.

drafts/use-cases/5.1.creating-event-with-put.md, line 10 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
+1 to what Pavlik said. Mentioning idempotency seems like a great idea.

Done.

drafts/use-cases/5.1.creating-event-with-put.md, line 135 at r4 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
> ie. it's Should be "i.e., its"

Done.

Comments from Reviewable

lanthaler commented 6 years ago

Reviewed 1 of 2 files at r4, 1 of 1 files at r5. Review status: all files reviewed at latest revision, 11 unresolved discussions.

drafts/use-cases/5.1.creating-event-with-put.md, line 42 at r1 (raw file): To close this out, I'd propose to change

This linked resource can already have an identifier or be an IriTemplate as presented in the above snippet. In this case the representation of the collection resource is not unlike that seen in the Searching events use case

to "The target of the operation can either be a concrete URL or an IriTemplate as shown in the example above. Concrete URLs are out of scope for this use case and might be discussed at a later point in a separate use case document."

drafts/use-cases/5.1.creating-event-with-put.md, line 72 at r1 (raw file):

Previously, elf-pavlik (elf Pavlik) wrote…
I agree with your point about opacity, I mostly meant that variable in the IRI doesn't have to match any value of 'property' in a payload. It looks like latest revision has `schema:name`which I think can work for 'slug'. I will still like to eventually like example with unique identifier after brainstorming it in #145

Agreed, it doesn't have to necessarily be a property from the payload. We need to define how this is supposed to work better at some point. I think this is currently one of the weakest aspects of Hydra.

drafts/use-cases/5.1.creating-event-with-put.md, line 135 at r4 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
Done.

Thanks

Comments from Reviewable

tpluscode commented 6 years ago

Review status: all files reviewed at latest revision, 10 unresolved discussions.

drafts/use-cases/5.1.creating-event-with-put.md, line 42 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
To close this out, I'd propose to change > This linked resource can already have an identifier or be an `IriTemplate` as presented in the above snippet. In this case the representation of the collection resource is not unlike that seen in the [Searching events use case](7.searching-events.md) to "The target of the operation can either be a concrete URL or an `IriTemplate` as shown in the example above. Concrete URLs are out of scope for this use case and might be discussed at a later point in a separate use case document."

Done.

Comments from Reviewable

lanthaler commented 6 years ago

:lgtm: I'll merge this as soon as @elf-pavlik and @alien-mcl resolved all remaining comments.

Reviewed 1 of 2 files at r4, 1 of 1 files at r6. Review status: all files reviewed at latest revision, 9 unresolved discussions.

Comments from Reviewable

alien-mcl commented 6 years ago
:lgtm:

Review status: all files reviewed at latest revision, 2 unresolved discussions.

drafts/use-cases/5.1.creating-event-with-put.md, line 34 at r1 (raw file):

Previously, tpluscode (Tomasz Pluskiewicz) wrote…
LGTM

Looks nice - do you want to extend those operation arrays with fluent-like API? There may be already something that we could use.

drafts/use-cases/5.1.creating-event-with-put.md, line 72 at r1 (raw file):

Previously, lanthaler (Markus Lanthaler) wrote…
Agreed, it doesn't have to necessarily be a property from the payload. We need to define how this is supposed to work better at some point. I think this is currently one of the weakest aspects of Hydra.

I believe the spec doesn't say anything on where the value of the property being mapped should come from. In some cases indeed it can be taken from the payload, but for some other i.e. from user input or auto-generated. I think this is a topic for another story.

Comments from Reviewable

lanthaler commented 6 years ago

@alien-mcl, please resolve the comments. Just LGTMing the PR isn't enough. You need the change the status of each comment:

Resolve

Review status: all files reviewed at latest revision, 2 unresolved discussions.

drafts/use-cases/5.1.creating-event-with-put.md, line 34 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…
Looks nice - do you want to extend those operation arrays with fluent-like API? There may be already something that we could use.

Yeah, that's the idea ;-)

drafts/use-cases/5.1.creating-event-with-put.md, line 72 at r1 (raw file):

Previously, alien-mcl (Karol Szczepański) wrote…
I believe the spec doesn't say anything on where the value of the property being mapped should come from. In some cases indeed it can be taken from the payload, but for some other i.e. from user input or auto-generated. I think this is a topic for another story.

Yep

Comments from Reviewable

elf-pavlik commented 6 years ago
:lgtm:

Review status: all files reviewed at latest revision, 2 unresolved discussions.

Comments from Reviewable

tpluscode commented 6 years ago

Does it mean we're ready to merge?

Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

lanthaler commented 6 years ago

Yep. Merging

Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

tpluscode commented 6 years ago

What is the process now to close #3, #118 and #141?

lanthaler commented 6 years ago

There's no formal process it mostly depends on the state of the issue.

For #3 we miss a decision on whether we introduce something akin to schema:action as Hydra currently requires users to define their own properties such as rsvp.

118 could either be closed (by you, since you opened it) or left open till it is clarified in the spec itself as it isn't properly documented at the moment.

It is not clear to me what #141 is about at this stage. I left a comment to clarify it