[sharing-ng] status code for multiple mount and unmount share

[S-Panta]

Describe the bug

Mounting the share multiple times results in 400 status code. BUt this request isn't a bad request but conflict in request and thus should be 409. As RFC states in https://datatracker.ietf.org/doc/html/rfc7231#section-6.5.8

The 409 (Conflict) status code indicates that the request could not
   be completed due to a conflict with the current state of the target
   resource.  

Same goes for unmounting share multiple times. At the moment, oCIS returns 424 failed dependency status code. MDN docs writes

The HTTP 424 Failed Dependency client error response code indicates that the method could not be performed on the resource because the requested action depended on another action, and that action failed.

What are the appropriate status code for these two request?

Steps to reproduce

1.Login to ocis as admin 2.create a folder test and share to einstein 3. click disable sync. 4. Send API request to mount resource. 5. send the request again to mount resource

curl -X POST 'https://host.docker.internal:9200/graph/v1beta1/drives/{share-Space-Id}/root/children'  
-d '{
    "remoteItem": {
        "id":"'item-id"
    }
}' -ueinstein:relativity

[S-Panta]

@rhafer @2403905

[S-Panta]

At the moment, multiple unmount request gives 400 status code.

[2403905]

@S-Panta I added the PR and some tests have been affected. Please validate the test result and clarify what is the suitable response code (400 or 404) in the given scenarios ?

runsh: Total unexpected failed scenarios throughout the test run:
apiSharingNg/enableDisableShareSync.feature:302
apiSharingNg/enableDisableShareSync.feature:378
apiSharingNg/enableDisableShareSync.feature:465
apiSharingNg/enableDisableShareSync.feature:547
apiSharingNg/enableDisableShareSync.feature:597

[saw-jan]

@S-Panta I added the PR and some tests have been affected. Please validate the test result and clarify what is the suitable response code (400 or 404) in the given scenarios ?

runsh: Total unexpected failed scenarios throughout the test run:
apiSharingNg/enableDisableShareSync.feature:302
apiSharingNg/enableDisableShareSync.feature:378
apiSharingNg/enableDisableShareSync.feature:465
apiSharingNg/enableDisableShareSync.feature:547
apiSharingNg/enableDisableShareSync.feature:597

404 for these scenarios looks valid

[S-Panta]

imo should be 404 as the shared resource gets deleted in those scenarios.

[2403905]

But it collides with previous fix https://github.com/owncloud/ocis/issues/8724

[S-Panta]

The bad request can only be the case if the value of resource-id doesn't match the format or is empty.

https://github.com/owncloud/ocis/blob/590a40d979c73d73f4cb72314135b6445480c6c2/tests/acceptance/features/apiSharingNg/enableDisableShareSync.feature#L302 Here we enable sync where there is no resource shared. Thus, it should be 404. Are there any tests to cover the scenario where the id is in a weird format? @saw-jan

[saw-jan]

But it collides with previous fix #8724

hmm, yeah @rhafer commented this for creating a drive item with random item id (https://github.com/owncloud/ocis/issues/8724)

I think this should return a 400 (Bad Request)

Should there be differences in /non-existing-file-id and /random-string? If yes then we have to test accordingly

[rhafer]

Should there be differences in /non-existing-file-id and /random-string? If yes then we have to test accordingly

No. Please don't. 400 was a bad idea. 404 in both cases is better I think.

[2403905]

Should there be differences in /non-existing-file-id and /random-string? If yes then we have to test accordingly

No. Please don't. 400 was a bad idea. 404 in both cases is better I think.

It would be great to keep the response codes in conformity. If I understand correctly in case v1beta1/drives/{drive-id}/items/{item-id} if drive-id or item-id is wrong we should respond with 404 because it is a part of the resource path. If the body payload or get parameter is wrong we should respond with 400. @rhafer @micbar Correct?

UPD: I discussed this with @butonic and he suggested if drive-id or item-id is a URL parameter

• respond 400 if the parameter is empty or malformed.
• respond 404 if the parameter has a valid format but the destination does not exist. upd: Also we should translate the gateway error.

[2403905]

Could there be problems with the fact that the proxy can cache 404?