Consistent error messaging for field names in query language

[al-niessner]

🗒️ Summary

Treat specific field names the same as wildcard field names.

🤖 AI Assistance Disclosure

• No AI assistance used
• AI used for light assistance (e.g., suggestions, refactoring, documentation help, minor edits)
• AI used for moderate content generation (AI generated some code or logic, but the developer authored or heavily revised the majority)
• AI generated substantial portions of this code

Estimated % of code influenced by AI: ___ %

⚙️ Test Data and/or Report

Automated tests should work as they cover this code block already.

♻️ Related Issues

#712

🤓 Reviewer Checklist

Reviewers: Please verify the following before approving this pull request.

Documentation and PR Content

• Documentation: README, Wiki, or inline documentation (Sphinx, Javadoc, Docstrings) have been updated to reflect these changes.
• Issue Traceability: The PR is linked to a valid GitHub Issue
• PR Title: The PR title is "user-friendly" clearly identifying what is being fixed or the new feature being added, that if you saw it in the Release Notes for a tool, you would be able to get the gist of what was done.

Security & Quality

• SonarCloud: Confirmed no new High or Critical security findings.
• Secrets Detection: Verified that the Secrets Detection scan passed and no sensitive information (keys, tokens, PII) is exposed.
• Code Quality: Code follows organization style guidelines and best practices for the specific language (e.g., PEP 8, Google Java Style).

Testing & Validation

• Test Accuracy: Verified that test data is accurate, representative of real-world PDS4 scenarios, and sufficient for the logic being tested.
• Coverage: Automated tests cover new logic and edge cases.
• Local Verification: (If applicable) Successfully built and ran the changes in a local or staging environment.

Maintenance

• Backward Compatibility: Confirmed that these changes do not break existing downstream dependencies or API contracts (or that breaking changes are clearly documented).

[tloubrieu-jpl]

Created by mistake on this PR. I was targeting another PR.

[jordanpadams]

@al-niessner @tloubrieu-jpl do we have any idea why these branch actions are running in what appears to be an infinite loop?

[al-niessner]

@jordanpadams Nope.

[jordanpadams]

@al-niessner when you have a chance, can you please try to debug what is going on with the integration tests? We cannot merge any of these updates without the integration tests integrating successfully or at least understand why it is hanging.

[jordanpadams]

Review

🐛 Bug — Wrong collection checked in non-wildcard branch

This is the most critical issue. The condition checks this.fieldNames (the output list being assembled) instead of this.knownFieldNames (the valid fields fetched from OpenSearch):

// BUGGY — this.fieldNames starts empty, so this will ALWAYS be false
// and throw for every specific field name on first use
if (this.fieldNames.contains(fn)) {
    this.fieldNames.add(fn);
} else {
    throw new ParseCancellationException(...);
}

Should be:

if (this.knownFieldNames.contains(fn)) {
    this.fieldNames.add(fn);
} else {
    throw new ParseCancellationException(...);
}

As written, every specific (non-wildcard) field name query will throw a ParseCancellationException on the first call since fieldNames is always empty at that point.

🐛 Typo in error message

throw new ParseCancellationException("The request '" + fieldname + "' does match any field names in the LDD.");

"does match" should be "does not match" — the message currently says the opposite of what it means.

⚠️ Behavior change: IllegalStateException vs. silent failure

Previously, if OpenSearch was unreachable during wildcard expansion, the error was logged and execution continued (wildcarding silently wouldn't work). Now it throws IllegalStateException for all queries. This is a potentially breaking change worth calling out explicitly if intentional.

⚠️ Inconsistency with issue #712 acceptance criteria (now resolved)

The acceptance criteria in #712 had a conflict on what to return when a field does not exist in the schema — "0 results" in one place, "400 error" in another. I've updated #712 to consistently require a 400 with a clear, descriptive error message for non-existent fields, which aligns with this PR's intent. The two bugs above just need to be fixed for this implementation to match.

[tloubrieu-jpl]

Hi @al-niessner ,

I am catching up with the registry updates.
While testing this PR with the integration test I had a 400 error with that request (from the integration tests).

curl --location 'http://localhost:8081/products?q=(pds%3APrimary_Result_Summary.pds%3Aprocessing_level%20eq%20%22Derived%22)&limit=10'

It sounds like a side effect from the update you have made but this field exists in the PDS4 information model:
https://pds.nasa.gov/datastandards/documents/dd/v1/PDS4_PDS_DD_1P00.html#d5e69406

Also I was hoping we could have a more explicit error message as specified in the acceptance criteria of the ticket: the API returns a 400 error with a clear, descriptive message identifying the unknown field name (e.g., "Field 'pds:Nonexistent_Field' does not exist in the registry schema")

Is that something which can be done ?

Thanks

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[al-niessner]

@jordanpadams @tloubrieu-jpl

Give me a thumbs up and I will merge it. Once merged, make a PR and do all of the bumps.

Note: need to merge NASA-PDS/registry#505 at the same time.

completed

[tloubrieu-jpl]

I am unable to test locally quickly as I am working on something else in the same repository, but what I see looks good.