Skip to content
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

Retracting operations #246

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

alien-mcl
Copy link
Member

Summary

This pull-request addresses issue #241.

More details

I've introduced a availability predicate that can mark a specific Operation as either Forbidden or Unauthorized. I didn't want to introduce NotImplemented or ConflictsWithCurrentStateOfResource suggested in the discussion as I felt these would conflict with the initial logic of the supported operation (why supporting it if it is not implemented) or it felt irrational (client didn't send a payload yet).

Feel free to deliberate on the approach. It was the simples thing to introduce to keep the vocabulary minimal yet still effective.

@asbjornu
Copy link
Member

asbjornu commented Aug 29, 2022

All variants of hydra:Availability have the same @type and are thus indistinguishable besides their @id. Would it be a good idea to at least make hydra:Available (indicating success) distinguishable from the others (which indicate error)? Perhaps by making the non-available types subclasses of hydra:Error (in addition to hydra:Availability), by adding a success flag, or something similar.

@alien-mcl
Copy link
Member Author

Hmm - client needs to know that everything other than Available means no go for an operation as the spec says. Why introducing another term?

@asbjornu
Copy link
Member

asbjornu commented Sep 1, 2022

I'm thinking in terms of future extensibility. It's already a rather direct mapping between hydra:Availability and different HTTP status codes:

Availability HTTP Status
hydra:Available 200 OK
hydra:Forbidden 403 Forbidden
hydra:Unauthorized 401 Unauthorized

Now, of course not all HTTP status codes will have meaningful mappings to hydra:Availability, but I can at least envision that some of the status codes in the 5xx range could be applicable, such as 503 Service Unavailable and 504 Gateway Timeout, in situations where upstream servers are providers of operations exposed by a downstream client as hydra:Operation and those upstream servers are unavailable for whatever reason.

Being able to distinguish why an operation isn't available beyond its name would therefore be useful, I think.

@alien-mcl
Copy link
Member Author

Damn notifications - nothing appeared on my mailbox :/

Anyway, I shall then replace hydra:Forbidden with something else as it was not my goal to mimic HTTP status codes. By naming it forbidden i had in mind exact meaning of this word - the operation is forbidden, even if it was announced earlier, it is available - client mustn't call it at any circumstances.

I've added hydra:Unauthorized as state related to user capabilities might be of interest - it is somehow available, but at the different circumstances.

All these terms introduced are related to retracting operations - HTTP status codes 5XX has nothing to do with retracting - these are server errors and I don't think it is a normal approach to expect those in any circumstances. As for client errors 4XX - I can't imagine any other named situation when a separate term could come in handy, but I'm opened for ideas.

Maybe a situation, when an operation is still available, but not with current application state - i.e. hydra:Unavailable might be useful, but I feel this would be a long shot.

@asbjornu
Copy link
Member

Anyway, I shall then replace hydra:Forbidden with something else as it was not my goal to mimic HTTP status codes. By naming it forbidden i had in mind exact meaning of this word - the operation is forbidden, even if it was announced earlier, it is available - client mustn't call it at any circumstances.

If a hydra:Forbidden operation is executed by the client due to having stale metadata or whatever other reason, isn't it natural to think that the server would respond with a 403 Forbidden?

I've added hydra:Unauthorized as state related to user capabilities might be of interest - it is somehow available, but at the different circumstances.

Exactly. So if a client inadvertently executes an operation currently marked as hydra:Unauthorized, the server would respond with 401 Unauthorized. Right?

All these terms introduced are related to retracting operations - HTTP status codes 5XX has nothing to do with retracting - these are server errors and I don't think it is a normal approach to expect those in any circumstances.

I tried to outline a use-case where this may be. Hopefully the below diagram makes it clearer:

sequenceDiagram
    participant Client
    participant Server
    participant U1
    participant U2

    Client->>Server: GET operations
    activate Server
        Server->>U1: GET operations
        activate U1
            U1->>Server: 503 Service Unavailable
        deactivate U1
        Server->>U2: GET operations
        activate U2
            U2->>Server: 200 OK { operations }
        deactivate U2
        Server->>Server: Aggregate operations
        Server->>Client: 200 OK { operations }
    deactivate Server
Loading

Client asks Server for its available operations. Some of Server's operations are delivered by upstream servers U1 and U2. To deliver a fresh list of available operations to the Client, Server asks U1 and U2 for their operations. U1 is currently unavailable and responds with HTTP 503 Service Unavailable. U2, however, is available and responds with its operations.

Server then aggregates the operations from U1 and U2 and responds to Client with U1's operations marked as hydra:Unavailable.

Maybe a situation, when an operation is still available, but not with current application state - i.e. hydra:Unavailable might be useful, but I feel this would be a long shot.

Perhaps. I think it's good to hash this out thoroughly, nonetheless. :)

@alien-mcl
Copy link
Member Author

If a hydra:Forbidden operation is executed by the client due to having stale metadata or whatever other reason, isn't it natural to think that the server would respond with a 403 Forbidden?

Hmm, no, not really - it generally depends on what is stale - operation may not be there at all, or expected type could have been changed or many other situations might have been changed and hydra:Forbidden wouldn't give it all about them. I'm even more into replacing it with simple and straightforward hydra:Unavailabble.

Exactly. So if a client inadvertently executes an operation currently marked as hydra:Unauthorized, the server would respond with 401 Unauthorized. Right?

Yes, this might be the only place that fits one-to-one with HTTP.

Client asks Server for its available operations. Some of Server's operations are delivered by upstream servers U1 and U2. To deliver a fresh list of available operations to the Client, Server asks U1 and U2 for their operations. U1 is currently unavailable and responds with HTTP 503 Service Unavailable. U2, however, is available and responds with its operations.

This example does not talk to me at all. If the U1 operations are not available why broadcastim them in the first place? I think this is against initial idea of retracting operations. Presented terms were meant to retract something that has been broadcasted earlier, not to denote an operation as a placeholder for something that might be there in future.

@alien-mcl
Copy link
Member Author

As I mentioned earlier - I've replaced Forbidden with Unavailable to remove somehow accidental coherence with HTTP

@alien-mcl
Copy link
Member Author

@asbjornu @tpluscode thoughts on this?

@asbjornu
Copy link
Member

Are we just going to assume any @id different to hydra:Available is a non-available operation?

@alien-mcl
Copy link
Member Author

Well, effectively yes. Shall we formalize it somehow in either the spec or in vocabulary? As for the Unauthorized I was thinking about adding a paragraph about potential security threats as emitting it in advance would reveal operation that would normally be hidden.

@ghost
Copy link

ghost commented Dec 21, 2022

@alien-mcl This happens normally when you bookmark a hyperlink and it becomes stale. E.g. you don't pay the monthly bills and the account is closed out of the API. Normally it is very rare to have a non-working hyperlink served at least it should be very rare with REST principles.

@alien-mcl
Copy link
Member Author

@Inf3rno I'm not sure which part are you referring? The one with possible security threat?

@ghost
Copy link

ghost commented Dec 22, 2022

@alien-mcl Normally the API should not serve hyperlinks to forbidden or unavailable operations. Trial and error is the SOA approach, afaik. by REST we just don't give the consumers broken hyperlinks if possible. It should be extremely rare to run on this kind of hyperlinks, e.g. stale bookmarks or complex preconditions which are better not to check by every page load.

@alien-mcl
Copy link
Member Author

Well, there is at least one scenario that Hydra supports from the very beginning - supported operations. It is possible to tell the client that each instance of some class supports some operations. This operation coule be also presented to the client with an IRI template, thus there is no real physical link provided, yet client still is able to mint one and invoke. It may be needed by the developer to retract such an announced operation. Somehow you're correct and that's why this feature appeared recenly, several years after first specification version.

@ghost
Copy link

ghost commented Dec 22, 2022

@alien-mcl What you are doing is querying a bunch of operations. The upper is sort of query all except some. The other approach is query some. After adding NOT support, the next will be adding AND and OR support and more operators for comparison. I would not start to develop a query language, I would rather just list the supported operations. It is simple and does not require complicated client or server side logic. Though who am I to tell you, I decided to write a query language in my private project too. :-)

