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

Add HateOAS to Paginated endpoints #46

Merged
merged 16 commits into from
Jul 16, 2020

Conversation

singaltanmay
Copy link
Contributor

This is a demo implementation of HateOAS on a single endpoint getAssembliesByTaxid().

@singaltanmay singaltanmay marked this pull request as draft July 10, 2020 12:14
@singaltanmay singaltanmay added the enhancement New feature or request label Jul 10, 2020
@singaltanmay singaltanmay added this to in progress in contig alias GSoC 2020 via automation Jul 10, 2020
@singaltanmay singaltanmay linked an issue Jul 10, 2020 that may be closed by this pull request
@singaltanmay singaltanmay self-assigned this Jul 10, 2020
@singaltanmay
Copy link
Contributor Author

singaltanmay commented Jul 10, 2020

The application fails to start as HateOAS is missing some configuration file. I have tried searching online for how to fix it but haven't found anythin useful yet. Nevertheless it can still be seen that implementation requires creating 2 dedicated functions buildPagedResources and createPaginationLink for every endpoint. Also there is a significant amount of code inside the controller function to convert a Page into a ResponseEntity with PagedResources. All of this code is either not testable or will need to be re-implemented in the test classes. Is there a better way to design this or is this how it will have to be?

@singaltanmay singaltanmay added help wanted Extra attention is needed question Further information is requested labels Jul 10, 2020
@singaltanmay
Copy link
Contributor Author

HateOAS also needs to use Page instead of Slice because it need to know the page number of the last search result. This means the database has to run the query on the entire db instead of returning the first PAGE_SIZE number of results. Doing this could significantly hurt performance, especially when we eventually start adding hundreds or thousands of scaffolds per assembly.

@singaltanmay singaltanmay removed this from in progress in contig alias GSoC 2020 Jul 10, 2020
@jmmut
Copy link
Collaborator

jmmut commented Jul 10, 2020

regarding the performance, first we have to make it work, then we will evaluate if the performance is good enough or not.

regarding the hateoas error, this one was tough. After several hours debugging, I could fix it and pushed a commit to your branch that also shows how to mock the responses. I haven't added tests that check the pagination links, tough, as I think it will be easier if we do first the endpoint of "assemblies/GCA_x/chromosomes" that will naturally lead to pagination with little test setup.

regarding buildPagedResources and createPaginationLink, I don't think there's so much stuff to do, check the taxid endpoint in my commit. PagedResourcesAssembler is a generic class that spring autowires and we can use for any entity without implementing anything. If we end up doing cross linking (put a link to chromosomes in the assembly response), then it's true we will have to build the links ourselves, but that's for another PR; for pagination links there's barely anything we have to do.

Regarding testing, a dummy mockup like the one I did is enough for most tests (unrelated to pagination). If we want to add a single test to check that the pagination links are correct, then, yes, the mock will need to be a bit more polished, but again, that will be easier to do with "assemblies/GCA_x/chromosomes", so I suggest working on that before testing hateoas pagination links.

@singaltanmay singaltanmay removed help wanted Extra attention is needed question Further information is requested labels Jul 12, 2020
@singaltanmay singaltanmay marked this pull request as ready for review July 13, 2020 11:07
@singaltanmay singaltanmay requested a review from jmmut July 13, 2020 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add HateOAS to paginated endpoints
2 participants