-
Notifications
You must be signed in to change notification settings - Fork 3
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
[ALS-5056] Get Query Site of Origin #148
Conversation
Luke-Sikina
commented
Oct 10, 2023
•
edited
Loading
edited
- Requirement: in order to know where to send data, we need to associate each query with the requester's site
- Create table of site code, site name, site email domain
- When an institutional query request is made:
- Parse the site code from the email of the requesting user
- Add that site code to the query request
- When persisting a query object after starting a query request
- Check for a site code in the query request
- Add it to the query meta if it exists
- S3 status start
- status enum
- add to query meta during copy
f8da791
to
51912b0
Compare
- Requirement: in order to know where to send data, we need to associate each query with the requester's site - Create table of site code, site name, site email domain - When an institutional query request is made: - Parse the site code from the email of the requesting user - Add that site code to the query request - When persisting a query object after starting a query request - Check for a site code in the query request - Add it to the query meta if it exists - Add some extra niceness for later data transfer work - S3 status code
51912b0
to
8221ff9
Compare
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
|
||
public class SiteParsingService { |
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.
Unit test
@@ -56,6 +56,9 @@ public class QueryRequest { | |||
@Schema(hidden = true) | |||
private UUID commonAreaUUID; | |||
|
|||
@Schema(hidden = true) |
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.
Discuss: we (I) are bloating the queryrequest object with GIC specific fields. Maybe a sumtype would be appropriate here? Make QueryRequestTemplate, have QueryRequest extend it, then have a GICQueryRequest with extra fields. Jackson can handle this.
@@ -277,7 +282,9 @@ public QueryStatus queryMetadata(UUID queryId, HttpHeaders headers){ | |||
* and resource specific query (could be a string or a json object) | |||
* @return {@link QueryStatus} | |||
*/ | |||
public QueryStatus institutionalQuery(QueryRequest dataQueryRequest, HttpHeaders headers) { | |||
public QueryStatus institutionalQuery(QueryRequest dataQueryRequest, HttpHeaders headers, String email) { | |||
String siteCode = siteParsingService.parseSiteOfOrigin(email).orElseThrow(() -> new RuntimeException("Bad email")); |
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.
Do I want to throw or just null the field
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.
But document the error better in the logs
|
||
CREATE TABLE `site` ( | ||
`uuid` binary(16) NOT NULL, | ||
`code` varchar(15) COLLATE utf8_bin DEFAULT NULL, |
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.
this should not be unique, same code can map to multiple domains