added the sorted-name-length-pair attribute to the model #30
Conversation
src/main/java/uk/ac/ebi/eva/evaseqcol/model/NameLengthPairEntity.java
Outdated
Show resolved
Hide resolved
| " \"name\": \""+ name +"\",\n" + | ||
| " \"length\": \""+ length +"\",\n" + | ||
| return "{" + | ||
| " \"name\": \""+ name +"\"," + |
There was a problem hiding this comment.
Do we still need the leading tabspace before the attributes?
There was a problem hiding this comment.
I don't think it's referring to a tabspace, but rather to an escape character \ to keep the double quotes that surrounds each attribute's name, which is required by the specification as far as I know.
There was a problem hiding this comment.
I meant the whitespace between the double quotes and the first backslash...
There was a problem hiding this comment.
Ohh okay, sorry.
I don't think we need that, although it's gonna be removed in the canonicalisation process. But it makes sense to remove it to reduce the effort for that function.
| SHA512ChecksumCalculator sha512ChecksumCalculator = new SHA512ChecksumCalculator(); | ||
| List<String> sortedNameLengthPairs = new ArrayList<>(); | ||
| for (NameLengthPairEntity entity: nameLengthPairList) { | ||
| String nameLengthHash = sha512ChecksumCalculator.calculateChecksum(entity.toString()); |
There was a problem hiding this comment.
Should this use the DigestCalculator so that the string is JSON canonicalized? Then you can keep the toString method human-readable. It also seems to be what the python implementation does.
There was a problem hiding this comment.
Absolutely, that's a good point.
I recently figured that out and created a to-do issue #32 to fix it.
There was a problem hiding this comment.
Ah sorry, didn't see the issue - thanks!
| @@ -0,0 +1,25 @@ | |||
| package uk.ac.ebi.eva.evaseqcol.model; | |||
There was a problem hiding this comment.
Is there a reason why this is in model rather than entities?
There was a problem hiding this comment.
I just wanted to separate intermediate classes from the seqCol's and assembly's entities, which are mostly persistent.
However, it might not be the best approach.
It maybe better to put it in entities.
There was a problem hiding this comment.
I'm fine with the separation, just wanted to understand the reason - it's true that there's a lot of things in entities now!
waterflow80
force-pushed
the
sorted-name-length-pair
branch
from
4ada668
to
1438fb2
Compare
Added the
sorted-name-length-pairsattribute to the model.You can test the endpoints at
PUT: SERVER_IP/eva/webservices/seqcol/collection/admin/seqcols/GCA_000146045.2, or atGET: /eva/webservices/seqcol/collection/eJ8GCVLEVtdnCN4OSqfkf6KoEOK9OUlr?level=2.