[24.0] Fix datasets API custom keys encoding

[davelopez]

Requires #17779
Fixes #17729 since the custom keys will be serialized correctly after this.

This is similar to #17779 but this time addressing history contents.

TODO

• Add potential missing fields to models
• Increase test coverage for the use of view and keys query parameters for HDAs and HDCAs.

How to test the changes?

(Select all options that apply)

• I've included appropriate automated tests.
• This is a refactoring of components with existing test coverage.
• Instructions for manual testing are as follows:
  1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[mvdbeek]

I think that's the right thing to do!

[davelopez]

I hit a bit of a roadblock with those "metadata_*" dynamic fields... I've been trying to find a clever solution but to no avail, so I will have to resort back to "extra=allow" for custom HDAs... 😞
The good news is that those fields will likely not use any encoded/decoded value so we are safe from this issue.

[mvdbeek]

resort back to "extra=allow" for custom HDAs.

using partial_model is still a big improvement ... in hindsight it'd have been clever to at least place the arbitrary metadata attributes under a common metadata object. I suppose it would result in a ridiculously large schema if we used create_model to create one model per datatype (which defines the custom metadata_* keys) ? 😆

[davelopez]

in hindsight it'd have been clever to at least place the arbitrary metadata attributes under a common metadata object.

That is exactly what I thought... is it too late to change it? I know it will be a breaking change... but having a simple:

metadata: Optional[Dict[str, Any]]

would certainly make everything cleaner and "easier to discover and handle" for clients.

[mvdbeek]

We could think about it for dev, but I worry that there might be scripts out there that expect stuff like metadata_bam_index in the response, meaning we'd have to add another view if we want to do this.

[mvdbeek]

Do you want to add a test maybe making sure that the dataset id is not an integer ?

[davelopez]

Yes, I'm still working on the tests, I wanted to figure out the HDCA's views and modify the models and tests accordingly.

[davelopez]

OK, I found out that collection serialization does not follow the same patterns as datasets, for example, the "serialization_params" are ignored, and only the view is considered. It seems to be handled here:

galaxy/lib/galaxy/managers/collections_util.py

Line 113 in ea1e84d

def dictify_dataset_collection_instance(

So either, there is no point in supporting partial models for those, or this is an ancient bug.

Update

The serialization behavior also depends on the endpoint we are hitting... there are differences between the "show" and the "index" endpoints when listing the contents of a history it will handle the serialization differently depending on the dev parameter...

galaxy/lib/galaxy/webapps/galaxy/services/history_contents.py

Line 311 in ea1e84d

if params.v == "dev":

[davelopez]

OK, if all tests pass now, it should be ready for review. There were some missing fields in some of the models. I tried to be very careful not to miss anything, but it is hard to tell. It would be great to devise a plan to make all the serialization process more homogeneous.

[davelopez]

This has to be duplicated for now since I couldn't find any common place to put the definition. The other enum is in lib/galaxy/util/hash_util.py

[mvdbeek]

Thanks for cleaning this up @davelopez!