PATCH method support

[seldon1000]

First, I'm sorry if this has already been asked before but I couldn't find any reference in the repo.
This library is extremely useful and makes it very easy to deal with a Nextcloud account. However, it's lacking PATCH method support and it's been kind of a headache for me.
I'm developing a client for Passwords (https://git.mdns.eu/nextcloud/passwords), a simple yet feature rich password manager for Nextcloud. It's mainly for a university project, but I will personally use it and I'd like to release it publicly (open sourced, of course). According to Passwords documentation, third party clients can edit passwords using a PATCH request. Since SSO library is missing PATCH support, I'm stuck with a pretty inappropriate procedure (premise I'm not using retrofit):

• Create a completely new password with updated data through a POST request, which only returns the ID of the newly created password;
• Pass that ID as a parameter to get all the details of that new password through another POST request (details produced by the server that can't be modified by the user in any way);
• Delete the old password through a DELETE request.

Code looks like this (Kotlin), I don't know if there are better workarounds:

val createRequest = NextcloudRequest.Builder()
    .setMethod("POST")
    .setUrl("$endpoint/password/create").setParameter(params)
    .build()

try {
    val response = JSONObject(
        nextcloudApi!!.performNetworkRequest(createRequest).bufferedReader()
            .lines()
            .collect(Collectors.joining("\n"))
    ).getString("id")

    val showRequest = NextcloudRequest.Builder()
        .setMethod("POST")
        .setUrl("$endpoint/password/show").setParameter(mapOf("id" to response))
        .build()

    val updatedPassword = Password(
        JSONObject(
            nextcloudApi!!.performNetworkRequest(listRequest).bufferedReader()
                .lines()
                .collect(Collectors.joining("\n"))
        )
    )

    updatedPassword.index = index
    storedPasswordsState.value[index] = updatedPassword

    val deleteRequest = NextcloudRequest.Builder()
        .setMethod("DELETE")
        .setUrl("$endpoint/password/delete")
        .setParameter(mapOf("id" to password.id))
        .build()

    nextcloudApi!!.performNetworkRequest(deleteRequest)
} catch (e: Exception) {
    showError()
}

The deleted password goes to trash, which is pretty confusing considering that the intention was just to edit it. Also, the ID of a password should remain the same, but the newly created password comes with a new ID that can't be modified afterwards. Finally, the creation date, which again should remain untouched, changes, even if the password has been on the server for weeks.

This is obviously not ideal. It works for me, but I can't release the app in this state. I'm thinking of dropping the SSO library in favor of a more traditional approach, but that would mean a lot of work in a field I have pretty much no experience in (I actually tried it before discovering this library, made some progress but it was a pain). Before proceeding, I just wanted to ask if there is any plan to support PATCH method in the near future.

[tobiasKaminsky]

Nice that you want to use it :)

What does PATCH need for you to work?
Is this HTTP PATCH or webDav PATCH?

[seldon1000]

@tobiasKaminsky It's http PATCH. I should have mentioned it before. From Passwords documentation, the update action creates a new revision of a password with an updated set of attributes and it's done through a http PATCH request. It needs several arguments (string, boolean...) and treats them like json objects, some are required by the server to execute the action, some are optional. Success status code is classic 200.

Ideal code would be something like this:

val params = mapOf(
        "id" to myPassword.id,     //required, immutable
        "password" to newPassword, //required, mutable
        "label" to newLabel,       //required, mutable
        "username" to newUsername, //optional, mutable
        "url" to newUrl,           //optional, mutable
        "notes" to newNotes        //optional, mutable
    )

val updateRequest = NextcloudRequest.Builder()
    .setMethod("PATCH")
    .setUrl("$endpoint/password/update")
    .setParameter(params)
    .build()

nextcloudApi!!.performNetworkRequest(updateRequest)

[tobiasKaminsky]

It seems that our old http library does not have a PATCH method.
But it seems that you can "just" use POST:
https://developer.salesforce.com/forums/?id=906F00000009CYWIA2

[seldon1000]

@tobiasKaminsky thank you. I read the tip you linked but I don't understand how I can do something similar using SSO library. I tried simply setting POST as request method like this:

val updateRequest = NextcloudRequest.Builder()
    .setMethod("POST")
    .setUrl("$endpoint/password/update")
    .setParameter(params)
    .build()

However, it just fails throwing a NextcloudHttpRequestFailedException, with a method-not-allawed status-code 405.

[David-Development]

The docs state that the update endpoint requires the PATCH method (Docs). So in order for POST requests to go through the API needs to allow the POST method for that particular endpoint first.

The only solution that I can think of right now is to use the method overwrite headers. However afaik that also requires the server to accept them. But it might be worth a try. So you'd need to add an header to the SSO requests that's called X-HTTP-Method-Override=PATCH. (Ref)

[seldon1000]

The server I'm using for tests is preconfigured by a private company. I asked them if the server accepts method override and they said it does, although I can't be sure of it. I tried with the override header and had no luck, the request keeps returning error 405. Maybe I'm doing it wrong:

.setHeader(mapOf("X-HTTP-Method-Override" to listOf("PATCH")))

Besides, if the override support is up to the individual server configuration, I imagine that the app could not work properly for some users who run servers without method override support, is it correct?

[stefan-niedermann]

After all there are 9 HTTP verbs and i stumbled upon this problem before. @tobiasKaminsky maybe we should check what's needed to support all HTTP verbs properly instead of coming up with workaround which might or might not work? What needs to be done to support all verbs? Is it just an old dependency in the files app or where's the problem at all? You mentioned that fixing the root cause would also fix #199 properly?

[tobiasKaminsky]

Is it just an old dependency in the files app or where's the problem at all?

Yes, in the old network library there is no PATCH.
We could switch to new library (okhttp) for this one.

[stefan-niedermann]

We could switch to new library (okhttp) for this one.

What needs to be done to achieve this? I guess we have to make changes in the Nextcloud files app?
Is it possible to just use OkHttp only for requests performed via the SSO? (to limit the impact of this change)?

[tobiasKaminsky]

I guess we have to make changes in the Nextcloud files app?

Yes, this is only a change in Files app.

Is it possible to just use OkHttp only for requests performed via the SSO? (to limit the impact of this change)?

Yes, this would work, but e.g. MKCOL would require also a switch from jackrabbit to davx5 lib.

[animalillo]

I would like to use sso but seems it's not compatible with my app code, needs patch for me to be useful.

[tobiasKaminsky]

@binsky08 create a PR on files.
Do we need then here on SSO something?

[binsky08]

Don't think so. I built the Nextcloud App with my changes and patch requests through this API worked without any other changes.

[stefan-niedermann]

I see the corresponding PR in the files app has been merged in the meantime 🚀.

The SSO accepts a plain String, so is there anything left we need to do on the SSO side?

@binsky08 if nothing more is necessary, would you mind closing this issue then? 🙂

[binsky08]

@stefan-niedermann there shouldn't be anything else to do, I just haven't had time to test it :)

[binsky08]

@seldon1000 @stefan-niedermann I successfully tested patch requests using the API with the Play Store version of the Nextcloud Files app.
I think the issue can be closed :)