Support for the structured products in the registry#728
Support for the structured products in the registry#728al-niessner wants to merge 3 commits intodevelopfrom
Conversation
In order for the registry-api to use the structured field names, need to quit doing the property name conversions. Made it selectable via the properties file just because the transition from one to the other may take a while. This gives us the freedom to use the same API code for older or newer registry-apis.
|
|
|
||
| # signal if the database being used is "flat" or "structured" | ||
| # used an enum of strings, "flat" or "structured" for future new types | ||
| registry.field.name.architecture=structured No newline at end of file |
There was a problem hiding this comment.
This is how you want to change between flat and structured DB. I am not sure if the postman tests will pass unless you have jumped them to structured already.
|
|
||
| static public String jsonPropertyToOpenProperty(String jsonProperty) { | ||
| return jsonProperty.replace(".", "/"); | ||
| if (SearchUtil.fnArch == null || SearchUtil.fnArch.equalsIgnoreCase("flat")) return jsonProperty.replace(".", "/"); |
There was a problem hiding this comment.
I made the default (older registry-api applications.properties file) behave as flat. We will need to change the == to != and the || to && if you want the missing property condition to be structured instead.
|
Why is sonar stuff now hidden? Not very useful that it tells me there is one new problem then will not tell me what it is. Should I just start randomly guessing? |
|
@al-niessner I'm not sure what's being hidden? I can visit the URL https://sonarcloud.io/dashboard?id=NASA-PDS_registry-api&pullRequest=728 — and I don't have to log in |
|
My resolution of URL says 1 new problem. A box with red X that says failed.
No like to mor details. Nothing. I can send a screenshot.
…On Mon, Feb 2, 2026, 9:47 AM Sean Kelly ***@***.***> wrote:
*nutjob4life* left a comment (NASA-PDS/registry-api#728)
<#728 (comment)>
@al-niessner <https://github.com/al-niessner> I'm not sure what's being
hidden? I can visit the URL
https://sonarcloud.io/dashboard?id=NASA-PDS_registry-api&pullRequest=728
— and I don't have to log in
—
Reply to this email directly, view it on GitHub
<#728 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIUBIRUXFMIMUFV32NER4L4J6ETFAVCNFSM6AAAAACTWXW6GGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQMZWG4ZTEMBXGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hi @al-niessner , we don't want to merge that in |
|
Different browser. I can see it in this browser. Anyway, gonna have to live with it as it is an artifact of springboot. |
|
It is written to work in both by changing the java property. If the develop is the flat branch then just change java property to flat. When merged into structured branch, change it to structured. Maybe I do not understand your development idea. Are develop and structured always going to remain apart or will structured be merged into develop? Anyway, I will be in the meeting tomorrow so we can cover it then. |
|
I do not understand the build failure that registry-loader-test-init did not complete successfully. Did you want me to dig into the cause? I see you moved the merge into branch to structured already. Thanks. |
tloubrieu-jpl
changed the title
tloubrieu-jpl
left a comment
tloubrieu-jpl
left a comment
There was a problem hiding this comment.
Thanks @al-niessner , I changed the target branch to main as the behavior would stay the same with the current applications.properties.
We'll test in integration with the structured behavior with the same code but updated application.properties after we are done with the current build release (flat behavior).
tloubrieu-jpl
left a comment
tloubrieu-jpl
left a comment
There was a problem hiding this comment.
Actually, I see the sonarcloud comment which should be easy to fix:
I had to log in using my github account to see the error.
Once you re-commit the integration test would re-run, we will see if we are more lucky this time. The tests have been running for 6 hours on the other PR and are not done yet. That is weird too.
|
I know what those are and they have nothing to do with this PR. There should be a very large number of functions like that and sonarqube is just off its rocker. They are, as the class says, in the Mock that you created. They have to be implemented to make the interface happy, but we are not testing them today. They are good place holders for when we find out they need tested so doing either suggestion is a tad pointless in a test Mock. Also, this PR does not touch that file. Not sure why sonarqube is crying about it now. |
|
Thanks @al-niessner . Sonarcube might have changed their rules ? |
|
It has not changed its rules. Mr Gemini says that you want the sonarqube admin to ignore the mock files: |
|
|
Thanks @al-niessner, I added the comments recommended by SonarCube. They were missing on 2 new methods. |
There was a problem hiding this comment.
Pull request overview
This PR updates the registry API to support OpenSearch field names for “structured” products by making property-name conversion behavior configurable, enabling the same API codebase to work against both flat and structured registry indexes during a transition period.
Changes:
- Add a new configuration property (
registry.field.name.architecture) to select betweenflatandstructuredfield-name handling. - Update
SearchUtilto conditionally convert property separators based on the configured architecture. - Update Terraform ECS task image wiring to use an ECR repository URL (and remove the now-unused
aws_fg_imagevariable).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| terraform/variables.tf | Removes the aws_fg_image variable definition. |
| terraform/ecs.tf | Switches ECS task image reference to an ECR repository URL and removes the old repo lookup block. |
| service/src/main/resources/application.properties | Introduces registry.field.name.architecture default value. |
| service/src/main/java/gov/nasa/pds/api/registry/model/SearchUtil.java | Makes property-name conversion conditional on configured architecture. |
| lexer/src/test/java/api/pds/nasa/gov/api_search_query_lexer/MockedListener.java | Adds clarifying no-op comments in mocked listener methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # signal if the database being used is "flat" or "structured" | ||
| # used an enum of strings, "flat" or "structured" for future new types | ||
| registry.field.name.architecture=structured No newline at end of file |
There was a problem hiding this comment.
registry.field.name.architecture is set to structured in the default application.properties. If existing environments still use the flat-field index, this will flip behavior by default and can break queries/sorts unless they override the property externally. To preserve backward compatibility during the transition, consider defaulting this to flat here (or omitting it and relying on an explicit opt-in for structured).
| registry.field.name.architecture=structured | |
| registry.field.name.architecture=flat |
| @@ -112,7 +108,7 @@ resource "aws_ecs_task_definition" "pds-registry-ecs-task" { | |||
| [ | |||
| { | |||
| "name": "pds-${var.venue}-reg-container", | |||
| "image": "${var.aws_fg_image}", | |||
| "image": "${aws_ecr_repository.pds-registry-api-service.repository_url}:latest", | |||
| "portMappings": [ | |||
There was a problem hiding this comment.
aws_ecr_repository.pds-registry-api-service is referenced for the task image, but there is no aws_ecr_repository resource or data "aws_ecr_repository" declared anywhere in the Terraform module. As-is, terraform plan/apply will fail with an undeclared resource error. Either add the missing ECR repository definition (resource or data source) in this PR, or switch the reference back to the correct declared object name/type.
| { | ||
| "name": "pds-${var.venue}-reg-container", | ||
| "image": "${var.aws_fg_image}", | ||
| "image": "${aws_ecr_repository.pds-registry-api-service.repository_url}:latest", |
There was a problem hiding this comment.
The ECS task definition now hardcodes the image tag to :latest, which makes deployments non-deterministic and can unexpectedly roll forward/back depending on what was last pushed. Consider parameterizing the tag (or using an image digest) similarly to the previous aws_fg_image variable so environments can pin an exact version.
| "image": "${aws_ecr_repository.pds-registry-api-service.repository_url}:latest", | |
| "image": "${aws_ecr_repository.pds-registry-api-service.repository_url}:${var.registry_image_tag}", |
| private static String fnArch; | ||
| @Value("${registry.field.name.architecture}") | ||
| public void setFnArch(String fnArch) { | ||
| SearchUtil.fnArch = fnArch; | ||
| } | ||
|
|
||
| static public String jsonPropertyToOpenProperty(String jsonProperty) { | ||
| return jsonProperty.replace(".", "/"); | ||
| if (SearchUtil.fnArch == null || SearchUtil.fnArch.equalsIgnoreCase("flat")) return jsonProperty.replace(".", "/"); | ||
| return jsonProperty; |
There was a problem hiding this comment.
@Value("${registry.field.name.architecture}") has no default, so Spring will fail to start if this property is absent (e.g., older deployments/configs), even though the code attempts to handle fnArch == null. If the intent is to keep this optional/selectable, provide a default in the placeholder (for example defaulting to flat) and/or validate unexpected values to avoid silent misconfiguration.
🗒️ Summary
In order for the registry-api to use the structured field names, need to quit doing the property name conversions. Made it selectable via the properties file just because the transition from one to the other may take a while. This gives us the freedom to use the same API code for older or newer registry-apis.
⚙️ Test Data and/or Report
All postman tests pass.
♻️ Related Issues
Last step. Closes #632
🤓 Reviewer Checklist
Reviewers: Please verify the following before approving this pull request.
Security & Quality
Testing & Validation
Documentation
Maintenance