Skip to content

Fix FDP v2 index API #637

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

Merged
merged 19 commits into from
Mar 10, 2025
Merged

Fix FDP v2 index API #637

merged 19 commits into from
Mar 10, 2025

Conversation

dennisvang
Copy link
Contributor

@dennisvang dennisvang commented Mar 3, 2025

Changes:

  • Replaced last occurrences of registrationTime, modificationTime, and lastRetrievalTime, by createdAt, updatedAt, and lastRetrievalAt, respectively, conforming to changes made in Migrate to PostgreSQL #519
  • Added IndexEntry.events to specify cascading and orphan removal
  • Added tests to reproduce issues mentioned below
  • Set IndexEntry.lastRetrievalAt in EventService.processMetadataRetrieval and in test fixture
  • Added a rudimentary validation of sort url query parameters

This fixes the following:

by updatedAt, createdAt, and lastRetrievalAt, respectively.
This handles the exception globally, via ExceptionControllerAdvice.
Alternatively, we could do this on the controller level, adding the method to the IndexEntryController.
@dennisvang dennisvang marked this pull request as ready for review March 3, 2025 21:55
@dennisvang dennisvang requested a review from MarekSuchanek March 3, 2025 22:02
@dennisvang
Copy link
Contributor Author

dennisvang commented Mar 4, 2025

The error Cannot invoke "java.time.Instant.isAfter(java.time.Instant)" because "lastRetrievalAt" is null still persists.
Also see #633 (comment)

sub-issue:

these were accidentally changed to Instant in previous commits, to match other DTOs, but that results in a Unix timestamp , whereas we want to exchange ISO 8601 dates
@dennisvang dennisvang marked this pull request as draft March 5, 2025 12:34
@dennisvang dennisvang removed the request for review from MarekSuchanek March 5, 2025 12:34
@dennisvang dennisvang changed the title handle invalid sort parameters Fix FDP v2 index API Mar 5, 2025
@dennisvang
Copy link
Contributor Author

dennisvang commented Mar 5, 2025

Now we can click FDP links and delete FDPs, but we get the following after "Trigger ping":

...
2025-03-05 17:20:35,295 271938 [fdp-task-3] WARN  org.hibernate.engine.jdbc.spi.SqlExceptionHelper - SQL Error: 0, SQLState: 23503
2025-03-05 17:20:35,296 271939 [fdp-task-3] ERROR org.hibernate.engine.jdbc.spi.SqlExceptionHelper - ERROR: insert or update on table "index_event" violates foreign key constraint "fk__index_event_triggered_by"
  Detail: Key (triggered_by)=(50884866-6eb4-4562-b500-a31d83226a32) is not present in table "index_event".
2025-03-05 17:20:35,298 271941 [fdp-task-3] ERROR org.fairdatapoint.service.index.event.EventService - Failed to retrieve metadata: could not execute statement [ERROR: insert or update on table "index_event" violates foreign key constraint "fk__index_event_triggered_by"
  Detail: Key (triggered_by)=(50884866-6eb4-4562-b500-a31d83226a32) is not present in table "index_event".] [insert into index_event (created_at,executed_at,finished_at,payload,related_to,remote_addr,triggered_by,type,updated_at,version,uuid) values (?,?,?,?,?,?,?,?,?,?,?)]; SQL [insert into index_event (created_at,executed_at,finished_at,payload,related_to,remote_addr,triggered_by,type,updated_at,version,uuid) values (?,?,?,?,?,?,?,?,?,?,?)]; constraint [fk__index_event_triggered_by]
2025-03-05 17:20:35,298 271941 [fdp-task-3] INFO  org.fairdatapoint.service.index.event.EventService - Finished metadata retrieval triggered by 50884866-6eb4-4562-b500-a31d83226a32

UPDATE: forgot to initialize the IndexEntry.events set.

based on an explicit list of allowed fields IndexEntryController.sortFields

also remove the handleUnknownField controller advice, which was too broadly scoped
(precommit hook not working...)
@dennisvang dennisvang requested a review from MarekSuchanek March 7, 2025 13:36
@dennisvang dennisvang marked this pull request as ready for review March 7, 2025 13:38
Copy link
Contributor

@MarekSuchanek MarekSuchanek left a comment

Choose a reason for hiding this comment

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

Looks great 👍🏻

@dennisvang dennisvang merged commit 4893d66 into develop Mar 10, 2025
13 checks passed
@dennisvang dennisvang deleted the fix/issue633 branch March 10, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants