Add endpoints for file object and info key editing

[nagem]

New endpoints:

Note: using either of these endpoints does not trigger jobs based on gear rules as per previous discussion This endpoint should spawn jobs

PUT /api/container/container_id/files/file_name

Endpoint that allows editing of file object.
Allowable fields:

• type
• measurements *
• modality

* Must be an array, replaces existing array

Example Request

PUT /api/acquisitions/5984d10adf01e2001433dde8/files/4784_1_1_localizer.dicom.zip HTTP/1.1
Content-Type: application/json

{
	"type": "dicom",
	"modality": "MR",
	"measurements": ["functional"]
}

POST /api/container/container_id/files/file_name/info

Format of request:

# set or delete required, can have both
{
	"set": {}, 
	"delete": []
}

• "set" is a non-empty map of key : value (values may be scalars or complex objects). All keys are added to the info key if they do not exist, or are replaced by the value if they do exist
• "delete" is a non-empty list of top level keys to be deleted. If key does not exist, request is ignored.

OR

{
	"replace": {}
}

• the value of the "replace" key will be set as the value of the info key for the file. It can be an empty map.

Example Requests

POST /api/acquisitions/5984d10adf01e2001433dde8/files/4784_1_1_localizer.dicom.zip/info HTTP/1.1
Content-Type: application/json

{
	"set": {
		"top_level_key": 123,
		"complex_object_key": {
			"a": 1,
			"b": 2
		}
	},
	"delete": ["keys", "to", "be", "removed"]
}

POST /api/acquisitions/5984d10adf01e2001433dde8/files/4784_1_1_localizer.dicom.zip/info HTTP/1.1
Content-Type: application/json

{
	"replace": {
		"notice": "this is a full replace of the info field"
	}
}

Breaking Changes

• None

Review Checklist

• Tests were added to cover all code changes
• Documentation was added / updated
• Code and tests follow standards in CONTRIBUTING.md

[nagem]

FYI @kofalt, @dpuccetti, @ryansanford

@dpuccetti a review of the syntax and that it will work for frontend clients is all I'll need from you.

[nagem]

@gsfr, @lmperry Is this intention still that editing a file's attributes via these routes will not trigger jobs? That was the intention with the classification change. Let me know if opinions have shifted.

An idea: it could be an opt-in two step process. If the client sends a query param like create_jobs=true, the backend could make the change, create a batch of the proposed jobs that would be created, and let the user review and cancel or launch the batch depending on their choice?

[nagem]

After discussion with @gsfr offline, changes should spawn jobs and it will be the default behavior. Any change to make it optional can come at a later date if necessary.

[kofalt]

Two quick suggestions, then LGTM!

[kofalt]

Suggestion: reject the update if both replace and (delete or set) is not None.

I get that the json schema prevents the handler from sending over a payload that is invalid in that manner, but it make sense (to me at least) that the persistent layer should only allow valid mutations.

[gsfr]

Don't we consider the schemas to be an integral part of the persistence layer? If not, wouldn't we need to add input validation all over the code? Maybe this is different somehow, and I'm not seeing it.

[kofalt]

We had a conversation about this very thing :) Our conclusion is that in general, you don't need to re-implement schemas, but the persistence layer isn't an HTTP layer and at minimum shouldn't expose undefined behavior. This is the rare exception (combining set with replace with delete).

[nagem]

Kofalt and I briefly discussed this. Since it's not business or schema logic, I added an extra check for other parts of the code that may call this function. I'd rather have a descriptive 500 if someone misused it than a mongo error about key-wise vs full key sets. I do agree that business logic/schema validation belongs at the handler levels.

[kofalt]

Excess ending whitespace ¯\(ツ)/¯

[gsfr]

This is an important step forward for us. Great to have it.

[gsfr]

Don't we consider the schemas to be an integral part of the persistence layer? If not, wouldn't we need to add input validation all over the code? Maybe this is different somehow, and I'm not seeing it.

[kofalt]

Changes LGTM!