Skip to content

Fix validate -u HTTP 400 error by removing deprecated pds:Resource registry query filters - #1658

Open
jordanpadams wants to merge 5 commits into
mainfrom
bugfix/1657-registry-query-fix
Open

Fix validate -u HTTP 400 error by removing deprecated pds:Resource registry query filters#1658
jordanpadams wants to merge 5 commits into
mainfrom
bugfix/1657-registry-query-fix

Conversation

@jordanpadams

@jordanpadams jordanpadams commented Aug 19, 2026

Copy link
Copy Markdown
Member

🗒️ Summary

Remove pds:Resource.pds:type filter clauses from the registry search query used by validate -u — these resource fields no longer exist in the registry and cause HTTP 400 errors. Also updates the pagination sort key from ops:Harvest_Info.ops:harvest_date_time to ops:Label_File_Info.ops:creation_date_time.

Additionally fixes a pre-existing NullPointerException in the finally block of getLatestJsonContext() when the HTTP connection fails before the Scanner is assigned.

Changed files:

  • src/main/resources/validate.properties — simplified query to (product_class eq "Product_Context")
  • src/main/java/gov/nasa/pds/validate/ValidateLauncher.java — updated searchAfterKey; fix NPE in finally block; make getLatestJsonContext() package-private for testability
  • src/main/java/gov/nasa/pds/validate/util/ToolInfo.java — add setProperty() to support overriding search URL in tests
  • src/test/java/gov/nasa/pds/validate/GetLatestJsonContextTest.java — new: 3 tests using a JDK HttpServer mock (happy path, pagination, HTTP 400 error)

🤖 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: 90%

⚙️ Test Data and/or Report

Automated tests added in GetLatestJsonContextTest — 3 scenarios using a local JDK HttpServer mock (no network required):

  1. Happy path — single-page response with 2 products; asserts output JSON written with correct Product_Context array
  2. Pagination — total=3 served over 2 pages; asserts exactly 2 HTTP requests made
  3. HTTP 400 error — asserts method does not throw and output file is not written

All 3 tests pass locally (mvn test -Dtest=GetLatestJsonContextTest).

♻️ Related Issues

Fixes #1657
Fixes #1659

🤓 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).

🤖 Generated with Claude Code

…ting sort key

Remove pds:Resource.pds:type filter clauses from the registry query that no
longer exist in the registry and cause HTTP 400 errors. Also update the sort
key from ops:Harvest_Info.ops:harvest_date_time to
ops:Label_File_Info.ops:creation_date_time.

Fixes #1657

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add GetLatestJsonContextTest with 3 tests (happy path, pagination,
  HTTP 400 error) using a JDK HttpServer mock — no network required
- Fix pre-existing NullPointerException in getLatestJsonContext() finally
  block when reader is null (HTTP connection fails before Scanner is created)
- Add ToolInfo.setProperty() to allow overriding search URL in tests
- Make getLatestJsonContext() package-private to enable direct testing

Fixes #1659
Refs #1657

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@nutjob4life nutjob4life left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic @jordanpadams, thank you so much.

With the help of an artificial intelligence, we've found a potential pagination issue:

The creation_date_time may not be unique in the registry and using search-after may skip records. For example, suppose the page size is 1000 and we see records like:

…
record  999   creation_date_time = 2024-01-01T12:00:00Z
record 1000   creation_date_time = 2024-01-01T12:00:00Z
record 1001   creation_date_time = 2024-01-01T12:00:00Z
record 1002   creation_date_time = 2024-01-01T12:00:00Z

If page 1 ends at record №1000 and the next request says

search-after=2024-01-01T12:00:00Z

what happens to record №1001 and №1002? It might be that the underlying API/OpenSearch has a deterministic tie-breaker; do we know?

Everything else looks copacetic though, and Maven's happy, which means: ✅

Maven details:

[INFO] Tests run: 326, Failures: 0, Errors: 0, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

@jordanpadams

Copy link
Copy Markdown
Member Author

Great catch, @nutjob4life — the concern is valid. ops:Label_File_Info.ops:creation_date_time is not guaranteed to be unique, so a page boundary landing on a run of records with identical timestamps could cause search-after to skip some of them.

The good news: lidvid is already present in each record's properties (we already extract it in parseJsonObjectWriteTofile), and LIDVID is inherently unique. We can add it as a secondary sort key to guarantee a stable, unambiguous cursor.

The PDS Search API accepts multiple sort fields as a comma-separated list, so the fix in getLatestJsonContext() would be:

final String searchAfterKey = "ops:Label_File_Info.ops:creation_date_time,lidvid";

and the search-after value becomes <timestamp>,<lidvid> — two values separated by a comma, consistent with how the API's multi-key search-after works.

I'll open a follow-up issue to track this so it doesn't get lost, and address it before the release cut.

🤖 Generated with Claude Code

…date_time

Add lidvid as secondary sort key so the search-after cursor is always unique,
preventing pages from skipping records that share the same creation timestamp.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

2 participants