@alien-mcl
Copy link
Member Author

I got somehow lost. We're not developing any query language.

The client currently needs to gather operations from all possible sources (API documentation's supported operations and in-band operations from the current state). It needs to do it either with i.e. some local tripple store SPARQL query (Hydra is RDF after all) or with some other custom logic, like Heracles.ts does.
Indeed adding possibility of retracting operations makes this a bit more complex - the result of the previous step should be trimmed with those retracted operations.

Indeed I won't die for this feature - I just remember there were some situations when we discussed such a possibility. If the community feels this is too far and we don't need it - I'll reject this one with no regrets.

@asbjornu
Copy link
Member

I think we may be talking a bit past each other because there is no agreed-upon, idiomatic way for a Hydra API to behave. Hydra makes data about possible HTTP operations available as hydra:supportedOperation described in ApiDocument and hydra:operation as inline hypermedia controls in the response to a previous request. Let's call these two mechanisms static and dynamic.

The issue at hand is that the static and dynamic metadata may be conflicting. A hydra:supportedOperation may not be available in the current state of the resource. One suggested way to deal with this is to not treat ApiDocumentation as a static resource, but instead re-request every time it changes.

To treat out-of-bound metadata such as ApiDocumentation just as dynamic as the requested resource, however, is unintuitive. I don't think that's going to sit well with many developers. I think ApiDocumentation instead should be viewed almost as static as an openapi.yml file, to generate the initial skeleton of an API client.

Treating ApiDocumentation as a static resource then brings us back to the problem of being able to express that a hydra:supportedOperation is no longer available in the current state of the resource. Simply removing a hydra:operation from a response isn't enough; the client already knows the operation exists. The server thus must be able to tell the client that the cannot be successfully executed given the current state of the resource.

Another way to look at this is that hydra:supportedOperation does not specify operation availability on an instance of a resource, just on its interface or definition. This means that auto-generated clients cannot make hydra:supportedOperation available on resource instances; instead, the operations are infrastructure that is used only when the hydra:operation in a response indicates that the operation should be available.

Either way, the spec needs to clarify this somehow. I don't think we can close this without figuring out a different approach to fixing the problem.

@alien-mcl
Copy link
Member Author

Thanks @asbjornu for detailed explanation. I'd love to deliberate more...

One suggested way to deal with this is to not treat ApiDocumentation as a static resource, but instead re-request every time it changes.

It won't change anything - API documentation can be static resource that barely changes over time making a client effectively not re-requesting it. Also I don't see how re-requesting would help event if it changes - the operation in context may be unchanged at all.

To treat out-of-bound metadata such as ApiDocumentation just as dynamic as the requested resource, however, is unintuitive. I don't think that's going to sit well with many developers.

I agree - I don't think we want to go this way.

This means that auto-generated clients cannot make hydra:supportedOperation available on resource instances

I dare to disagree. As you've already noted, hydra:supportedOperation specifies an operation on the definition level - it's a way of extending a class with new features. If an instance of that very class, it must comply or it shouldn't be considered that class.

instead, the operations are infrastructure that is used only when the hydra:operation in a response indicates that the operation should be available.

There would be no need for hydra:supportedOperation as inlined hydra:operation is fully sufficient. The hydra:supportedOperation was not mean to work like that and we won't change it in the spec - it would break everything in that matter that exists now.

@Inf3rno, I was thinking about your statement:

The upper is sort of query all except some. The other approach is query some. After adding NOT support, the next will be adding AND and OR support and more operators for comparison.

I don't think it has to do anything with querying. I'd stick to simple operations on sets (I believe set difference matches this case). Ok, matching operation may not be that simple (that's why I initially suggested direct operation IRI comparison), but it's doable event with simple clients.

Anyway, I seek for consensus on simple thing: do we need such a feature or shall we leave the clients with raw protocol responses - server will forbid an operation and the client will get to know about that sooner or later (later in this case)

@ghost
Copy link

ghost commented Jan 5, 2023

I think we may be talking a bit past each other because there is no agreed-upon, idiomatic way for a Hydra API to behave. Hydra makes data about possible HTTP operations available as hydra:supportedOperation described in ApiDocument and hydra:operation as inline hypermedia controls in the response to a previous request. Let's call these two mechanisms static and dynamic.

The issue at hand is that the static and dynamic metadata may be conflicting. A hydra:supportedOperation may not be available in the current state of the resource. One suggested way to deal with this is to not treat ApiDocumentation as a static resource, but instead re-request every time it changes.

To treat out-of-bound metadata such as ApiDocumentation just as dynamic as the requested resource, however, is unintuitive. I don't think that's going to sit well with many developers. I think ApiDocumentation instead should be viewed almost as static as an openapi.yml file, to generate the initial skeleton of an API client.

Treating ApiDocumentation as a static resource then brings us back to the problem of being able to express that a hydra:supportedOperation is no longer available in the current state of the resource. Simply removing a hydra:operation from a response isn't enough; the client already knows the operation exists. The server thus must be able to tell the client that the cannot be successfully executed given the current state of the resource.

Another way to look at this is that hydra:supportedOperation does not specify operation availability on an instance of a resource, just on its interface or definition. This means that auto-generated clients cannot make hydra:supportedOperation available on resource instances; instead, the operations are infrastructure that is used only when the hydra:operation in a response indicates that the operation should be available.

Either way, the spec needs to clarify this somehow. I don't think we can close this without figuring out a different approach to fixing the problem.

As far as I understand the concept the API documentation is static, it contains resource type descriptions along their supported operations. The API responds with resource representations, which contain the resources which can have multiple types each of them multiple supported operations. For the actual resource not all of the operations are available, e.g. for the current user writing is not possible or the resource is in a state where update, deletion is not possible, etc. If the client calls an operation that is not available, that's an issue. One way to deal with the problem is describing only the available operations in the actual resource representation. Another way is describing all of them and telling that the actual operations is or is not available for the actual resource. I prefer the former one, e.g. in the case of a webpage it does not make sense to serve broken hyperlinks and write in the comment that you can try them out if you want to, but they are broken, so you are just wasting your time. Webpages just don't serve broken hyperlinks. Though maybe I misunderstood the problem and we are talking about something different here.

@ghost
Copy link

ghost commented Jan 5, 2023

Thanks @asbjornu for detailed explanation. I'd love to deliberate more...

One suggested way to deal with this is to not treat ApiDocumentation as a static resource, but instead re-request every time it changes.

It won't change anything - API documentation can be static resource that barely changes over time making a client effectively not re-requesting it. Also I don't see how re-requesting would help event if it changes - the operation in context may be unchanged at all.

To treat out-of-bound metadata such as ApiDocumentation just as dynamic as the requested resource, however, is unintuitive. I don't think that's going to sit well with many developers.

I agree - I don't think we want to go this way.

This means that auto-generated clients cannot make hydra:supportedOperation available on resource instances

I dare to disagree. As you've already noted, hydra:supportedOperation specifies an operation on the definition level - it's a way of extending a class with new features. If an instance of that very class, it must comply or it shouldn't be considered that class.

instead, the operations are infrastructure that is used only when the hydra:operation in a response indicates that the operation should be available.

There would be no need for hydra:supportedOperation as inlined hydra:operation is fully sufficient. The hydra:supportedOperation was not mean to work like that and we won't change it in the spec - it would break everything in that matter that exists now.

@Inf3rno, I was thinking about your statement:

The upper is sort of query all except some. The other approach is query some. After adding NOT support, the next will be adding AND and OR support and more operators for comparison.

I don't think it has to do anything with querying. I'd stick to simple operations on sets (I believe set difference matches this case). Ok, matching operation may not be that simple (that's why I initially suggested direct operation IRI comparison), but it's doable event with simple clients.

Anyway, I seek for consensus on simple thing: do we need such a feature or shall we leave the clients with raw protocol responses - server will forbid an operation and the client will get to know about that sooner or later (later in this case)

Well what I was talking about that we have a set of available and not-available operations for each resource and each resource can have multiple types, which can have many supported operations. Normally we describe the resource types in the documentation, so we don't have to repeat this description countless times in our resources. For the actual resource we just give which types it has. After that if not all of the operations are available for the types, then we need to decide which ones are available. The simplest way is returning a set of available operations. If we want to be even more dense, then we return a query, which can be used on the documentation and which results the set of available operations. E.g. "all for type1 and a,b,c, but not d for type2 and all operations that start with 'x' for type3". If the resource representation contains the parameters for the IRI template, then they can be auto-filled, so many times all we need is just the operation names and no parameter description for the actual operation. Though this might make the response harder to process.

@alien-mcl
Copy link
Member Author

@Inf3rno

I think you understand the problem correctly.

The thing is, moving toward former solution would break all that currently exists and is based on hydra - it was never supposed to work that way. That's why the latter solution is somehow closer to my heart. There is a third solution -do nothing, let the client fail invoking an operation discovering the issue in runtime (as it is now).

As for the statement that Webpages just don't serve broken hyperlinks - why am I seeing dozens of broken links leading to 404s ? Also, a bookmarked link may become obsolete in time - smart client could discover it with proper tools.

I can also imagine a situation when a resource is of several types causing some operations to be inoperable due to some logic. I know it's a long shot and multiple resource types is nothing easy to work with, but this is RDF world after all and there is nothing that may hold you back in this - retracting operations may be the only solution for that scenario.

@asbjornu
Copy link
Member

asbjornu commented Jan 5, 2023

I'm a bit torn. I agree with @Inf3rno that we should try to model Hydra's behavior on the already well-established hypermedia mechanisms of HTML. Informing a client about operations that aren't available at runtime, is therefore something we should avoid. Only the operations that are available given the current state of the resource, should be published.

However, there is value in clients knowing which operations may become available at runtime, so they can build in support for them somehow. Just like with OpenAPI. I think we may need to disconnect hydra:supportedOperation from hydra:operation somehow. The operations a client can execute at any time during the lifetime of a resource is not the union of all hydra:supportedOperation and hydra:operation. At least not imho.

@ghost
Copy link

ghost commented Jan 6, 2023

@Inf3rno

I think you understand the problem correctly.

The thing is, moving toward former solution would break all that currently exists and is based on hydra - it was never supposed to work that way. That's why the latter solution is somehow closer to my heart. There is a third solution -do nothing, let the client fail invoking an operation discovering the issue in runtime (as it is now).

As for the statement that Webpages just don't serve broken hyperlinks - why am I seeing dozens of broken links leading to 404s ? Also, a bookmarked link may become obsolete in time - smart client could discover it with proper tools.

I can also imagine a situation when a resource is of several types causing some operations to be inoperable due to some logic. I know it's a long shot and multiple resource types is nothing easy to work with, but this is RDF world after all and there is nothing that may hold you back in this - retracting operations may be the only solution for that scenario.

Okay, the wording wasn't the best here. I meant, that normally it is not the intended behavior to have broken links.

We could model it with types accurately with very small types which represent a fragment of the resource state. Each of these types could have operations and the resource state would be the composition of these types. It works in the RDF world, but when we create an abstraction it is usually a lot more vague and it requires a lot of extra work and thinking to create an accurate abstraction, so I guess for most developers this kind of approach does not work.

The SOA approach was that we had a WSDL documentation and let the client fail in such situations. I think for REST this is not acceptable and the representation should contain the actual state of the resource or something close to it with this kind of details. Having obsolete links and bookmarks should not be an issue, they are sort of cache after all, and when they are broken the client can update cache with the same process it used to find these links. So I think there are two types of broken links here with two different reasons and better not to mix them. As far as I know the SOA type broken links can lead to a lot more failed calls than the cache type broken links and each failed call is resource wasting from server perspective. Another thing that if you want to avoid SOA type failed calls, you must copy the logic to the client which leads to the failure. E.g. copy the access control logic, for example checking whether the user has admin right, because only admins can use a certain link. Which we try to avoid normally, because it strongly couples the client to the access control implementation of the service. So it is against REST principles. On the other hand having a link cache meets completely with REST principles.

Copy link
Contributor

@tpluscode tpluscode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for coming late to the party. I quickly read the discussion so far and I have some thoughts.

re broken links

I don't exactly agree with @Inf3rno here. At least in some cases websites will very much serve "broken" links in the sense that following it will redirect to a login page or a page where additional permissions can be requested etc. Removing links is not the only option, and possibly not the right one either

Also a reminder that Hydra has always offered an option for fine-grained support for operations using RDF types. A most unspecific example could be a generic </api/DeletableResource> supported class which would support DELETE. Simple as that, anyone who requests a resource that they are allowed to delete, would see an additional rdf:type </api/DeletableResource> triple.

There is but one very bad problem with such a design (as is with inline operations and context-specific retraction of supported operations). Caching may become very difficult if different clients get different representations.

Which comes to an important observation: retracting operations may in fact be desirable for two very different reasons:

  1. the state of the resource (eg. published article cannot be modified)
  2. client-specific reasons (eg. unauthenticated clients can only GET)

The former is easy, and the rdf:type method should be sufficient. The types like </api/DeletableResource> can even dynamically added to the response, or even precomputed and stored in the database.


Now, finally, the important part. In the latter case, I somewhat feel that Hydra Core may not need the feature to retract operations. At least not in the form where individual operation is retracted.

What I think would be much more useful and powerful is a kind of "computed operations" but the context-specific nature where auth needs to be taken into account is tricky.

I get the impression that a client-side set of optional rules would be a better fit. Again, not necessarily a core feature

{
"@type": "Operation",
"method": "POST",
"expects": "http://api.example.com/doc/#Upload",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does that even work? By matching hydra:expects? That doesn't seem right...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected (no pun intended), to retract by rdf:type

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described above - idea is to either match by the operation iri or exact match of the predicates enlisted

@tpluscode
Copy link
Contributor

On second thought, auth-specific operations would ideally be served dynamically in the API Documentation. Nowhere in the spec does it say that it is a static resource. Once logged, the documentation would then be expanded with the additional operations a client is allowed to invoke

@asbjornu
Copy link
Member

asbjornu commented Jan 6, 2023

I think it's actually better to remove hydra:supportedOperation from the ApiDocumentation entirely than to make ApiDocumentation dynamic. ApiDocumentation being dynamic is just not something developers will expect.

At least unless we are able to somehow design hydra:supportedOperation as a template for which operations might become available, instead of how it is now where the available operations at any time is the static union of hydra:supportedOperation and hydra:operation.

And to answer @alien-mcl's question: Yes, Hydra Core needs a way to retract operations. Preferably by just excluding them from the response (regardless of what's specified in ApiDocumentation).

@tpluscode
Copy link
Contributor

ApiDocumentation being dynamic is just not something developers will expect.

We may have a different expectation here, but that is one important difference which puts Hydra ahead of everyone else. The API Documentation can change over time...

as a template for which operations might become available

We could accomplish that with OWL or SHACL by having a "computed class". For example.

</api/EditableArticle> 
  a rdfs:Class, hydra:Class, sh:NodeShape ;
  rdfs:subClassOf </api/Article> ;
  sh:property [
    sh:path schema:status ;
    sh:in ( </api/Draft> ) ;
  ] ;
  hydra:supportedOperation [
    a schema:UpdateAction ;
    hydra:method "PUT" ;
    hydra:expects </api/Article> ;
  ] ;
.

A resource representation would not need to be explicitly typed </api/EditableArticle> but instead a representation could be dynamically matched against that shape-class to determine whether that operation is allowed or not.

A server could make that precomputation too, if desired, which would accommodate the authorization-dependent operation

Yes, Hydra Core needs a way to retract operations

I am not convinced. Operation being additive are going to be much easier overall.

@asbjornu
Copy link
Member

asbjornu commented Jan 6, 2023

We could accomplish that with OWL or SHACL by having a "computed class".

Do I interpret you correctly in that this is thus not something we can (or at least should) solve in Hydra Core? Or do you see a way to redefine how hydra:supportedOperation is specified in ApiDocumentation to accomplish this?

Yes, Hydra Core needs a way to retract operations

I am not convinced. Operation being additive are going to be much easier overall.

As long as it accomplishes the same result (i.e. some operations may not be available and the client doesn't have to perform an HTTP request to find out), I agree an additive approach is preferable. But as it stands right now, the additive approach does not allow us to say "operation X is not available at this moment", which is what this pull request is trying to fix.

If we could fix it in ApiDocumentation instead, I think that would be much better.

@tpluscode
Copy link
Contributor

Do I interpret you correctly in that this is thus not something we can (or at least should) solve in Hydra Core?

It's somewhere in-between. If you take away the sh: bit, what remains is a hydra:Class with an operation. If the server would use the SHACL rule (ie. "draft article is editable") to assert the type dynamically, then it's just an implementation detail. If the client would need to process such rules (SHACL, OWL, or otherwise), then it goes beyond Core.

Now that I wrote the above paragraph, I see that it all comes back to the same idea of asserting <> a </api/EditableArticle>. It boils down to how the rules are expressed and when they are processed. N3 rules would also work for that, such as

{ ?x schema:status </api/Draft> } => { ?x a </api/EditableResource> } .

But as it stands right now, the additive approach does not allow us to say "operation X is not available at this moment", which is what this pull request is trying to fix.

Yes, and I do see the usefulness of that.


Okay @alien-mcl, after some consideration, I agree that we should add a way to announce unavailable operations but I'm not agreeing with the proposed method. Maybe that could be "detached" from the resource itself in its representation. I don't remember now, but at some point we did discuss that the representation could be composed of multiple graphs. To illustrate

graph </api> {
  </api/Article> 
    hydra:supportedOperation [
      hydra:method "POST" ; a schema:UpdateAction ;
    ] ;
    hydra:supportedOperation </api/DeleteOperation> .

  </api/DeleteOperation> hydra:method "DELETE" .
}

graph </article/foo> {
  # both operations available
  </article/foo> a </api/Article> .
}

graph </article/bar> {
  </article/bar> a </api/Article> .

  # retracts update operation type
  [ 
    a hydra:ForbiddenOperation ;
    hydra:retractedOperation schema:UpdateAction ; 
    schema:target </article/bar> ;
    hydra:description "You do not have permission to update" ;
  ] .
}

graph </article/baz> {
  </article/baz> a </api/Article> .

  # retracts delete operation by operation URI
  [ 
    # (implicitly)
    a hydra:UnavailableOperation ;
    hydra:retractedOperation </api/DeleteOperation> ;
    schema:target </article/baz> ;
    hydra:description "Published articles cannot be deleted" ;
  ] .
}

I just came up with this, so please be gentle :).

  1. The rdf:type of these blank resources would match the proposed hydra:Availability only that there would never be the positive hydra:Available - only negations
  2. I did not want to reuse hydra:operation because of its domain/range and definition
  3. schema:target could be replaced with hydra:target but again, saves one additional term

spec/latest/core/index.html Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Jan 6, 2023

On second thought, auth-specific operations would ideally be served dynamically in the API Documentation. Nowhere in the spec does it say that it is a static resource. Once logged, the documentation would then be expanded with the additional operations a client is allowed to invoke

I thought about something similar for access control, to hide operations available only for admins. From security perspective it makes sense, because the attackers don't know exactly what they get when they manage to hack the system, so they might be less motivated because of this.

@ghost
Copy link

ghost commented Jan 6, 2023

At least in some cases websites will very much serve "broken" links in the sense that following it will redirect to a login page or a page where additional permissions can be requested etc.

From my perspective these should be normal links and permissions should be checked before returning such a link.

Ofc. there are other solutions as well, I wrote maybe 2-3 possible solutions in the previous comment. I personally like the RDF composite type approach and not serving unavailable links the best so far. Though it is hard to choose if we don't know much about what the developers really need. Not sure if it is possible to support multiple solutions at once. Or should Hydra core be as simple as possible without any sugar terms?

@ghost
Copy link

ghost commented Jan 6, 2023

I think it's actually better to remove hydra:supportedOperation from the ApiDocumentation entirely than to make ApiDocumentation dynamic. ApiDocumentation being dynamic is just not something developers will expect.

At least unless we are able to somehow design hydra:supportedOperation as a template for which operations might become available, instead of how it is now where the available operations at any time is the static union of hydra:supportedOperation and hydra:operation.

And to answer @alien-mcl's question: Yes, Hydra Core needs a way to retract operations. Preferably by just excluding them from the response (regardless of what's specified in ApiDocumentation).

Well if we have a documented standard which says that API Documentation can be dynamic based on privileges, then they must expect it. Being able to properly cache different versions of the documentation would worry me more.

@ghost
Copy link

ghost commented Jan 6, 2023

I thought about this more. The main reason I think the composite approach and hiding not available operations is not the best, because we don't know the reason why a certain operation is not available. If we return it to the client explicitly and they expect it, then they can solve the issue easier. Though this appears to be some sort of debug related problem for a debug mode, not something one would generally expect.

@tpluscode
Copy link
Contributor

tpluscode commented Jan 6, 2023 via email

@alien-mcl
Copy link
Member Author

alien-mcl commented Jan 6, 2023

Oh dear - so many comments I don't know where to start.

@tpluscode

Existing implementations which do not understand the "availability" will inevitably interpret as an inline operation.

Fair point - maybe introducing hydra:ForbiddenOperation is better. I don't like the hydra:target approach - I'd prefer to stick the forbidden operation to the resource. We had some discussions about actions, but it died several years ago and I don't feel we want to dig it out now.

@asbjornu

If we could fix it in ApiDocumentation instead, I think that would be much better.

I don't feel like we want to change that part - it exists there as it is not for several years and fundamental changes in that matter introduced in the last feature I acknowledge a completion for the hydra would be a wrong move.

I think it's actually better to remove hydra:supportedOperation from the ApiDocumentation entirely than to make
ApiDocumentation dynamic. ApiDocumentation being dynamic is just not something developers will expect.

While API Documentation can change in time for obvious reasons (new features in the API, etc.), I don't feel like it should be considered a dynamic resource. Hydra does not mandate to stick to either API documentation or inlined hypermedia controls. I heard of implementations using purely API documentation approach as well as of implementations that were fully API documentation ignorant (pure inline hypermedia approach). The only the that hydra requires is an existence of the API Documentation that points to an entry point(s).

@tpluscode
Copy link
Contributor

Fair point - maybe introducing hydra:ForbiddenOperation is better.

Maybe a property?

</article/foo>
  hydra:retractedOperation [
    # forbid all POST requests
    hydra:method "POST" ;
  ] ;
  hydra:retractedOperation [
    # forbid operation of specific type
    hydra:operation schema:RemoveAction ; 
  ] ;
  hydra:retractedOperation </api/DeleteOperation> ;
.

Only the last one is problematic because it's not possible to add context of why </api/DeleteOperation> is unavailable. Unless we allow it as hydra:retractedOperation [ hydra:operation </api/DeleteOperation> ] so that the object can be both an operation's URI as well as type

I don't like the hydra:target approach - I'd prefer to stick the forbidden operation to the resource

The reason I proposed it as reverse was to separate the api-specific hypermedia from the actual resource properties within a representation.

@ghost
Copy link

ghost commented Jan 7, 2023

Oh dear - so many comments I don't know where to start.

@tpluscode

Existing implementations which do not understand the "availability" will inevitably interpret as an inline operation.

Fair point - maybe introducing hydra:ForbiddenOperation is better. I don't like the hydra:target approach - I'd prefer to stick the forbidden operation to the resource. We had some discussions about actions, but it died several years ago and I don't feel we want to dig it out now.

I agree, though I think forbidden is not the right word here and UnavailableOperation would be better because the word "forbidden" is taken by 403 forbidden, so one would expect that they are forbidden because of authorization reasons, which is not necessarily always true. Separating them from available operations while keeping backward compatibility would be nice.

@alien-mcl
Copy link
Member Author

Ok @Inf3rno, is that direction OK for you. Do we have an agreement that pure additive approach is not an option for us here?

@ghost
Copy link

ghost commented Jan 7, 2023

@alien-mcl Yes I like it. Sure, I agree.

@alien-mcl
Copy link
Member Author

I've revamped whole approach to reflect what we've agreed on. Now there is retractedOperation predicate instead of availability attached to Operation. I've retracted whole Available named instance as in this approach it would be strange to have a retractedOperation with reason of Available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants