-
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
Proof-of-concept prototype of crosslinking capability to provide applicable tools for a specific product (/products/{lidvid}/tools
)
#498
base: develop
Are you sure you want to change the base?
Conversation
Thanks @tariqksoliman ! |
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.
Some comments - will look closely at the meat of CrossLinks.java once doco is there
service/for
Outdated
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.
Request: remove extraneous file
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.
Branch needs to be rebased on #489
@tariqksoliman @tloubrieu-jpl this should be simple - if it isn't and you'd like to get this in before taking the time to understand the new approach, let me know and I should be able to sort out the conflict pretty quickly
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.
Docstring please - ideally I should be able to read it and walk away with a good understanding of what it is/does (for the class, and any inobvious methods)
https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html
service/unittests.txt
Outdated
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.
Prefer: remove file and add path to .gitignore
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.
Does this need to be configurable, or is this behaviour a relatively-immutable structured collection of values which could be defined within the Java code?
I'm taking it on faith that loading from a JSON configuration is the way to go here vs defining a constant - as long as that question was considered that's fine.
@tariqksoliman we can talk more about this offline at the next IMG - EN tag-up, but for an operational deployment to enable supporting this at scale across the PDS, we will need to refactor this to use Product_Service, e.g. this one for analyst's notebook. I thought I documented this in the ticket, but looks like this was just an offline discussion at our last meeting. Thanks for the PR! |
/products/{lidvid}/tools
)
@tloubrieu-jpl As request, I've moved cross-links.json into |
Switching this PR to draft for the time being until we are able to get this into a more production-ready state |
Staging
🗒️ Summary
/products/{lidvid}/tools
lidvid -> tool
mappings. Located at/service/cross-links.json
CrossLinksLoader.java
file to load that file on startCrossLinks.java
file to represent that json file and perform the mapping logicproductCrossLinks()
method toProductsController.java
CrossLinks
to get all tool cross-links.⚙️ Test Data and/or Report
Sample curls:
Explanation of cross-links.json
Sample:
injectableParams
: Doesn't nothing but indicate with params can be set in a tool's"base"
url.tools
name
: tool name that just gets passed to the responsebase
: Base template url to deep link to the given lidvid product in the tool.injectableParams
can be templated by wrapping it in curly brackets:{injectableVariable}
description
: tool description that just gets passed to the responsealiases
: Sometimes tools have slightly different names for things. If a tool requires "m20" in their url, aliases allows such mapping.acceptOnly
: Leave as empty array to accept all, otherwise if a lidvid's field's value does not match what's configured here, the tool entry will not show in the response.acceptOnly
s are ANDed together. "match" supports java regexesreject
: Leave as empty array to not specify any rejections. Otherwise if any lidvid's field matches a reject case, the tool entry will not show up in the response. "match" supports java regexes♻️ Related Issues
Fixes #472
Note: This is my first PR and first time using Java in over a decade so please review this with extra caution.