-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added a metadata table - one-to-many relationship enhanced #83
Changes from 1 commit
51a95ac
5bf0216
85bf1b3
665237a
c26668b
fc9b52a
ea6e9e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package uk.ac.ebi.eva.evaseqcol.entities; | ||
|
||
import lombok.Data; | ||
import org.hibernate.annotations.CreationTimestamp; | ||
|
||
import javax.persistence.Column; | ||
import javax.persistence.Embeddable; | ||
import javax.persistence.EnumType; | ||
import javax.persistence.Enumerated; | ||
import javax.persistence.Temporal; | ||
import javax.persistence.TemporalType; | ||
import java.util.Date; | ||
|
||
@Data | ||
@Embeddable | ||
public class SeqColMetadataEntity { | ||
|
||
@Column(name = "source_id") | ||
private String sourceIdentifier; // Eg: INSDC Acession | ||
|
||
@Column(name = "source_url") | ||
private String sourceUrl; | ||
|
||
@Enumerated(EnumType.STRING) | ||
@Column(name = "naming_convention") | ||
private SeqColEntity.NamingConvention namingConvention; | ||
|
||
@Column(name = "timestamp", updatable = false, columnDefinition="TIMESTAMP DEFAULT CURRENT_TIMESTAMP") | ||
tcezard marked this conversation as resolved.
Show resolved
Hide resolved
|
||
@Temporal(TemporalType.TIMESTAMP) | ||
@CreationTimestamp | ||
private Date timestamp; | ||
|
||
public SeqColMetadataEntity setNamingConvention(SeqColEntity.NamingConvention namingConvention) { | ||
this.namingConvention = namingConvention; | ||
return this; | ||
} | ||
|
||
public SeqColMetadataEntity setSourceIdentifier(String sourceIdentifier) { | ||
this.sourceIdentifier = sourceIdentifier; | ||
return this; | ||
} | ||
|
||
public SeqColMetadataEntity setSourceUrl(String sourceUrl) { | ||
this.sourceUrl = sourceUrl; | ||
return this; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,13 @@ | ||
package uk.ac.ebi.eva.evaseqcol.repo; | ||
|
||
import org.springframework.data.jpa.repository.JpaRepository; | ||
import org.springframework.data.jpa.repository.Query; | ||
import org.springframework.stereotype.Repository; | ||
|
||
import uk.ac.ebi.eva.evaseqcol.entities.SeqColLevelOneEntity; | ||
import uk.ac.ebi.eva.evaseqcol.entities.SeqColMetadataEntity; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not being used in this class There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
import java.util.List; | ||
|
||
@Repository | ||
public interface SeqColLevelOneRepository extends JpaRepository<SeqColLevelOneEntity, String> { | ||
|
@@ -14,4 +18,10 @@ public interface SeqColLevelOneRepository extends JpaRepository<SeqColLevelOneEn | |
void removeSeqColLevelOneEntityByDigest(String digest); | ||
|
||
void deleteAll(); | ||
|
||
@Query(value = "select source_id, source_url, naming_convention, timestamp from seqcol_md where digest = ?1", nativeQuery = true) | ||
List<Object[]> findMetadataBySeqColDigest(String digest); | ||
|
||
@Query(value = "select source_id, source_url, naming_convention, timestamp from seqcol_md", nativeQuery = true) | ||
List<Object[]> findAllMetadata(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,14 +8,17 @@ | |
import uk.ac.ebi.eva.evaseqcol.entities.SeqColLevelOneEntity; | ||
import uk.ac.ebi.eva.evaseqcol.digests.DigestCalculator; | ||
import uk.ac.ebi.eva.evaseqcol.entities.SeqColLevelTwoEntity; | ||
import uk.ac.ebi.eva.evaseqcol.entities.SeqColMetadataEntity; | ||
import uk.ac.ebi.eva.evaseqcol.repo.SeqColLevelOneRepository; | ||
import uk.ac.ebi.eva.evaseqcol.utils.JSONExtData; | ||
import uk.ac.ebi.eva.evaseqcol.utils.JSONIntegerListExtData; | ||
import uk.ac.ebi.eva.evaseqcol.utils.JSONLevelOne; | ||
import uk.ac.ebi.eva.evaseqcol.utils.JSONStringListExtData; | ||
|
||
import java.io.IOException; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.Date; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Optional; | ||
|
@@ -62,12 +65,17 @@ public List<SeqColLevelOneEntity> getAllSeqColLevelOneObjects(){ | |
|
||
/** | ||
* Construct a seqCol level 1 entity out of three seqCol level 2 entities that | ||
* hold names, lengths and sequences objects*/ | ||
* hold names, lengths and sequences objects | ||
* TODO: Change the signature of this method and make it accept metadata object instead of namingconvention and source id*/ | ||
public SeqColLevelOneEntity constructSeqColLevelOne(List<SeqColExtendedDataEntity<List<String>>> stringListExtendedDataEntities, | ||
List<SeqColExtendedDataEntity<List<Integer>>> integerListExtendedDataEntities, | ||
SeqColEntity.NamingConvention convention) throws IOException { | ||
SeqColEntity.NamingConvention convention, String sourceId) throws IOException { | ||
SeqColLevelOneEntity levelOneEntity = new SeqColLevelOneEntity(); | ||
JSONLevelOne jsonLevelOne = new JSONLevelOne(); | ||
SeqColMetadataEntity metadata = new SeqColMetadataEntity() | ||
.setNamingConvention(convention) | ||
.setSourceIdentifier(sourceId); | ||
levelOneEntity.addMetadata(metadata); | ||
|
||
// Looping over List<String> types | ||
for (SeqColExtendedDataEntity<List<String>> dataEntity: stringListExtendedDataEntities) { | ||
|
@@ -99,14 +107,13 @@ public SeqColLevelOneEntity constructSeqColLevelOne(List<SeqColExtendedDataEntit | |
levelOneEntity.setSeqColLevel1Object(jsonLevelOne); | ||
String digest0 = digestCalculator.getSha512Digest(levelOneEntity.toString()); | ||
levelOneEntity.setDigest(digest0); | ||
levelOneEntity.setNamingConvention(convention); | ||
return levelOneEntity; | ||
} | ||
|
||
/** | ||
* Construct a Level 1 seqCol out of a Level 2 seqCol*/ | ||
public SeqColLevelOneEntity constructSeqColLevelOne( | ||
SeqColLevelTwoEntity levelTwoEntity, SeqColEntity.NamingConvention convention) throws IOException { | ||
SeqColLevelTwoEntity levelTwoEntity, SeqColEntity.NamingConvention convention, String sourceId) throws IOException { | ||
DigestCalculator digestCalculator = new DigestCalculator(); | ||
JSONExtData<List<String>> sequencesExtData = new JSONStringListExtData(levelTwoEntity.getSequences()); | ||
JSONExtData<List<Integer>> lengthsExtData = new JSONIntegerListExtData(levelTwoEntity.getLengths()); | ||
|
@@ -151,7 +158,7 @@ public SeqColLevelOneEntity constructSeqColLevelOne( | |
lengthsExtEntity | ||
); | ||
|
||
return constructSeqColLevelOne(stringListExtendedDataEntities,integerListExtendedDataEntities, convention); | ||
return constructSeqColLevelOne(stringListExtendedDataEntities,integerListExtendedDataEntities, convention, sourceId); | ||
} | ||
|
||
/** | ||
|
@@ -208,4 +215,28 @@ public List<SeqColExtendedDataEntity<List<Integer>>> constructIntegerListExtData | |
return integerListExtendedDataEntities; | ||
} | ||
|
||
public List<SeqColMetadataEntity> metadataObjectArrayListToMetadataList(List<Object[]> metadataArray) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this method need to be public ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not necessarily, it can be private in our case. However I'm not sure if we might need to used it outside of this class in the future. |
||
List<SeqColMetadataEntity> metadataList = new ArrayList<>(); | ||
for (Object[] metadataElements : metadataArray) { | ||
SeqColMetadataEntity metadataEntity = new SeqColMetadataEntity(); | ||
metadataEntity.setSourceIdentifier((String) metadataElements[0]); | ||
metadataEntity.setSourceUrl((String) metadataElements[1]); | ||
metadataEntity.setNamingConvention(SeqColEntity.NamingConvention.valueOf( | ||
(String) metadataElements[2] | ||
)); | ||
metadataEntity.setTimestamp((Date) metadataElements[3]); | ||
tcezard marked this conversation as resolved.
Show resolved
Hide resolved
|
||
metadataList.add(metadataEntity); | ||
} | ||
return metadataList; | ||
} | ||
|
||
public List<SeqColMetadataEntity> getAllMetadata() { | ||
List<Object[]> metadataArrayList = repository.findAllMetadata(); | ||
return metadataObjectArrayListToMetadataList(metadataArrayList); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth using something like TupleTransformer in the repository to do this? I'm not sure how much difference it makes in practice compared to your conversion method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've a made a little enhancement in the transformation of the fetched md objects using streams (commit), however using This is an example I found online that uses the List<PostRecord> postRecords = entityManager.createQuery("""
select
p.id,
p.title,
p.createdOn,
p.createdBy,
p.updatedOn,
p.updatedBy
from Post p
order by p.id
""")
.unwrap(org.hibernate.query.Query.class)
.setTupleTransformer(
(tuple, aliases) -> {
int i =0;
return new PostRecord(
longValue(tuple[i++]),
stringValue(tuple[i++]),
new AuditRecord(
localDateTimeValue(tuple[i++]),
stringValue(tuple[i++]),
localDateTimeValue(tuple[i++]),
stringValue(tuple[i++])
)
);
}
)
.getResultList(); Since we're using an interface repository, implementing this logic might require adding an additional repository class that will execute the custom query. Not sure if that's gonna make the design more complex or not... Idk. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, your implementation is pretty neat so let's keep it like that 👍 |
||
} | ||
|
||
public List<SeqColMetadataEntity> getMetadataBySeqcolDigest(String digest) { | ||
List<Object[]> metadataArrayList = repository.findMetadataBySeqColDigest(digest); | ||
return metadataObjectArrayListToMetadataList(metadataArrayList); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the rational for making the metadata available for a given digest and we could keep it for now.
However I think the metadata should injected in the SeqCol level2 to and be added with their own properties.
This could be added to
SeqColService.getSeqColByDigestAndLevel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also be added on request via a path variable - e.g.
/collection/{digest}?level=2&metadata=true
- in case anyone wants the "plain" level 2 object?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a very good idea, it preserves the endpoint specified/required by the spec, and extend it with other optional parameters.