Skip to content
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

Gene ws server v2 #120

Merged
merged 13 commits into from
Aug 5, 2019
Merged

Conversation

junaidnz97
Copy link
Contributor

The purpose of this PR is to add an endpoint "/v2/genes/{geneIds}" which retrieves a list of FeatureCoordinates for the given geneIds.

public GeneWSServerV2() {
}

@GetMapping(value = "/{geneIds}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea is to return the variants in the specified genes instead of returning the gene's coordinates. So change this to /v2/genes/{geneIds}/variants and return the variants using the coordinates

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed :)

MultiMongoDbFactory.setDatabaseNameForCurrentThread(DBAdaptorConnector.getDBName(species + "_" + assembly));
List<FeatureCoordinates> featureCoordinates = service.findAllByGeneIdsOrGeneNames(geneIds, geneIds);
if (featureCoordinates.size() == 0) {
return new ResponseEntity(featureCoordinates, HttpStatus.NOT_FOUND);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my opinion, an empty list is more a "204 no content" than a "404 not found"

Copy link
Contributor Author

@junaidnz97 junaidnz97 Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay :)
Fixed 👍

return new ResponseEntity(featureCoordinates, HttpStatus.NOT_FOUND);
}
String regions = "";
for (int i = 0; i < featureCoordinates.size(); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of this loop, use

        String regions = featureCoordinates.stream().map(this::getRegionString).collect(Collectors.joining(","));

I prefer the stream version not only for being shorter and with less redundancy, but also because conditions like if (i != featureCoordinates.size() - 1) are confusing and prone to bugs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed 👍

variantList.forEach(variant -> {
chromosomes.add(variant.getChromosome());
});
return chromosomes;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is called testGetVariantsGeneHelper but is returning chromosomes. This should return variants

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed :)

junaidnz97 and others added 2 commits August 1, 2019 19:48
Co-Authored-By: andresfsilva <afsilva04@gmail.com>
@andresfsilva andresfsilva merged commit 3eb2018 into EBIvariation:master Aug 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants