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

SO-5872 Resource has upgrade #1199

Open
wants to merge 6 commits into
base: 9.x
Choose a base branch
from

Conversation

AAAlinaaa
Copy link
Contributor

No description provided.

@AAAlinaaa AAAlinaaa self-assigned this Sep 1, 2023
// 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);
Copy link
Member

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()
Copy link
Member

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) {
Copy link
Member

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))
Copy link
Member

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.

@cmark cmark added the feature label Sep 5, 2023
@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Attention: 222 lines in your changes are missing coverage. Please review.

Comparison is base (5e9cfc5) 64.43% compared to head (d1335f7) 64.06%.
Report is 357 commits behind head on 9.x.

Files Patch % Lines
...tion/TerminologyResourceCollectionRestService.java 24.56% 42 Missing and 1 partial ⚠️
...equest/resource/BaseMetadataResourceConverter.java 72.66% 31 Missing and 10 partials ⚠️
...resource/BaseTerminologyResourceCreateRequest.java 53.84% 23 Missing and 7 partials ⚠️
...c/com/b2international/snowowl/core/Dependency.java 4.00% 24 Missing ⚠️
...nternational/snowowl/core/TerminologyResource.java 58.33% 7 Missing and 3 partials ⚠️
...2international/commons/options/OptionsBuilder.java 50.00% 7 Missing and 1 partial ⚠️
...tional/snowowl/core/internal/ResourceDocument.java 65.21% 7 Missing and 1 partial ⚠️
...re/request/resource/BaseResourceSearchRequest.java 88.88% 3 Missing and 3 partials ⚠️
...ternational/snowowl/core/ResourceURIWithQuery.java 16.66% 5 Missing ⚠️
...lection/TerminologyResourceCollectionRequests.java 28.57% 5 Missing ⚠️
... and 23 more
Additional details and impacted files
@@             Coverage Diff              @@
##                9.x    #1199      +/-   ##
============================================
- Coverage     64.43%   64.06%   -0.37%     
- Complexity    13069    13142      +73     
============================================
  Files          1775     1799      +24     
  Lines         60755    61580     +825     
  Branches       5652     5694      +42     
============================================
+ Hits          39145    39450     +305     
- Misses        19199    19688     +489     
- Partials       2411     2442      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cmark cmark added the on hold label Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants