Fix updating ID for resource #3239
Conversation
The front-end is using the client side ID generation to send in parallel all the chunks so not sure it's possible without changing the API (for exemple having a client side UUID then a real UUID saved in the database at the end of the upload process)… https://github.com/datagouv/front-end/blob/700fa2e57f9ae347c7b59e8eb1e5dbbff659a475/utils/datasets.ts#L175-L214 |
There was a problem hiding this comment.
Thank you for the investigation and proposed solution!
I don't think the resource id field is currently set by the frontend.
The uuid header in the multipart isn't currently used to create the Resource object, it is only used to identify the chunks the file belong to.
Comparing the uuid with the resource id shows they differ in this chunk upload test case.
I think we should make sure the resource id field is entirely readonly instead.
| form = api.validate(ResourceForm) | ||
| resource = Resource() | ||
| form = api.validate(ResourceFormWithoutId) | ||
| resource = Resource(id=str(uuid4())) |
There was a problem hiding this comment.
Why are you setting the id explicitly here?
| try: | ||
| dataset.add_resource(resource_b) | ||
| assert False | ||
| except RuntimeError: | ||
| print("Ok!") |
There was a problem hiding this comment.
| try: | |
| dataset.add_resource(resource_b) | |
| assert False | |
| except RuntimeError: | |
| print("Ok!") | |
| with pytest.raises(RuntimeError): | |
| dataset.add_resource(resource_b) |
| @@ -897,6 +900,9 @@ def get(cls, id_or_slug): | |||
| def add_resource(self, resource): | |||
| """Perform an atomic prepend for a new resource""" | |||
| resource.validate() | |||
| if resource.id in [r.id for r in self.resources]: | |||
| raise RuntimeError("Cannot add resource with already existing ID") | |||
There was a problem hiding this comment.
Should it be a RuntimeError by the way?
There was a problem hiding this comment.
I'm not sure about the policy on errors. Do we create a new error type? Do we use something generic?
There was a problem hiding this comment.
I would say we could use a MongoEngineValidationError as you did above?
No description provided.