Added a metadata table - one-to-many relationship enhanced #83

waterflow80 · 2024-04-20T21:47:22Z

Description

closes #13
This PR has almost the same logic as the one-to-many relationship approach (PR #82), however with a different approach.

Changes

Java

We'll be using a new Java model using the @ElementCollection, @CollectionTable in the SeqcolLevelOneEntity:

...
@ElementCollection(fetch = FetchType.LAZY)
@CollectionTable(name = "seqcol_md", joinColumns =
@JoinColumn(name = "digest", nullable = false, updatable = false))
private Set<SeqColMetadataEntity> metadata;
...

I think this will reduce the complexity of using @One-to-many relationship and managing different entities.
See more in this article.

Data model

The new seqcol_md table will remain almost the same, but without a primary key , only digest as a foreign key. This will allow for multiple entries with same digest (and eventually the other attributes):

(Timestamp is still not working for some reason, we can debug it later).

Endpoints

We added a new endpoint for the metadata:
/eva/webservices/seqcol/collection/{digest}/metadata, which will return the following result.

[
    {
        "sourceIdentifier": "GCA_000146045.2",
        "sourceUrl": null,
        "namingConvention": "UCSC",
        "timestamp": null
    }
]

I would like to hear your feedback about this approach, and about the different techniques, logic, and data structures used.

@ElementCollection

@ElementCollection annotation

tcezard

I agree with the Database design which looks good.
I offered some possible improvement that could be done here or in a subsequent PR.

src/main/java/uk/ac/ebi/eva/evaseqcol/entities/SeqColMetadataEntity.java

tcezard · 2024-04-24T10:45:29Z

src/main/java/uk/ac/ebi/eva/evaseqcol/repo/SeqColLevelOneRepository.java

 import org.springframework.stereotype.Repository;

 import uk.ac.ebi.eva.evaseqcol.entities.SeqColLevelOneEntity;
+import uk.ac.ebi.eva.evaseqcol.entities.SeqColMetadataEntity;


Not being used in this class

src/main/java/uk/ac/ebi/eva/evaseqcol/service/SeqColLevelOneService.java

tcezard · 2024-04-24T10:50:04Z

src/main/java/uk/ac/ebi/eva/evaseqcol/service/SeqColLevelOneService.java

@@ -208,4 +215,28 @@ public List<SeqColExtendedDataEntity<List<Integer>>> constructIntegerListExtData
        return integerListExtendedDataEntities;
    }

+    public List<SeqColMetadataEntity> metadataObjectArrayListToMetadataList(List<Object[]> metadataArray) {


Does this method need to be public ?

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.

tcezard · 2024-04-24T11:00:01Z

src/main/java/uk/ac/ebi/eva/evaseqcol/controller/seqcol/SeqColController.java

@@ -83,6 +83,13 @@ public ResponseEntity<?> getSeqColByDigestAndLevel(
        return new ResponseEntity<>(HttpStatus.NOT_FOUND);
    }

+    @GetMapping(value = "/collection/{digest}/metadata")


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.

{ "sequences": [...], "names": : [...], "length": : [...], "naming_convention": GENBANK, "source_id": "GCA_000001", "source_url": "https://..." "ingested_on": 2024-04-21T12:00:00 }

This could be added to SeqColService.getSeqColByDigestAndLevel

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?

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?

I think that's a very good idea, it preserves the endpoint specified/required by the spec, and extend it with other optional parameters.

apriltuesday

I think using ElementCollection seems reasonable here - if I understand correctly, we can query based on metadata fields (i.e. "give me all the seqcols ingested after X date"), but metadata rows can't be created without a corresponding seqcol and will be automatically deleted if we remove its corresponding seqcol (both of which make sense). And the code is simplified which is always good.

The new seqcol_md table will remain almost the same, but without a primary key , only digest as a foreign key.

Is there still a requirement that the combination of all fields is unique? Based on this paragraph. I think this is what we want, just wanted to clarify.

src/main/java/uk/ac/ebi/eva/evaseqcol/controller/seqcol/SeqColController.java

apriltuesday · 2024-04-24T12:34:22Z

src/main/java/uk/ac/ebi/eva/evaseqcol/controller/seqcol/SeqColController.java

@@ -83,6 +83,13 @@ public ResponseEntity<?> getSeqColByDigestAndLevel(
        return new ResponseEntity<>(HttpStatus.NOT_FOUND);
    }

+    @GetMapping(value = "/collection/{digest}/metadata")


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?

apriltuesday · 2024-04-24T13:23:07Z

src/main/java/uk/ac/ebi/eva/evaseqcol/service/SeqColLevelOneService.java

+
+    public List<SeqColMetadataEntity> getAllMetadata() {
+        List<Object[]> metadataArrayList = repository.findAllMetadata();
+        return metadataObjectArrayListToMetadataList(metadataArrayList);


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.

I've a made a little enhancement in the transformation of the fetched md objects using streams (commit), however using TupleTransformer might be a little bit tricky and require some changes in the design.

This is an example I found online that uses the TupleTransformer:

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.

Makes sense, your implementation is pretty neat so let's keep it like that 👍

…Controller.java Co-authored-by: April Shen <[email protected]>

…to List<MetadataEntity>

…path variable, into a request param within the getSeqcolByDigest endpoint

implemeted the one to many relationship for the metadata table using he

51a95ac

@ElementCollection annotation

waterflow80 requested review from tcezard, apriltuesday and nitin-ebi April 20, 2024 21:47

tcezard requested changes Apr 24, 2024

View reviewed changes

waterflow80 added 2 commits April 24, 2024 14:18

changed timestamp into created_on

5bf0216

remove unused class

85bf1b3

apriltuesday reviewed Apr 24, 2024

View reviewed changes

waterflow80 and others added 4 commits April 24, 2024 15:03

Update src/main/java/uk/ac/ebi/eva/evaseqcol/controller/seqcol/SeqCol…

665237a

…Controller.java Co-authored-by: April Shen <[email protected]>

fix timestamp ingestion

c26668b

enhanced the transformation of the fetched tuple fron List<Object[]> …

fc9b52a

…to List<MetadataEntity>

chnaged the metadata endpoint from an independent endpoint that uses …

ea6e9e6

…path variable, into a request param within the getSeqcolByDigest endpoint

waterflow80 requested review from apriltuesday and tcezard May 17, 2024 11:26

waterflow80 mentioned this pull request May 17, 2024

Complete the source url in the metadata entity #87

Open

apriltuesday approved these changes May 21, 2024

View reviewed changes

tcezard approved these changes May 22, 2024

View reviewed changes

waterflow80 merged commit e9548b9 into EBIvariation:main May 22, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a metadata table - one-to-many relationship enhanced #83

Added a metadata table - one-to-many relationship enhanced #83

waterflow80 commented Apr 20, 2024

tcezard left a comment

tcezard Apr 24, 2024

waterflow80 Apr 24, 2024

tcezard Apr 24, 2024

waterflow80 Apr 24, 2024

tcezard Apr 24, 2024

apriltuesday Apr 24, 2024

waterflow80 Apr 24, 2024

apriltuesday left a comment

apriltuesday Apr 24, 2024

apriltuesday Apr 24, 2024

waterflow80 May 17, 2024

apriltuesday May 21, 2024

Added a metadata table - one-to-many relationship enhanced #83

Added a metadata table - one-to-many relationship enhanced #83

Conversation

waterflow80 commented Apr 20, 2024

Description

Changes

Java

Data model

Endpoints

tcezard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apriltuesday left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment