plone / plone.restapi

RESTful API for Plone.
http://plonerestapi.readthedocs.org/
84 stars 73 forks source link

Choose valid id if id is provided for content creation #1613

Open ksuess opened 1 year ago

ksuess commented 1 year ago

By now content creation, or concrete adding created content to its container, fails, if content with this id provided by the api call already exists in its container.

Steps to reprouce: POST request to /mypage with id "foo", etc Another POST request to /mypage with id "foo", etc

The reason, the use case, for this is: In Volto you can create content, that gets an id that clashes with for example routes that only Volto knows, not the backend. Create a page with title "Register". You are redirected after creation to the register form. So if in Volto the id is explicitly provided, like for example by appending "-1": "register-1", on REST api call, this would solve the problem, nearly. Do it twice, and it fails, cause plone.restapi does not apply INameChooser(container).chooseName for a call WITH id.

davisagli commented 1 year ago

Note: This only happens when id is included in the POST, not when it is being created automatically from the title.

I don't think we have consensus on whether it is better to throw an error or normalize the id if the client explicitly requested an id that is not available. It might depend on the use case. There's some related discussion in https://github.com/plone/plone.restapi/issues/1487.

ksuess commented 1 year ago

@davisagli In general, no id should be provided at all. Generating an id is the job of the backend. By now I do not see another solution, than explicitly providing an id to solve issue https://github.com/plone/volto/issues/4486

davisagli commented 1 year ago

In the long run I think a better solution for that issue would be to give frontend routes a path like /:register that cannot collide with content ids. It's a complicated topic. Some names are reserved because they have a special use on the frontend and some are reserved because they have a special use on the backend.

davisagli commented 1 year ago

My own opinion: I think it is best by default for the API to throw an error if the client specifies an id and it is not available exactly as specified. Then the situation is clear. But, I would support adding an option (header or querystring) to tell the API to normalize the id and choose a different one if needed, instead of throwing an error. Then the client has a way to opt in to that if it doesn't care about getting a precise id.

ksuess commented 1 year ago

Great input. Me personnally, for the moment, would opt for your suggestion to prevent name clashes at all by "give frontend routes a path like /:register that cannot collide with content ids."

davisagli commented 1 year ago

The problem with my suggestion is that it's not backwards-compatible. So that's why I said "in the long run". But it could happen for Volto 17, now, if the @plone/volto-team is okay with it. Not sure if I'm stirring up some longstanding bikeshed debate...

BhuvaneshPatil commented 1 year ago

In the long run I think a better solution for that issue would be to give frontend routes a path like /:register that cannot collide with content ids. It's a complicated topic. Some names are reserved because they have a special use on the frontend and some are reserved because they have a special use on the backend.

This is great idea but it has some limitations - For route like - /:register, we consider register as path param. example - '/controlpanel/rules/:id/configure' in this path id is parameter.

davisagli commented 1 year ago

No, I don't mean a variable parameter, I mean the literal string /:register

BhuvaneshPatil commented 1 year ago

Another suggestion for this is, we can add extra property in POST request, that will decide the behavior of id creation ( object creation ).

It can have two values (of course names can be changed) -

second value will be default value. first one will be sent when the generated id will be in routes that clashes with default paths in volto application. For example - register

This value will tell backend that it should consider that id generated by default is duplicate, and generate it again.

This may be not so robust implementation for the use case, various factor can be taken into consideration.

Please share your thought on the same.