-
Notifications
You must be signed in to change notification settings - Fork 33
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
SO-5872 Resource has upgrade #1199
base: 9.x
Are you sure you want to change the base?
Conversation
// create a version for the resource | ||
return new BranchSnapshotContentRequest<>(Branch.MAIN_PATH, | ||
new ResourceRepositoryCommitRequestBuilder() | ||
.setBody(tx -> { | ||
dependentResourceIds.forEach(id -> { | ||
ResourceDocument oldDocument = tx.lookup(id, ResourceDocument.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be better to lookup these resources in bulk.
@@ -195,10 +197,35 @@ public Boolean execute(RepositoryContext context) { | |||
monitor.worked(1); | |||
// }); | |||
|
|||
//Find resources that depend on this resource | |||
List<String> dependentResourceIds = ResourceRequests.prepareSearch() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you only need the ID then please use field selection.
.map(Resource::getId) | ||
.collect(Collectors.toList()); | ||
|
||
if (resourceToVersion instanceof TerminologyResourceCollection) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect, when versioning a collection the system will version all child resources. This will be handled in a different issue, please remove this block.
//Find resources that depend on this resource | ||
List<String> dependentResourceIds = ResourceRequests.prepareSearch() | ||
.setLimit(10_000) | ||
.filterByDependency(String.format("(uri:%s OR uri:%s/* OR uri:%s\\?*) AND scope:domain) ", resource, resource, resource)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restricting to just the domain scope is an invalid assumption. This code should work regardless of the scope value of the dependency entries.
No description provided.