Skip to content

Port registry-common#296 and #303: warn on unresolvable LDD association and fix AOSS delete-by-query pagination - #123

Merged
jordanpadams merged 5 commits into
mainfrom
issues/registry-common-295-296-303-ports
Jul 28, 2026
Merged

Port registry-common#296 and #303: warn on unresolvable LDD association and fix AOSS delete-by-query pagination#123
jordanpadams merged 5 commits into
mainfrom
issues/registry-common-295-296-303-ports

Conversation

@jordanpadams

@jordanpadams jordanpadams commented Jul 23, 2026

Copy link
Copy Markdown
Member

🗒️ Summary

Ports three fixes from registry-common into the monorepo's common module:

registry-common#296 → ClassAttrAssociationParser:
Replaces the throw with a warn-and-skip when an association has isAttribute=true but no attribute ID can be resolved (neither "attributeId" nor "identifier" key present). Previously the parser threw LddException, halting the entire LDD load. Now it logs a warning and continues, matching the registry-common behaviour and allowing JsonLddLoader's zero-field sentinel guard (#295, already present here) to work correctly on the next run. Also corrects the comment: the association key format is determined by LDD tooling version, not IM version.

registry-common#303 → RestClientWrapper._performDBQRequest():
Loops until the search page comes back empty rather than doing a single pass. AOSS eventual consistency means deleted docs remain visible to subsequent searches, so a single-pass search-then-delete silently leaves documents behind on every call. Adds INFO-level progress logging (one line per batch) and DEBUG-level per-doc logging.

registry-common#295 (JsonLddLoader sentinel guard): Already present in this repo (merged via issues/81-aoss-ldd-propagation-race). No changes needed.

🤖 AI Assistance Disclosure

  • No AI assistance used
  • 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: 70%

⚙️ Test Data and/or Report

  • ClassAttrAssociationParser change: covered by existing TestLddFieldResolution parameterised tests (MRO 1M00_1400 and CART 1Q00_1970 fixtures). The warn path is exercised when an association has isAttribute=true but no resolvable ID.
  • RestClientWrapper change: validate by running registry-manager delete-dd -ns <namespace> against an AOSS node with multiple pages of docs; confirm all are deleted (count reaches 0) and no change in behaviour against standard OpenSearch.

♻️ Related Issues

Refs #88 (LDD fields silently not loaded — ClassAttrAssociationParser fix)
Refs NASA-PDS/registry-common#296
Refs NASA-PDS/registry-common#303

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

…ociation, fix AOSS DBQ pagination

- ClassAttrAssociationParser: replace throw with warn+skip when an association
  has isAttribute=true but no resolvable attribute ID (neither 'attributeId' nor
  'identifier' key found). Adds Logger; removes now-unused LddException import.
  Ports NASA-PDS/registry-common#296 (refs #88).

- RestClientWrapper._performDBQRequest(): loop until the search page is empty
  rather than doing a single pass. AOSS eventual consistency means deleted docs
  remain visible to subsequent searches, so a single-pass search-then-delete
  silently leaves documents behind. Adds INFO-level progress logging per batch;
  per-doc detail at DEBUG. Ports NASA-PDS/registry-common#303.

Note: registry-common#295 (JsonLddLoader sentinel guard) was already applied to
this repo in the issues/81-aoss-ldd-propagation-race merge.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jordanpadams
jordanpadams requested a review from a team as a code owner July 23, 2026 01:24
@jordanpadams jordanpadams added the bug Something isn't working label Jul 23, 2026
@jordanpadams jordanpadams self-assigned this Jul 23, 2026
jordanpadams and others added 4 commits July 22, 2026 18:32
…over newer version

- DataLoader: add ignoreConflicts flag (default false). When true, 409 Conflict
  on "create" is logged at DEBUG and treated as success rather than counted as a
  failure. Also improves the error message to name the index and count of truly
  failed documents.

- JsonLddLoader: enable ignoreConflicts=true on the -dd loader so that loading
  an older LDD version over an already-loaded newer version no longer produces a
  misleading "Failed to upload all documents" ERROR. Adds INFO log when the
  requested LDD is older than what is already in the registry.

- SchemaUpdater: demote log.error to log.warn in the updateSchema catch block
  (harvest continues — not a product failure). In updateLdd catch blocks, move
  ERROR log inside the lddInfo.isEmpty() guard so it only fires when a product
  will actually fail; the has-fallback branch uses WARN. Convert format strings
  to parameterised log4j2 style throughout.

Ports NASA-PDS/registry-common#304 (fixes NASA-PDS/harvest#342).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…dVersions sentinel, add tests

- LddVersions: promote DEFAULT_DATE to public; add public DEFAULT_LAST_DATE constant
  (Instant) so callers don't repeat the sentinel string literal.

- JsonLddLoader: replace hardcoded "1965-01-01..." with LddVersions.DEFAULT_LAST_DATE;
  add parse-check guard so the "not newer" log is only emitted when the LDD date is
  actually parseable (avoids misleading output on bad date strings); demote from INFO
  to DEBUG.

- DataLoader: correct the comment on the uploaded!=numRecords branch — clarifies
  that when ignoreConflicts=false, 409s ARE counted as failures (a shortfall means
  real write failures). Pass ex as trailing arg to log.warn in SchemaUpdater so the
  full stack trace is recorded.

- SchemaUpdater: pass exception object as trailing log arg so stack trace is recorded.

- TestDataLoaderIgnoreConflicts: new unit test covering LddVersions sentinel constant
  consistency, 409 counted/not-counted per ignoreConflicts flag, non-409 errors always
  counted, and successful items producing zero errors.

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

New test class covering the harvest#342 scenario end-to-end without a live cluster:

- lddDateToIsoInstant: confirms no-timezone dates (e.g. "2022-09-19T07:35:36")
  parse correctly and compare as older than timezone-qualified dates.
- createEsDataFile with older LDD: asserts "create" action lines in NDJSON —
  these yield 409s on pre-existing docs, accepted silently by ignoreConflicts=true.
- createEsDataFile with newer LDD: asserts "index" action lines so existing
  field definitions are updated.
- createEsDataFile with no prior LDD (epoch sentinel): asserts "index" lines
  for a first-ever load.
- No-timezone date in LDD treated as older than newer registry date: the exact
  regression from harvest#342.

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

Copy link
Copy Markdown

@jordanpadams
jordanpadams merged commit 3f52dd2 into main Jul 28, 2026
2 of 3 checks passed
@jordanpadams
jordanpadams deleted the issues/registry-common-295-296-303-ports branch July 28, 2026 19:35
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

Development

Successfully merging this pull request may close these issues.

2 participants