Remove corresponding issues on branch delete #1224
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 9.x #1224 +/- ##
============================================
+ Coverage 47.96% 48.00% +0.04%
- Complexity 13831 13852 +21
============================================
Files 1942 1943 +1
Lines 94946 94989 +43
Branches 10955 10957 +2
============================================
+ Hits 45541 45600 +59
+ Misses 46378 46360 -18
- Partials 3027 3029 +2 ☔ View full report in Codecov by Sentry. |
| removeStaleIssues(context, List.of(resourceURI), previousAnalysisRunIds); | ||
| } | ||
|
|
||
| public void removeStaleIssues(ServiceProvider context, List<String> resourceURIs, List<String> previousAnalysisRunIds) { |
There was a problem hiding this comment.
| public void removeStaleIssues(ServiceProvider context, List<String> resourceURIs, List<String> previousAnalysisRunIds) { | |
| public void removeStaleIssues(ServiceProvider context, List<String> resourceURIs, List<String> resultIds) { |
| removeStaleIssues(context, resourceURIs, Collections.emptyList()); | ||
| } | ||
|
|
||
| public void removeStaleIssues(ServiceProvider context, String resourceURI, List<String> previousAnalysisRunIds) { |
There was a problem hiding this comment.
| public void removeStaleIssues(ServiceProvider context, String resourceURI, List<String> previousAnalysisRunIds) { | |
| public void removeStaleIssues(ServiceProvider context, String resourceURI, List<String> resultIds) { |
| public void removeStaleIssues(ServiceProvider context, List<String> resourceURIs, List<String> previousAnalysisRunIds) { | ||
| AsyncRequest<Boolean> request; | ||
|
|
||
| if (!previousAnalysisRunIds.isEmpty()) { |
There was a problem hiding this comment.
| if (!previousAnalysisRunIds.isEmpty()) { | |
| if (!CompareUtils.isEmpty(resultIds)) { |
| } else { | ||
| request = ValidationRequests.issues() | ||
| .prepareDelete() | ||
| .setCodeSystemURIs(resourceURIs) |
There was a problem hiding this comment.
What happens if the resourceURIs is null or empty?
There was a problem hiding this comment.
Nothing good, we shouldn't delete without resource uri filters.
There was a problem hiding this comment.
Yeah, but that's a flaw in the API. If the caller should only use one input parameter, then we should split this method into two separate implementations instead of merging them into one with if else blocks.
| .buildAsync(); | ||
| } | ||
|
|
||
| final String description = String.format("Remove validation issues on stale/removed branch(es) %s", Joiner.on(", ").join(resourceURIs)); |
There was a problem hiding this comment.
| final String description = String.format("Remove validation issues on stale/removed branch(es) %s", Joiner.on(", ").join(resourceURIs)); | |
| final String description = String.format("Remove validation issues on stale/removed branch(es) of '%s'", Joiner.on(", ").join(resourceURIs)); |
| public ValidationCleanupService() { | ||
| } | ||
|
|
||
| public void removeStaleIssues(ServiceProvider context, String resourceURI) { |
There was a problem hiding this comment.
Rename all these methods to scheduleStaleIssueRemoval since it is not actually removing anything from the index but schedules a job that removes it.
| JobRequests.prepareSchedule() | ||
| .setRequest(request) | ||
| .setDescription(description) | ||
| .setUser(context.service(User.class).getUserId()) |
There was a problem hiding this comment.
This is the default behavior in ScheduleJobRequest, you can leave this property unset if you don't want to override the requester:
AAAlinaaa
force-pushed
the
issue/SO-5533-clear-stale-validation-issues
branch
from
dc8f309
to
8500f18
Compare
| try { | ||
| //Schedule a job to remove issues corresponding to this branch | ||
| TerminologyResource resource = context.service(PathTerminologyResourceResolver.class).resolve(context, context.info().id(), getBranchPath()); | ||
| String resourceURI = resource.getResourceURI(getBranchPath()).toString(); |
There was a problem hiding this comment.
It looks like we have to treat the new tilde path variants (XZX~branch) as well here. Could you please add support for that?
...ional.snowowl.core/src/com/b2international/snowowl/core/branch/ValidationCleanupService.java
Show resolved
Hide resolved
| } else { | ||
| request = ValidationRequests.issues() | ||
| .prepareDelete() | ||
| .setCodeSystemURIs(resourceURIs) |
There was a problem hiding this comment.
Yeah, but that's a flaw in the API. If the caller should only use one input parameter, then we should split this method into two separate implementations instead of merging them into one with if else blocks.
No description provided.