Skip to content
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

Deployment tests data cleanup enhancements #119

Merged
merged 8 commits into from
Jul 18, 2024

Conversation

rpoet-jh
Copy link
Contributor

@rpoet-jh rpoet-jh commented Jul 15, 2024

The following changes have been made to the deployment tests data cleanup feature:

  • Added a new application property named pass.test.skip.deposits that is true by default. If this property is true, any deposit for a submission associated with the deployment test grant will not be deposited into the downstream repository.
    • There is a new DevNullTransport that will be used in the skip deposits case that will do nothing with the package contents but will create the RepositoryCopy and set the Deposit.depositStatus to ACCEPTED.
  • If the pass.test.skip.deposits is false, then the deployment tests deposits will be made in the downstream repositories.
    • In this case, the new service class DspaceDepositService is used to delete the DSpace deposits. The deposit will be deleted from DSpace in about an hour after it was created.
    • The DspaceDepositService code is invoked as part of the DeploymentTestDataJob which run periodically to clean up deployment test data.

@rpoet-jh rpoet-jh requested a review from tsande16 July 15, 2024 20:31
@rpoet-jh rpoet-jh self-assigned this Jul 15, 2024
Copy link
Contributor

@tsande16 tsande16 left a comment

Choose a reason for hiding this comment

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

Looks good! Only two little comments about null checks and possibly handling them and logging them.

ResponseEntity<Void> csrfResponse = restClient.get()
.uri("/security/csrf")
.exchange((request, response) -> new ResponseEntity<>(null, response.getHeaders(), HttpStatus.OK));
String xsrfToken = csrfResponse.getHeaders().get("DSPACE-XSRF-TOKEN").get(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

May want to null check on this in case DSPACE-XSRF-TOKEN is null or empty list. Possibly log if it's null. This would be helpful with debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

.body(bodyPair)
.retrieve()
.toBodilessEntity();
String authToken = authResponse.getHeaders().get("Authorization").get(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

May want a null check here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@rpoet-jh rpoet-jh merged commit 37f7c3e into main Jul 18, 2024
2 checks passed
@rpoet-jh rpoet-jh deleted the russ-992-depl-tests-no-deposit-opt branch August 1, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleanup repository deposits made by pass-acceptance-testing
2 participants