Checking for undefined attributes that could be sent by the user in the comparison POST request #74
Conversation
waterflow80
changed the title
There was a problem hiding this comment.
Tim can confirm once he's back from holiday, but for what it's worth I think your implementation makes sense and the code looks good. My only suggestion would be to add your example (or a version of it) to the tests as well, as it's useful for understanding the behaviour.
| // "array_elements" attribute | "a" | ||
| for (String attribute: seqColAAttributeSet) { | ||
| // Looping through each attribute of seqcolA, Eg: "sequences", "lengths", etc... | ||
| comparisonResult.putIntoArrayElements("a", attribute, seqColAEntityMap.get(attribute).size()); |
There was a problem hiding this comment.
I was going to say that we should check the that seqColAEntityMap.get(attribute) returns a List but it is currently enforced anyway.
We may need to support in the future sequence collection that contains non-list attributes. But that's for another issue.
There was a problem hiding this comment.
Absolutely, I was going to mention that in the next meeting.
Currently, when providing a single value attribute, the service throws an exception and I didn't know how to handle it (how the spec specifies how should we handle it).
Description
Currently, for the POST comparison endpoint, the seqcol service can only function when the user provides attributes that are already defined in our seqcol service implementation, and it will crash when the user provides an undefined attribute within the seqcol body.
So we're trying to make the service function even it gets undefined attributes from the user in the POST comparison endpoint request's body.
Fixes #70
Type of change
We'll be catching the exception thrown when the service tries to cast the undefined_attribute into a seqcol attribute of one of our implementation's in some of comparison result's parts, and ignore that attribute in that particular part.
Note: the attribute may be ignored for some parts of the comparison but it can appear in other parts (see the example below)
New Comparison Result output example
When providing the following secol level 2 object in the comparison POST body:
{ "sequences": [ "SQ.lZyxiD_ByprhOUzrR1o1bq0ezO_1gkrn", "SQ.vw8jTiV5SAPDH4TEIZhNGylzNsQM4NC9", "SQ.A_i2Id0FjBI-tQyU4ZaCEdxRzQheDevn", "SQ.QXSUMoZW_SSsCCN9_wc-xmubKQSOn3Qb", "SQ.UN_b-wij0EtsgFqQ2xNsbXs_GYQQIbeQ", "SQ.z-qJgWoacRBV77zcMgZN9E_utrdzmQsH", "SQ.9wkqGXgK6bvM0gcjBiTDk9tAaqOZojlR", "SQ.K8ln7Ygob_lcVjNh-C8kUydzZjRt3UDf", "SQ.hb1scjdCWL89PtAkR0AVH9-dNH5R0FsN", "SQ.DKiPmNQT_aUFndwpRiUbgkRj4DPHgGjd", "SQ.RwKcMXVadHZub1qL0Y5c1gmNU1_vHFme", "SQ.1sw7ZtgO9JRb1kUEuhVz1wBix5_8Opci", "SQ.V7DQqMKG7bcyxiMZK9wNjkK-udR7hrad", "SQ.R8nT1N2qQFMc_uVMQUVMw-D2GcVmb5v6", "SQ.DPa_ORXLkGyyCbW9SWeqePfortM-Vdlm", "SQ.koyLEKoDOQtGHjb4r0m3o2SXxI09Z_sI" ], "names": [ "chrI", "chrII", "chrIII", "chrIV", "chrV", "chrVI", "chrVII", "chrVIII", "chrIX", "chrX", "chrXI", "chrXII", "chrXIII", "chrXIV", "chrXV", "chrXVI" ], "lengths": [ 230218, 813184, 316620, 1531933, 576874, 270161, 1090940, 562643, 439888, 745751, 666816, 1078177, 924431, 784333, 1091291, 948066 ], "md5_sequences": [ "6681ac2f62509cfc220d78751b8dc524", "97a317c689cbdd7e92a5c159acd290d2", "54f4a74aa6392d9e19b82c38aa8ab345", "74180788027e20df3de53dcb2367d9e3", "d2787193198c8d260f58f2097f9e1e39", "b7ebc601f9a7df2e1ec5863deeae88a3", "a308c7ebf0b67c4926bc190dc4ba8ed8", "f66a4f8eef89fc3c3a393fe0210169f1", "4eae53ae7b2029b7e1075461c3eb9aac", "6757b8c7d9cca2c56401e2484cf5e2fb", "e72df2471be793f8aa06850348a896fa", "77945d734ab92ad527d8920c9d78ac1c", "073f9ff1c599c1a6867de2c7e4355394", "188bca5110182a786cd42686ec6882c6", "7e02090a38f05459102d1a9a83703534", "232475e9a61a5e07f9cb2df4a2dad757" ], "sorted_name_length_pairs": [ "4KSEeZmiD9VkYC7ecIAsX7aXQggFwp9H", "B2VqaWiRary0j7btzmlGhNuoVPqlE21J", "d5VdLsb-kcdSepxl1dxSNk9KWEwL2bZ2", "FU4O09TlQ0Ls3f-kumtsnAag66MxsaNH", "H9xUzOT5xqMmL-Epq0qARIT3446kTZmR", "hio8YzqYfZVVMcINhQPDVd5xREYAMrzA", "kH3mpix7X2RAEENZYueDqxK0X5m06pan", "lKQL4D5qqnuToblXhGL1ZXMAuR6o6ng4", "NwP-e9miFDExu2aFejtku7KvmGGXla2R", "pFlAwWxDmSH9KkdK_yHBA0xtql0lSmTf", "QveClkmJYVlkucglvpJmON5uTL0Eu0No", "qWT4vrlChGgN5N_w3_-_NyQfkFxn98eN", "sdKiPkMb6nKtNfN_BP-X8284rdYQYDhM", "uKS-fRsOtAxqCTxktf6kUhwsBC9BMT9I", "w7OoScU_vDJJtw4UsGV_hd5u_je0fTei", "w8srPsF7XUe6GMvrLRI5gXX_DYAO3k_O" ], "new_attribute_1": [ "1111111111111111", "2222222222222222", "3333333333333333", "aaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbb" ], "new_attribute_2": ["New_attribute_val"] }We get the following Comparison Result:
{ "digests": { "a": "3mTg0tAA3PS-R1TzelLVWJ2ilUzoWfVq", "b": "rkTW1yZ0e22IN8K-0frqoGOMT8dynNyE" }, "attributes": { "a_and_b": [ "sequences", "names", "lengths", "md5_sequences", "sorted_name_length_pairs" ], "a_only": [], "b_only": [ "new_attribute_1", "new_attribute_2" ] }, "array_elements": { "a": { "lengths": 16, "md5_sequences": 16, "names": 16, "sequences": 16, "sorted_name_length_pairs": 16 }, "b": { "lengths": 16, "md5_sequences": 16, "names": 16, "new_attribute_1": 5, "new_attribute_2": 1, "sequences": 16, "sorted_name_length_pairs": 16 }, "a_and_b": { "lengths": 16, "md5_sequences": 16, "names": 0, "sequences": 16, "sorted_name_length_pairs": 0 }, "a_and_b_same_order": { "lengths": true, "md5_sequences": true, "names": null, "sequences": true, "sorted_name_length_pairs": null } } }We can see that
new_attribute_1andnew_attribute_2appear in some of the comparisonResult's body parts, and don't in other parts where they can't be compared to existing attributes of our implementation.Further discussion
@tcezard So in this PR we made the service able to accept undefined attributes from the user in the POST comparison request, but these attributes will only appear in some parts of the comparison result.
Is this the right implementation or should we completely ignore the undefined attributes given in the comparison result body ? And in that case, what should be the output result (format) of that endpoint ?
Note
This branch is a child of the one of #73, so I think it's better to merge that one first for easier review.