Skip to content

Conversation

@Pfeil
Copy link
Member

@Pfeil Pfeil commented Jan 2, 2025

Motivated by #243, as it tests precisely this case. This PR was not able to reproduce the issue!

This implements dryrun for the update API. This means:

  • It will require a valid record, including matching PIDs in URL and record (although the record PID can be omitted, as before).
  • It will require a valid etag of the previous version
  • The PID needs to exist
  • The new etag will be returned
  • Nothing really changes
    • The PID record will not be modified
    • No AMQP message will be sent
    • No elastic index will be updated, etc

How does it differ from the create-dryrun? Mainly in that it has the properties of an update:

  • The PID needs to exist in beforehand (opposite to create)
  • Etag is required
  • New etag will be returned

Why do we need this?

  • Consistency: It seems this is kind of expected by users.
  • Testing: It allows to at least partially test the handle protocol with create/update, although it does currently not really increase the test coverage. This may change over time as we plan to package all functionality which does not need authentication into other classes, which then may be executed in tests.

Other noteworthy changes:

  • improved logging on update if PIDs do not match, to have a more comprehensible source of information in such error reports
  • added an update test using dryrun using the read-only handle protocol tests

Breaking changes:

  • none expected

@Pfeil Pfeil changed the title Update dryrun Implement dryrun parameter for PID updates Jan 2, 2025
@coveralls
Copy link

coveralls commented Jan 2, 2025

Pull Request Test Coverage Report for Build #429

Details

  • 129 of 138 (93.48%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.07%) to 72.4%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/main/java/edu/kit/datamanager/pit/web/impl/TypingRESTResourceImpl.java 129 138 93.48%
Totals Coverage Status
Change from base Build #425: 0.07%
Covered Lines: 884
Relevant Lines: 1221

💛 - Coveralls

@Pfeil Pfeil self-assigned this Jan 2, 2025
@Pfeil Pfeil added the enhancement New feature or request label Jan 2, 2025
Pfeil added 4 commits January 2, 2025 16:02
Validation is a comparatively long task, which should not be done in-between. Etag tests now disable validation, making the assumption that validation is not the issue.
@Pfeil Pfeil merged commit 2a05903 into master Jan 2, 2025
6 checks passed
@Pfeil Pfeil deleted the update-dryrun branch January 2, 2025 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants