From d961c0a405a2d16785e5769dcb5b879c14a997e8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 6 Sep 2019 16:26:54 +0200 Subject: [PATCH] SONAR-13093 Optimize cache of issues in Compute Engine --- .../build.gradle | 3 + ...ProjectAnalysisTaskContainerPopulator.java | 4 +- ...CloseIssuesOnRemovedComponentsVisitor.java | 10 +- .../issue/IntegrateIssuesVisitor.java | 24 +- .../projectanalysis/issue/IssueLifecycle.java | 3 +- .../{IssueCache.java => ProtoIssueCache.java} | 9 +- .../step/PersistIssuesStep.java | 19 +- .../step/SendIssueNotificationsStep.java | 12 +- .../projectanalysis/util/cache/DiskCache.java | 84 +----- .../cache/JavaSerializationDiskCache.java | 116 +++++++ .../util/cache/ProtobufIssueDiskCache.java | 283 ++++++++++++++++++ .../src/main/protobuf/issue_cache.proto | 98 ++++++ ...eIssuesOnRemovedComponentsVisitorTest.java | 20 +- .../issue/IntegrateIssuesVisitorTest.java | 80 ++--- .../step/PersistIssuesStepTest.java | 63 ++-- .../step/SendIssueNotificationsStepTest.java | 85 +++--- ...va => JavaSerializationDiskCacheTest.java} | 13 +- server/sonar-db-dao/build.gradle | 5 + .../org/sonar/core/issue/DefaultIssue.java | 12 +- .../java/org/sonar/core/issue/FieldDiffs.java | 3 +- 20 files changed, 712 insertions(+), 234 deletions(-) rename server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/{IssueCache.java => ProtoIssueCache.java} (82%) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/JavaSerializationDiskCache.java create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java create mode 100644 server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto rename server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/{DiskCacheTest.java => JavaSerializationDiskCacheTest.java} (87%) diff --git a/server/sonar-ce-task-projectanalysis/build.gradle b/server/sonar-ce-task-projectanalysis/build.gradle index 5cfdd62a4d6..fdef5bf68b5 100644 --- a/server/sonar-ce-task-projectanalysis/build.gradle +++ b/server/sonar-ce-task-projectanalysis/build.gradle @@ -43,6 +43,8 @@ dependencies { compileOnly 'com.google.code.findbugs:jsr305' + compile project(':server:sonar-db-dao') + testCompile 'com.google.code.findbugs:jsr305' testCompile 'com.tngtech.java:junit-dataprovider' testCompile 'org.apache.logging.log4j:log4j-api' @@ -59,4 +61,5 @@ dependencies { testFixturesApi testFixtures(project(':server:sonar-ce-task')) testFixturesCompileOnly 'com.google.code.findbugs:jsr305' + } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java index ed5b4fd315c..f38405ef195 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java @@ -62,7 +62,7 @@ import org.sonar.ce.task.projectanalysis.issue.DefaultAssignee; import org.sonar.ce.task.projectanalysis.issue.EffortAggregator; import org.sonar.ce.task.projectanalysis.issue.IntegrateIssuesVisitor; import org.sonar.ce.task.projectanalysis.issue.IssueAssigner; -import org.sonar.ce.task.projectanalysis.issue.IssueCache; +import org.sonar.ce.task.projectanalysis.issue.ProtoIssueCache; import org.sonar.ce.task.projectanalysis.issue.IssueCounter; import org.sonar.ce.task.projectanalysis.issue.IssueCreationDateCalculator; import org.sonar.ce.task.projectanalysis.issue.IssueLifecycle; @@ -231,7 +231,7 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop RuleRepositoryImpl.class, ScmAccountToUserLoader.class, ScmAccountToUser.class, - IssueCache.class, + ProtoIssueCache.class, DefaultAssignee.class, IssueVisitors.class, IssueLifecycle.class, diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitor.java index e941137ac67..2e4ec262ca8 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitor.java @@ -24,7 +24,7 @@ import java.util.Set; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit; import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter; -import org.sonar.ce.task.projectanalysis.util.cache.DiskCache; +import org.sonar.ce.task.projectanalysis.util.cache.DiskCache.CacheAppender; import org.sonar.core.issue.DefaultIssue; import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER; @@ -36,15 +36,15 @@ public class CloseIssuesOnRemovedComponentsVisitor extends TypeAwareVisitorAdapt private final ComponentIssuesLoader issuesLoader; private final ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues; - private final IssueCache issueCache; + private final ProtoIssueCache protoIssueCache; private final IssueLifecycle issueLifecycle; - public CloseIssuesOnRemovedComponentsVisitor(ComponentIssuesLoader issuesLoader, ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues, IssueCache issueCache, + public CloseIssuesOnRemovedComponentsVisitor(ComponentIssuesLoader issuesLoader, ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues, ProtoIssueCache protoIssueCache, IssueLifecycle issueLifecycle) { super(CrawlerDepthLimit.PROJECT, POST_ORDER); this.issuesLoader = issuesLoader; this.componentsWithUnprocessedIssues = componentsWithUnprocessedIssues; - this.issueCache = issueCache; + this.protoIssueCache = protoIssueCache; this.issueLifecycle = issueLifecycle; } @@ -54,7 +54,7 @@ public class CloseIssuesOnRemovedComponentsVisitor extends TypeAwareVisitorAdapt } private void closeIssuesForDeletedComponentUuids(Set deletedComponentUuids) { - try (DiskCache.DiskAppender cacheAppender = issueCache.newAppender()) { + try (CacheAppender cacheAppender = protoIssueCache.newAppender()) { for (String deletedComponentUuid : deletedComponentUuids) { List issues = issuesLoader.loadOpenIssues(deletedComponentUuid); for (DefaultIssue issue : issues) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java index d3c25226230..316f3477db7 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitor.java @@ -26,7 +26,7 @@ import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit; import org.sonar.ce.task.projectanalysis.component.ReferenceBranchComponentUuids; import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter; -import org.sonar.ce.task.projectanalysis.util.cache.DiskCache; +import org.sonar.ce.task.projectanalysis.util.cache.DiskCache.CacheAppender; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.util.stream.MoreCollectors; @@ -34,17 +34,17 @@ import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { - private final IssueCache issueCache; + private final ProtoIssueCache protoIssueCache; private final IssueLifecycle issueLifecycle; private final IssueVisitors issueVisitors; private final IssueTrackingDelegator issueTracking; private final SiblingsIssueMerger issueStatusCopier; private final ReferenceBranchComponentUuids referenceBranchComponentUuids; - public IntegrateIssuesVisitor(IssueCache issueCache, IssueLifecycle issueLifecycle, IssueVisitors issueVisitors, IssueTrackingDelegator issueTracking, + public IntegrateIssuesVisitor(ProtoIssueCache protoIssueCache, IssueLifecycle issueLifecycle, IssueVisitors issueVisitors, IssueTrackingDelegator issueTracking, SiblingsIssueMerger issueStatusCopier, ReferenceBranchComponentUuids referenceBranchComponentUuids) { super(CrawlerDepthLimit.FILE, POST_ORDER); - this.issueCache = issueCache; + this.protoIssueCache = protoIssueCache; this.issueLifecycle = issueLifecycle; this.issueVisitors = issueVisitors; this.issueTracking = issueTracking; @@ -54,7 +54,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { @Override public void visitAny(Component component) { - try (DiskCache.DiskAppender cacheAppender = issueCache.newAppender()) { + try (CacheAppender cacheAppender = protoIssueCache.newAppender()) { issueVisitors.beforeComponent(component); TrackingResult tracking = issueTracking.track(component); fillNewOpenIssues(component, tracking.newIssues(), cacheAppender); @@ -67,7 +67,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { } } - private void fillNewOpenIssues(Component component, Stream newIssues, DiskCache.DiskAppender cacheAppender) { + private void fillNewOpenIssues(Component component, Stream newIssues, CacheAppender cacheAppender) { List newIssuesList = newIssues .peek(issueLifecycle::initNewOpenIssue) .collect(MoreCollectors.toList()); @@ -83,7 +83,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { } } - private void copyIssues(Component component, Map matched, DiskCache.DiskAppender cacheAppender) { + private void copyIssues(Component component, Map matched, CacheAppender cacheAppender) { for (Map.Entry entry : matched.entrySet()) { DefaultIssue raw = entry.getKey(); DefaultIssue base = entry.getValue(); @@ -92,7 +92,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { } } - private void fillExistingOpenIssues(Component component, Map matched, DiskCache.DiskAppender cacheAppender) { + private void fillExistingOpenIssues(Component component, Map matched, CacheAppender cacheAppender) { for (Map.Entry entry : matched.entrySet()) { DefaultIssue raw = entry.getKey(); DefaultIssue base = entry.getValue(); @@ -101,7 +101,7 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { } } - private void closeIssues(Component component, Stream issues, DiskCache.DiskAppender cacheAppender) { + private void closeIssues(Component component, Stream issues, CacheAppender cacheAppender) { issues.forEach(issue -> { // TODO should replace flag "beingClosed" by express call to transition "automaticClose" issue.setBeingClosed(true); @@ -110,10 +110,12 @@ public class IntegrateIssuesVisitor extends TypeAwareVisitorAdapter { }); } - private void process(Component component, DefaultIssue issue, DiskCache.DiskAppender cacheAppender) { + private void process(Component component, DefaultIssue issue, CacheAppender cacheAppender) { issueLifecycle.doAutomaticTransition(issue); issueVisitors.onIssue(component, issue); - cacheAppender.append(issue); + if (issue.isNew() || issue.isChanged() || issue.isCopied()) { + cacheAppender.append(issue); + } } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java index 72526f97d2e..7fe33850709 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java @@ -147,7 +147,8 @@ public class IssueLifecycle { result.setUserUuid(c.userUuid()); result.setCreationDate(c.creationDate()); // Don't copy "file" changelogs as they refer to file uuids that might later be purged - c.diffs().entrySet().stream().filter(e -> !e.getKey().equals(IssueFieldsSetter.FILE)) + c.diffs().entrySet().stream() + .filter(e -> !e.getKey().equals(IssueFieldsSetter.FILE)) .forEach(e -> result.setDiff(e.getKey(), e.getValue().oldValue(), e.getValue().newValue())); if (result.diffs().isEmpty()) { return Optional.empty(); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCache.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ProtoIssueCache.java similarity index 82% rename from server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCache.java rename to server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ProtoIssueCache.java index dafcb1561fd..bf1ee1ef29a 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCache.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ProtoIssueCache.java @@ -22,21 +22,20 @@ package org.sonar.ce.task.projectanalysis.issue; import java.io.File; import org.sonar.api.utils.System2; import org.sonar.api.utils.TempFolder; -import org.sonar.ce.task.projectanalysis.util.cache.DiskCache; -import org.sonar.core.issue.DefaultIssue; +import org.sonar.ce.task.projectanalysis.util.cache.ProtobufIssueDiskCache; /** * Cache of all the issues involved in the analysis. Their state is as it will be * persisted in database (after issue tracking, auto-assignment, ...) */ -public class IssueCache extends DiskCache { +public class ProtoIssueCache extends ProtobufIssueDiskCache { // this constructor is used by picocontainer - public IssueCache(TempFolder tempFolder, System2 system2) { + public ProtoIssueCache(TempFolder tempFolder, System2 system2) { super(tempFolder.newFile("issues", ".dat"), system2); } - public IssueCache(File file, System2 system2) { + public ProtoIssueCache(File file, System2 system2) { super(file, system2); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStep.java index d7a61ff8840..d4db74ea7da 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStep.java @@ -22,8 +22,9 @@ package org.sonar.ce.task.projectanalysis.step; import java.util.ArrayList; import java.util.List; import java.util.Map; +import org.apache.commons.io.FileUtils; import org.sonar.api.utils.System2; -import org.sonar.ce.task.projectanalysis.issue.IssueCache; +import org.sonar.ce.task.projectanalysis.issue.ProtoIssueCache; import org.sonar.ce.task.projectanalysis.issue.RuleRepository; import org.sonar.ce.task.projectanalysis.issue.UpdateConflictResolver; import org.sonar.ce.task.step.ComputationStep; @@ -49,24 +50,26 @@ public class PersistIssuesStep implements ComputationStep { private final System2 system2; private final UpdateConflictResolver conflictResolver; private final RuleRepository ruleRepository; - private final IssueCache issueCache; + private final ProtoIssueCache protoIssueCache; private final IssueStorage issueStorage; public PersistIssuesStep(DbClient dbClient, System2 system2, UpdateConflictResolver conflictResolver, - RuleRepository ruleRepository, IssueCache issueCache, IssueStorage issueStorage) { + RuleRepository ruleRepository, ProtoIssueCache protoIssueCache, IssueStorage issueStorage) { this.dbClient = dbClient; this.system2 = system2; this.conflictResolver = conflictResolver; this.ruleRepository = ruleRepository; - this.issueCache = issueCache; + this.protoIssueCache = protoIssueCache; this.issueStorage = issueStorage; } @Override public void execute(ComputationStep.Context context) { + context.getStatistics().add("cacheSize", FileUtils.byteCountToDisplaySize(protoIssueCache.fileSize())); IssueStatistics statistics = new IssueStatistics(); try (DbSession dbSession = dbClient.openSession(true); - CloseableIterator issues = issueCache.traverse()) { + + CloseableIterator issues = protoIssueCache.traverse()) { List addedIssues = new ArrayList<>(ISSUE_BATCHING_SIZE); List updatedIssues = new ArrayList<>(ISSUE_BATCHING_SIZE); @@ -86,8 +89,6 @@ public class PersistIssuesStep implements ComputationStep { persistUpdatedIssues(statistics, updatedIssues, mapper, changeMapper); updatedIssues.clear(); } - } else { - statistics.untouched++; } } persistNewIssues(statistics, addedIssues, mapper, changeMapper); @@ -156,14 +157,12 @@ public class PersistIssuesStep implements ComputationStep { private int inserts = 0; private int updates = 0; private int merged = 0; - private int untouched = 0; private void dumpTo(ComputationStep.Context context) { context.getStatistics() .add("inserts", String.valueOf(inserts)) .add("updates", String.valueOf(updates)) - .add("merged", String.valueOf(merged)) - .add("untouched", String.valueOf(untouched)); + .add("merged", String.valueOf(merged)); } } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java index 72fd08f6c2d..46d91525b7a 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java @@ -39,7 +39,7 @@ import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.analysis.Branch; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.TreeRootHolder; -import org.sonar.ce.task.projectanalysis.issue.IssueCache; +import org.sonar.ce.task.projectanalysis.issue.ProtoIssueCache; import org.sonar.ce.task.projectanalysis.notification.NotificationFactory; import org.sonar.ce.task.step.ComputationStep; import org.sonar.core.issue.DefaultIssue; @@ -72,17 +72,17 @@ public class SendIssueNotificationsStep implements ComputationStep { */ static final Set> NOTIF_TYPES = ImmutableSet.of(NewIssuesNotification.class, MyNewIssuesNotification.class, IssuesChangesNotification.class); - private final IssueCache issueCache; + private final ProtoIssueCache protoIssueCache; private final TreeRootHolder treeRootHolder; private final NotificationService service; private final AnalysisMetadataHolder analysisMetadataHolder; private final NotificationFactory notificationFactory; private final DbClient dbClient; - public SendIssueNotificationsStep(IssueCache issueCache, TreeRootHolder treeRootHolder, + public SendIssueNotificationsStep(ProtoIssueCache protoIssueCache, TreeRootHolder treeRootHolder, NotificationService service, AnalysisMetadataHolder analysisMetadataHolder, NotificationFactory notificationFactory, DbClient dbClient) { - this.issueCache = issueCache; + this.protoIssueCache = protoIssueCache; this.treeRootHolder = treeRootHolder; this.service = service; this.analysisMetadataHolder = analysisMetadataHolder; @@ -111,12 +111,12 @@ public class SendIssueNotificationsStep implements ComputationStep { NewIssuesStatistics newIssuesStats = new NewIssuesStatistics(onCurrentAnalysis); Map assigneesByUuid; try (DbSession dbSession = dbClient.openSession(false)) { - Iterable iterable = issueCache::traverse; + Iterable iterable = protoIssueCache::traverse; Set assigneeUuids = stream(iterable.spliterator(), false).map(DefaultIssue::assignee).filter(Objects::nonNull).collect(Collectors.toSet()); assigneesByUuid = dbClient.userDao().selectByUuids(dbSession, assigneeUuids).stream().collect(toMap(UserDto::getUuid, dto -> dto)); } - try (CloseableIterator issues = issueCache.traverse()) { + try (CloseableIterator issues = protoIssueCache.traverse()) { processIssues(newIssuesStats, issues, assigneesByUuid, notificationStatistics); } if (newIssuesStats.hasIssuesOnCurrentAnalysis()) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/DiskCache.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/DiskCache.java index 81fa28b2548..d278ce84637 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/DiskCache.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/DiskCache.java @@ -19,90 +19,20 @@ */ package org.sonar.ce.task.projectanalysis.util.cache; -import java.io.File; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.ObjectOutputStream; -import java.io.OutputStream; import java.io.Serializable; -import org.apache.commons.io.FileUtils; -import org.apache.commons.io.IOUtils; -import org.sonar.api.utils.System2; import org.sonar.core.util.CloseableIterator; -/** - * Serialize and deserialize objects on disk. No search capabilities, only traversal (full scan). - */ -public class DiskCache { - - private final File file; - private final System2 system2; - - public DiskCache(File file, System2 system2) { - this.system2 = system2; - this.file = file; - OutputStream output = null; - boolean threw = true; - try { - // writes the serialization stream header required when calling "traverse()" - // on empty stream. Moreover it allows to call multiple times "newAppender()" - output = new ObjectOutputStream(new FileOutputStream(file)); - output.flush(); - threw = false; - } catch (IOException e) { - throw new IllegalStateException("Fail to write into file: " + file, e); - } finally { - if (threw) { - // do not hide initial exception - IOUtils.closeQuietly(output); - } else { - // raise an exception if can't close - system2.close(output); - } - } - } - - public DiskAppender newAppender() { - return new DiskAppender(); - } - - public CloseableIterator traverse() { - try { - return new ObjectInputStreamIterator<>(FileUtils.openInputStream(file)); - } catch (IOException e) { - throw new IllegalStateException("Fail to traverse file: " + file, e); - } - } +public interface DiskCache { + long fileSize(); - public class DiskAppender implements AutoCloseable { - private final ObjectOutputStream output; + CacheAppender newAppender(); - private DiskAppender() { - try { - this.output = new ObjectOutputStream(new FileOutputStream(file, true)) { - @Override - protected void writeStreamHeader() { - // do not write stream headers as it's already done in constructor of DiskCache - } - }; - } catch (IOException e) { - throw new IllegalStateException("Fail to open file " + file, e); - } - } + CloseableIterator traverse(); - public DiskAppender append(O object) { - try { - output.writeObject(object); - output.reset(); - return this; - } catch (IOException e) { - throw new IllegalStateException("Fail to write into file " + file, e); - } - } + interface CacheAppender extends AutoCloseable { + CacheAppender append(I object); @Override - public void close() { - system2.close(output); - } + void close(); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/JavaSerializationDiskCache.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/JavaSerializationDiskCache.java new file mode 100644 index 00000000000..40e50783973 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/JavaSerializationDiskCache.java @@ -0,0 +1,116 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.task.projectanalysis.util.cache; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.ObjectOutputStream; +import java.io.OutputStream; +import java.io.Serializable; +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; +import org.sonar.api.utils.System2; +import org.sonar.core.util.CloseableIterator; + +/** + * Serialize and deserialize objects on disk. No search capabilities, only traversal (full scan). + */ +public class JavaSerializationDiskCache implements DiskCache { + + private final File file; + private final System2 system2; + + public JavaSerializationDiskCache(File file, System2 system2) { + this.system2 = system2; + this.file = file; + OutputStream output = null; + boolean threw = true; + try { + // writes the serialization stream header required when calling "traverse()" + // on empty stream. Moreover it allows to call multiple times "newAppender()" + output = new ObjectOutputStream(new FileOutputStream(file)); + output.flush(); + threw = false; + } catch (IOException e) { + throw new IllegalStateException("Fail to write into file: " + file, e); + } finally { + if (threw) { + // do not hide initial exception + IOUtils.closeQuietly(output); + } else { + // raise an exception if can't close + system2.close(output); + } + } + } + + @Override + public long fileSize() { + return file.length(); + } + + @Override + public CacheAppender newAppender() { + return new JavaSerializationCacheAppender(); + } + + @Override + public CloseableIterator traverse() { + try { + return new ObjectInputStreamIterator<>(FileUtils.openInputStream(file)); + } catch (IOException e) { + throw new IllegalStateException("Fail to traverse file: " + file, e); + } + } + + public class JavaSerializationCacheAppender implements CacheAppender { + private final ObjectOutputStream output; + + private JavaSerializationCacheAppender() { + try { + this.output = new ObjectOutputStream(new FileOutputStream(file, true)) { + @Override + protected void writeStreamHeader() { + // do not write stream headers as it's already done in constructor of DiskCache + } + }; + } catch (IOException e) { + throw new IllegalStateException("Fail to open file " + file, e); + } + } + + @Override + public CacheAppender append(O object) { + try { + output.writeObject(object); + output.reset(); + return this; + } catch (IOException e) { + throw new IllegalStateException("Fail to write into file " + file, e); + } + } + + @Override + public void close() { + system2.close(output); + } + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java new file mode 100644 index 00000000000..3dc527a31ad --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java @@ -0,0 +1,283 @@ +/* + * SonarQube + * Copyright (C) 2009-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.task.projectanalysis.util.cache; + +import com.google.common.base.Joiner; +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableSet; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.OutputStream; +import java.util.Collections; +import java.util.Date; +import java.util.Map; +import javax.annotation.CheckForNull; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.RuleType; +import org.sonar.api.utils.Duration; +import org.sonar.api.utils.System2; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.DefaultIssueComment; +import org.sonar.core.issue.FieldDiffs; +import org.sonar.core.util.CloseableIterator; +import org.sonar.core.util.Protobuf; +import org.sonar.db.protobuf.DbIssues; + +import static java.util.Optional.ofNullable; + +public class ProtobufIssueDiskCache implements DiskCache { + private static final String TAGS_SEPARATOR = ","; + private static final Splitter TAGS_SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings(); + + private final File file; + private final System2 system2; + + public ProtobufIssueDiskCache(File file, System2 system2) { + this.file = file; + this.system2 = system2; + } + + @Override + public long fileSize() { + return file.length(); + } + + @Override + public CacheAppender newAppender() { + try { + return new ProtoCacheAppender(); + } catch (FileNotFoundException e) { + throw new IllegalStateException(e); + } + } + + @Override + public CloseableIterator traverse() { + CloseableIterator protoIterator = Protobuf.readStream(file, IssueCache.Issue.parser()); + return new CloseableIterator() { + @CheckForNull + @Override + protected DefaultIssue doNext() { + if (protoIterator.hasNext()) { + return toDefaultIssue(protoIterator.next()); + } + return null; + } + + @Override + protected void doClose() { + protoIterator.close(); + } + }; + } + + private static DefaultIssue toDefaultIssue(IssueCache.Issue next) { + DefaultIssue defaultIssue = new DefaultIssue(); + defaultIssue.setKey(next.getKey()); + defaultIssue.setType(RuleType.valueOf(next.getRuleType())); + defaultIssue.setComponentUuid(next.hasComponentUuid() ? next.getComponentUuid() : null); + defaultIssue.setComponentKey(next.getComponentKey()); + defaultIssue.setModuleUuid(next.hasModuleUuid() ? next.getModuleUuid() : null); + defaultIssue.setModuleUuidPath(next.hasModuleUuidPath() ? next.getModuleUuidPath() : null); + defaultIssue.setProjectUuid(next.getProjectUuid()); + defaultIssue.setProjectKey(next.getProjectKey()); + defaultIssue.setRuleKey(RuleKey.parse(next.getRuleKey())); + defaultIssue.setLanguage(next.hasLanguage() ? next.getLanguage() : null); + defaultIssue.setSeverity(next.hasSeverity() ? next.getSeverity() : null); + defaultIssue.setManualSeverity(next.getManualSeverity()); + defaultIssue.setMessage(next.hasMessage() ? next.getMessage() : null); + defaultIssue.setLine(next.hasLine() ? next.getLine() : null); + defaultIssue.setGap(next.hasGap() ? next.getGap() : null); + defaultIssue.setEffort(next.hasEffort() ? Duration.create(next.getEffort()) : null); + defaultIssue.setStatus(next.getStatus()); + defaultIssue.setResolution(next.hasResolution() ? next.getResolution() : null); + defaultIssue.setAssigneeUuid(next.hasAssigneeUuid() ? next.getAssigneeUuid() : null); + defaultIssue.setChecksum(next.hasChecksum() ? next.getChecksum() : null); + defaultIssue.setAttributes(next.getAttributesMap()); + defaultIssue.setAuthorLogin(next.hasAuthorLogin() ? next.getAuthorLogin() : null); + next.getCommentsList().forEach(c -> defaultIssue.addComment(toDefaultIssueComment(c))); + defaultIssue.setTags(ImmutableSet.copyOf(TAGS_SPLITTER.split(next.hasTags() ? "" : next.getTags()))); + defaultIssue.setLocations(next.hasLocations() ? next.getLocations() : null); + defaultIssue.setIsFromExternalRuleEngine(next.getIsFromExternalRuleEngine()); + defaultIssue.setCreationDate(new Date(next.getCreationDate())); + defaultIssue.setUpdateDate(next.hasUpdateDate() ? new Date(next.getUpdateDate()) : null); + defaultIssue.setCloseDate(next.hasCloseDate() ? new Date(next.getCloseDate()) : null); + defaultIssue.setCurrentChangeWithoutAddChange(next.hasCurrentChanges() ? toDefaultIssueChanges(next.getCurrentChanges()) : null); + defaultIssue.setNew(next.getIsNew()); + defaultIssue.setCopied(next.getIsCopied()); + defaultIssue.setBeingClosed(next.getBeingClosed()); + defaultIssue.setOnDisabledRule(next.getOnDisabledRule()); + defaultIssue.setChanged(next.getIsChanged()); + defaultIssue.setSendNotifications(next.getSendNotifications()); + defaultIssue.setSelectedAt(next.hasSelectedAt() ? next.getSelectedAt() : null); + + for (IssueCache.FieldDiffs protoFieldDiffs : next.getChangesList()) { + defaultIssue.addChange(toDefaultIssueChanges(protoFieldDiffs)); + } + + return defaultIssue; + } + + private static IssueCache.Issue toProto(IssueCache.Issue.Builder builder, DefaultIssue defaultIssue) { + builder.clear(); + builder.setKey(defaultIssue.key()); + builder.setRuleType(defaultIssue.type().getDbConstant()); + ofNullable(defaultIssue.componentUuid()).ifPresent(builder::setComponentUuid); + builder.setComponentKey(defaultIssue.componentKey()); + ofNullable(defaultIssue.moduleUuid()).ifPresent(builder::setModuleUuid); + ofNullable(defaultIssue.moduleUuidPath()).ifPresent(builder::setModuleUuidPath); + builder.setProjectUuid(defaultIssue.projectUuid()); + builder.setProjectKey(defaultIssue.projectKey()); + builder.setRuleKey(defaultIssue.ruleKey().toString()); + ofNullable(defaultIssue.language()).ifPresent(builder::setLanguage); + ofNullable(defaultIssue.severity()).ifPresent(builder::setSeverity); + builder.setManualSeverity(defaultIssue.manualSeverity()); + ofNullable(defaultIssue.message()).ifPresent(builder::setMessage); + ofNullable(defaultIssue.line()).ifPresent(builder::setLine); + ofNullable(defaultIssue.gap()).ifPresent(builder::setGap); + ofNullable(defaultIssue.effort()).map(Duration::toMinutes).ifPresent(builder::setEffort); + builder.setStatus(defaultIssue.status()); + ofNullable(defaultIssue.resolution()).ifPresent(builder::setResolution); + ofNullable(defaultIssue.assignee()).ifPresent(builder::setAssigneeUuid); + ofNullable(defaultIssue.checksum()).ifPresent(builder::setChecksum); + ofNullable(defaultIssue.attributes()).ifPresent(builder::putAllAttributes); + ofNullable(defaultIssue.authorLogin()).ifPresent(builder::setAuthorLogin); + defaultIssue.defaultIssueComments().forEach(c -> builder.addComments(toProtoComment(c))); + ofNullable(defaultIssue.tags()).ifPresent(t -> builder.setTags(String.join(TAGS_SEPARATOR, t))); + ofNullable(defaultIssue.getLocations()).ifPresent(l -> builder.setLocations((DbIssues.Locations) l)); + builder.setIsFromExternalRuleEngine(defaultIssue.isFromExternalRuleEngine()); + builder.setCreationDate(defaultIssue.creationDate().getTime()); + ofNullable(defaultIssue.updateDate()).map(Date::getTime).ifPresent(builder::setUpdateDate); + ofNullable(defaultIssue.closeDate()).map(Date::getTime).ifPresent(builder::setCloseDate); + ofNullable(defaultIssue.currentChange()).ifPresent(c -> builder.setCurrentChanges(toProtoIssueChanges(c))); + builder.setIsNew(defaultIssue.isNew()); + builder.setIsCopied(defaultIssue.isCopied()); + builder.setBeingClosed(defaultIssue.isBeingClosed()); + builder.setOnDisabledRule(defaultIssue.isOnDisabledRule()); + builder.setIsChanged(defaultIssue.isChanged()); + builder.setSendNotifications(defaultIssue.mustSendNotifications()); + ofNullable(defaultIssue.selectedAt()).ifPresent(builder::setSelectedAt); + + for (FieldDiffs fieldDiffs : defaultIssue.changes()) { + builder.addChanges(toProtoIssueChanges(fieldDiffs)); + } + + return builder.build(); + } + + private static DefaultIssueComment toDefaultIssueComment(IssueCache.Comment comment) { + DefaultIssueComment issueComment = new DefaultIssueComment() + .setCreatedAt(new Date(comment.getCreatedAt())) + .setUpdatedAt(new Date(comment.getUpdatedAt())) + .setNew(comment.getIsNew()) + .setKey(comment.getKey()) + .setIssueKey(comment.getIssueKey()) + .setMarkdownText(comment.getMarkdownText()); + + if (comment.hasUserUuid()) { + issueComment.setUserUuid(comment.getUserUuid()); + } + return issueComment; + } + + private static IssueCache.Comment toProtoComment(DefaultIssueComment comment) { + IssueCache.Comment.Builder builder = IssueCache.Comment.newBuilder() + .setCreatedAt(comment.createdAt().getTime()) + .setUpdatedAt(comment.updatedAt().getTime()) + .setIsNew(comment.isNew()) + .setKey(comment.key()) + .setIssueKey(comment.issueKey()) + .setMarkdownText(comment.markdownText()); + + if (comment.userUuid() != null) { + builder.setUserUuid(comment.userUuid()); + } + return builder.build(); + } + + private static FieldDiffs toDefaultIssueChanges(IssueCache.FieldDiffs fieldDiffs) { + FieldDiffs defaultIssueFieldDiffs = new FieldDiffs() + .setUserUuid(fieldDiffs.getUserUuid()) + .setCreationDate(new Date(fieldDiffs.getCreationDate())); + + if (fieldDiffs.hasIssueKey()) { + defaultIssueFieldDiffs.setIssueKey(fieldDiffs.getIssueKey()); + } + + for (Map.Entry e : fieldDiffs.getDiffsMap().entrySet()) { + defaultIssueFieldDiffs.setDiff(e.getKey(), + e.getValue().hasOldValue() ? e.getValue().getOldValue() : null, + e.getValue().hasNewValue() ? e.getValue().getNewValue() : null); + } + + return defaultIssueFieldDiffs; + } + + private static IssueCache.FieldDiffs toProtoIssueChanges(FieldDiffs fieldDiffs) { + IssueCache.FieldDiffs.Builder builder = IssueCache.FieldDiffs.newBuilder() + .setCreationDate(fieldDiffs.creationDate().getTime()); + + if (fieldDiffs.issueKey() != null) { + builder.setIssueKey(fieldDiffs.issueKey()); + } + + String userUuid = fieldDiffs.userUuid(); + if (userUuid != null) { + builder.setUserUuid(userUuid); + } + + for (Map.Entry e : fieldDiffs.diffs().entrySet()) { + IssueCache.Diff.Builder diffBuilder = IssueCache.Diff.newBuilder(); + if (e.getValue().oldValue() != null) { + diffBuilder.setOldValue(e.getValue().oldValue().toString()); + } + if (e.getValue().newValue() != null) { + diffBuilder.setNewValue(e.getValue().newValue().toString()); + + } + builder.putDiffs(e.getKey(), diffBuilder.build()); + } + + return builder.build(); + } + + private class ProtoCacheAppender implements CacheAppender { + private final OutputStream out; + private final IssueCache.Issue.Builder builder; + + private ProtoCacheAppender() throws FileNotFoundException { + this.out = new BufferedOutputStream(new FileOutputStream(file, true)); + this.builder = IssueCache.Issue.newBuilder(); + } + + @Override + public CacheAppender append(DefaultIssue object) { + Protobuf.writeStream(Collections.singleton(toProto(builder, object)), out); + return this; + } + + @Override + public void close() { + system2.close(out); + } + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto b/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto new file mode 100644 index 00000000000..bb3352920a5 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto @@ -0,0 +1,98 @@ +// SonarQube, open source software quality management tool. +// Copyright (C) 2008-2016 SonarSource +// mailto:contact AT sonarsource DOT com +// +// SonarQube is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; either +// version 3 of the License, or (at your option) any later version. +// +// SonarQube is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with this program; if not, write to the Free Software Foundation, +// Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + + +// IMPORTANT +// This is beta version of specification. It will evolve during next +// releases and is not forward-compatible yet. + +syntax = "proto2"; + +import "db-issues.proto"; + +option java_package = "org.sonar.ce.task.projectanalysis.util.cache"; +option optimize_for = SPEED; + +message Issue { + optional string key = 1; + optional int32 ruleType = 2; + optional string componentUuid = 3; + optional string componentKey = 4; + optional string moduleUuid = 5; + optional string moduleUuidPath = 6; + optional string projectUuid = 7; + optional string projectKey = 8; + optional string ruleKey = 9; + optional string language = 10; + optional string severity = 11; + optional bool manualSeverity = 12; + optional string message = 13; + optional int32 line = 14; + optional double gap = 15; + optional int64 effort = 16; + optional string status = 17; + optional string resolution = 18; + optional string assigneeUuid = 19; + optional string checksum = 20; + map attributes = 21; + optional string authorLogin = 22; + optional string tags = 23; + optional sonarqube.db.issues.Locations locations = 24; + + optional bool isFromExternalRuleEngine = 25; + + // FUNCTIONAL DATES + optional int64 creationDate = 26; + optional int64 updateDate = 27; + optional int64 closeDate = 28; + + repeated FieldDiffs changes = 29; + optional FieldDiffs currentChanges = 30; + + optional bool isNew = 31; + optional bool isCopied = 32; + optional bool beingClosed = 33; + optional bool onDisabledRule = 34; + optional bool isChanged = 35; + optional bool sendNotifications = 36; + optional int64 selectedAt = 37; + + repeated Comment comments = 38; +} + +message Comment { + optional string issueKey = 1; + optional string userUuid = 2; + optional int64 createdAt = 3; + optional int64 updatedAt = 4; + optional string key = 5; + optional string markdownText = 6; + optional bool isNew = 7; +} + +message FieldDiffs { + optional string userUuid = 1; + optional int64 creationDate = 2; + optional string issueKey = 3; + map diffs = 4; +} + +message Diff { + optional string oldValue = 1; + optional string newValue = 2; +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitorTest.java index 1ee09d31c0b..abce9e939e8 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitorTest.java @@ -21,10 +21,13 @@ package org.sonar.ce.task.projectanalysis.issue; import java.util.Arrays; import java.util.Collections; +import java.util.Date; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.RuleType; import org.sonar.api.utils.System2; import org.sonar.ce.task.projectanalysis.component.ReportComponent; import org.sonar.ce.task.projectanalysis.component.VisitorsCrawler; @@ -49,14 +52,14 @@ public class CloseIssuesOnRemovedComponentsVisitorTest { ComponentIssuesLoader issuesLoader = mock(ComponentIssuesLoader.class); ComponentsWithUnprocessedIssues componentsWithUnprocessedIssues = mock(ComponentsWithUnprocessedIssues.class); IssueLifecycle issueLifecycle = mock(IssueLifecycle.class); - IssueCache issueCache; + ProtoIssueCache protoIssueCache; VisitorsCrawler underTest; @Before public void setUp() throws Exception { - issueCache = new IssueCache(temp.newFile(), System2.INSTANCE); + protoIssueCache = new ProtoIssueCache(temp.newFile(), System2.INSTANCE); underTest = new VisitorsCrawler( - Arrays.asList(new CloseIssuesOnRemovedComponentsVisitor(issuesLoader, componentsWithUnprocessedIssues, issueCache, issueLifecycle))); + Arrays.asList(new CloseIssuesOnRemovedComponentsVisitor(issuesLoader, componentsWithUnprocessedIssues, protoIssueCache, issueLifecycle))); } @Test @@ -65,13 +68,14 @@ public class CloseIssuesOnRemovedComponentsVisitorTest { String issueUuid = "ABCD"; when(componentsWithUnprocessedIssues.getUuids()).thenReturn(newHashSet(fileUuid)); - DefaultIssue issue = new DefaultIssue().setKey(issueUuid); + DefaultIssue issue = new DefaultIssue().setKey(issueUuid).setType(RuleType.BUG).setCreationDate(new Date()) + .setComponentKey("c").setProjectUuid("u").setProjectKey("k").setRuleKey(RuleKey.of("r", "r")).setStatus("OPEN"); when(issuesLoader.loadOpenIssues(fileUuid)).thenReturn(Collections.singletonList(issue)); underTest.visit(ReportComponent.builder(PROJECT, 1).build()); verify(issueLifecycle).doAutomaticTransition(issue); - CloseableIterator issues = issueCache.traverse(); + CloseableIterator issues = protoIssueCache.traverse(); assertThat(issues.hasNext()).isTrue(); DefaultIssue result = issues.next(); @@ -87,7 +91,7 @@ public class CloseIssuesOnRemovedComponentsVisitorTest { underTest.visit(ReportComponent.builder(PROJECT, 1).build()); verifyZeroInteractions(issueLifecycle); - CloseableIterator issues = issueCache.traverse(); + CloseableIterator issues = protoIssueCache.traverse(); assertThat(issues.hasNext()).isFalse(); } @@ -96,7 +100,7 @@ public class CloseIssuesOnRemovedComponentsVisitorTest { underTest.visit(ReportComponent.builder(DIRECTORY, 1).build()); verifyZeroInteractions(issueLifecycle); - CloseableIterator issues = issueCache.traverse(); + CloseableIterator issues = protoIssueCache.traverse(); assertThat(issues.hasNext()).isFalse(); } @@ -105,7 +109,7 @@ public class CloseIssuesOnRemovedComponentsVisitorTest { underTest.visit(ReportComponent.builder(FILE, 1).build()); verifyZeroInteractions(issueLifecycle); - CloseableIterator issues = issueCache.traverse(); + CloseableIterator issues = protoIssueCache.traverse(); assertThat(issues.hasNext()).isFalse(); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java index 2c8dff9c260..3f228813d62 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java @@ -19,6 +19,7 @@ */ package org.sonar.ce.task.projectanalysis.issue; +import java.util.Date; import java.util.List; import java.util.Optional; import org.junit.Before; @@ -50,6 +51,8 @@ import org.sonar.ce.task.projectanalysis.source.NewLinesRepository; import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository; import org.sonar.ce.task.projectanalysis.source.SourceLinesRepositoryRule; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.FieldDiffs; +import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.issue.tracking.Tracker; import org.sonar.db.DbClient; import org.sonar.db.DbTester; @@ -63,11 +66,13 @@ import org.sonar.db.rule.RuleTesting; import org.sonar.scanner.protocol.Constants; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.server.issue.IssueFieldsSetter; +import org.sonar.server.issue.workflow.IssueWorkflow; import static com.google.common.collect.Lists.newArrayList; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -114,7 +119,9 @@ public class IntegrateIssuesVisitorTest { private AnalysisMetadataHolder analysisMetadataHolder = mock(AnalysisMetadataHolder.class); private IssueFilter issueFilter = mock(IssueFilter.class); private MovedFilesRepository movedFilesRepository = mock(MovedFilesRepository.class); - private IssueLifecycle issueLifecycle = mock(IssueLifecycle.class); + private IssueChangeContext issueChangeContext = mock(IssueChangeContext.class); + private IssueLifecycle issueLifecycle = new IssueLifecycle(analysisMetadataHolder, issueChangeContext, mock(IssueWorkflow.class), new IssueFieldsSetter(), + mock(DebtCalculator.class), ruleRepositoryRule); private IssueVisitor issueVisitor = mock(IssueVisitor.class); private ReferenceBranchComponentUuids mergeBranchComponentsUuids = mock(ReferenceBranchComponentUuids.class); private SiblingsIssueMerger issueStatusCopier = mock(SiblingsIssueMerger.class); @@ -131,7 +138,7 @@ public class IntegrateIssuesVisitorTest { private PullRequestTrackerExecution prBranchTracker; private ReferenceBranchTrackerExecution mergeBranchTracker; private ActiveRulesHolder activeRulesHolder = new AlwaysActiveRulesHolderImpl(); - private IssueCache issueCache; + private ProtoIssueCache protoIssueCache; private TypeAwareVisitor underTest; @@ -154,13 +161,15 @@ public class IntegrateIssuesVisitorTest { mergeBranchTracker = new ReferenceBranchTrackerExecution(rawInputFactory, mergeInputFactory, new Tracker<>()); trackingDelegator = new IssueTrackingDelegator(prBranchTracker, mergeBranchTracker, tracker, analysisMetadataHolder); treeRootHolder.setRoot(PROJECT); - issueCache = new IssueCache(temp.newFile(), System2.INSTANCE); + protoIssueCache = new ProtoIssueCache(temp.newFile(), System2.INSTANCE); when(issueFilter.accept(any(DefaultIssue.class), eq(FILE))).thenReturn(true); - underTest = new IntegrateIssuesVisitor(issueCache, issueLifecycle, issueVisitors, trackingDelegator, issueStatusCopier, referenceBranchComponentUuids); + when(issueChangeContext.date()).thenReturn(new Date()); + underTest = new IntegrateIssuesVisitor(protoIssueCache, issueLifecycle, issueVisitors, trackingDelegator, issueStatusCopier, referenceBranchComponentUuids); } @Test public void process_new_issue() { + ruleRepositoryRule.add(RuleKey.of("xoo", "S001")); when(analysisMetadataHolder.isBranch()).thenReturn(true); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() .setMsg("the message") @@ -173,19 +182,36 @@ public class IntegrateIssuesVisitorTest { underTest.visitAny(FILE); - verify(issueLifecycle).initNewOpenIssue(defaultIssueCaptor.capture()); - DefaultIssue capturedIssue = defaultIssueCaptor.getValue(); - assertThat(capturedIssue.ruleKey().rule()).isEqualTo("S001"); + assertThat(newArrayList(protoIssueCache.traverse())).hasSize(1); + } - verify(issueStatusCopier).tryMerge(FILE, singletonList(capturedIssue)); + @Test + public void process_existing_issue() { - verify(issueLifecycle).doAutomaticTransition(capturedIssue); + RuleKey ruleKey = RuleTesting.XOO_X1; + // Issue from db has severity major + addBaseIssue(ruleKey); + + // Issue from report has severity blocker + ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() + .setMsg("new message") + .setRuleRepository(ruleKey.repository()) + .setRuleKey(ruleKey.rule()) + .setSeverity(Constants.Severity.BLOCKER) + .build(); + reportReader.putIssues(FILE_REF, asList(reportIssue)); + fileSourceRepository.addLine(FILE_REF, "line1"); + + underTest.visitAny(FILE); + + List issues = newArrayList(protoIssueCache.traverse()); + assertThat(issues).hasSize(1); + assertThat(issues.get(0).severity()).isEqualTo(Severity.BLOCKER); - assertThat(newArrayList(issueCache.traverse())).hasSize(1); } @Test - public void process_existing_issue() { + public void dont_cache_existing_issue_if_unmodified() { RuleKey ruleKey = RuleTesting.XOO_X1; // Issue from db has severity major @@ -203,15 +229,7 @@ public class IntegrateIssuesVisitorTest { underTest.visitAny(FILE); - ArgumentCaptor rawIssueCaptor = ArgumentCaptor.forClass(DefaultIssue.class); - ArgumentCaptor baseIssueCaptor = ArgumentCaptor.forClass(DefaultIssue.class); - verify(issueLifecycle).mergeExistingOpenIssue(rawIssueCaptor.capture(), baseIssueCaptor.capture()); - assertThat(rawIssueCaptor.getValue().severity()).isEqualTo(Severity.BLOCKER); - assertThat(baseIssueCaptor.getValue().severity()).isEqualTo(Severity.MAJOR); - - verify(issueLifecycle).doAutomaticTransition(defaultIssueCaptor.capture()); - assertThat(defaultIssueCaptor.getValue().ruleKey()).isEqualTo(ruleKey); - List issues = newArrayList(issueCache.traverse()); + List issues = newArrayList(protoIssueCache.traverse()); assertThat(issues).hasSize(1); assertThat(issues.get(0).severity()).isEqualTo(Severity.BLOCKER); @@ -219,6 +237,7 @@ public class IntegrateIssuesVisitorTest { @Test public void execute_issue_visitors() { + ruleRepositoryRule.add(RuleKey.of("xoo", "S001")); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() .setMsg("the message") .setRuleRepository("xoo") @@ -242,13 +261,10 @@ public class IntegrateIssuesVisitorTest { addBaseIssue(ruleKey); // No issue in the report - underTest.visitAny(FILE); - verify(issueLifecycle).doAutomaticTransition(defaultIssueCaptor.capture()); - assertThat(defaultIssueCaptor.getValue().isBeingClosed()).isTrue(); - List issues = newArrayList(issueCache.traverse()); - assertThat(issues).hasSize(1); + List issues = newArrayList(protoIssueCache.traverse()); + assertThat(issues).hasSize(0); } @Test @@ -289,17 +305,13 @@ public class IntegrateIssuesVisitorTest { underTest.visitAny(FILE); - ArgumentCaptor rawIssueCaptor = ArgumentCaptor.forClass(DefaultIssue.class); - ArgumentCaptor baseIssueCaptor = ArgumentCaptor.forClass(DefaultIssue.class); - verify(issueLifecycle).copyExistingOpenIssueFromBranch(rawIssueCaptor.capture(), baseIssueCaptor.capture(), eq("master")); - assertThat(rawIssueCaptor.getValue().severity()).isEqualTo(Severity.BLOCKER); - assertThat(baseIssueCaptor.getValue().severity()).isEqualTo(Severity.MAJOR); - - verify(issueLifecycle).doAutomaticTransition(defaultIssueCaptor.capture()); - assertThat(defaultIssueCaptor.getValue().ruleKey()).isEqualTo(ruleKey); - List issues = newArrayList(issueCache.traverse()); + List issues = newArrayList(protoIssueCache.traverse()); assertThat(issues).hasSize(1); assertThat(issues.get(0).severity()).isEqualTo(Severity.BLOCKER); + assertThat(issues.get(0).isNew()).isFalse(); + assertThat(issues.get(0).isCopied()).isTrue(); + assertThat(issues.get(0).changes()).hasSize(1); + assertThat(issues.get(0).changes().get(0).diffs()).contains(entry(IssueFieldsSetter.FROM_BRANCH, new FieldDiffs.Diff("master",null))); } private void addBaseIssue(RuleKey ruleKey) { diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java index 71ab05babeb..5941fe60b4e 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java @@ -34,7 +34,7 @@ import org.sonar.api.utils.System2; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; import org.sonar.ce.task.projectanalysis.batch.BatchReportReaderRule; import org.sonar.ce.task.projectanalysis.issue.AdHocRuleCreator; -import org.sonar.ce.task.projectanalysis.issue.IssueCache; +import org.sonar.ce.task.projectanalysis.issue.ProtoIssueCache; import org.sonar.ce.task.projectanalysis.issue.RuleRepositoryImpl; import org.sonar.ce.task.projectanalysis.issue.UpdateConflictResolver; import org.sonar.ce.task.projectanalysis.util.cache.DiskCache; @@ -60,6 +60,7 @@ import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.data.MapEntry.entry; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -88,7 +89,7 @@ public class PersistIssuesStepTest extends BaseStepTest { private DbSession session = db.getSession(); private DbClient dbClient = db.getDbClient(); private UpdateConflictResolver conflictResolver = mock(UpdateConflictResolver.class); - private IssueCache issueCache; + private ProtoIssueCache protoIssueCache; private ComputationStep underTest; private AdHocRuleCreator adHocRuleCreator = mock(AdHocRuleCreator.class); @@ -100,10 +101,10 @@ public class PersistIssuesStepTest extends BaseStepTest { @Before public void setup() throws Exception { - issueCache = new IssueCache(temp.newFile(), System2.INSTANCE); + protoIssueCache = new ProtoIssueCache(temp.newFile(), System2.INSTANCE); reportReader.setMetadata(ScannerReport.Metadata.getDefaultInstance()); - underTest = new PersistIssuesStep(dbClient, system2, conflictResolver, new RuleRepositoryImpl(adHocRuleCreator, dbClient, analysisMetadataHolder), issueCache, + underTest = new PersistIssuesStep(dbClient, system2, conflictResolver, new RuleRepositoryImpl(adHocRuleCreator, dbClient, analysisMetadataHolder), protoIssueCache, new IssueStorage()); } @@ -121,17 +122,20 @@ public class PersistIssuesStepTest extends BaseStepTest { ComponentDto file = db.components().insertComponent(newFileDto(project, null)); when(system2.now()).thenReturn(NOW); - issueCache.newAppender().append(new DefaultIssue() + protoIssueCache.newAppender().append(new DefaultIssue() .setKey("ISSUE") .setType(RuleType.CODE_SMELL) .setRuleKey(rule.getKey()) .setComponentUuid(file.uuid()) + .setComponentKey(file.getKey()) .setProjectUuid(project.uuid()) + .setProjectKey(project.getKey()) .setSeverity(BLOCKER) .setStatus(STATUS_OPEN) .setNew(false) .setCopied(true) .setType(RuleType.BUG) + .setCreationDate(new Date(NOW)) .setSelectedAt(NOW) .addComment(new DefaultIssueComment() .setKey("COMMENT") @@ -139,6 +143,7 @@ public class PersistIssuesStepTest extends BaseStepTest { .setUserUuid("john_uuid") .setMarkdownText("Some text") .setCreatedAt(new Date(NOW)) + .setUpdatedAt(new Date(NOW)) .setNew(true)) .setCurrentChange( new FieldDiffs() @@ -162,8 +167,8 @@ public class PersistIssuesStepTest extends BaseStepTest { List changes = dbClient.issueChangeDao().selectByIssueKeys(session, Arrays.asList("ISSUE")); assertThat(changes).extracting(IssueChangeDto::getChangeType).containsExactly(IssueChangeDto.TYPE_COMMENT, IssueChangeDto.TYPE_FIELD_CHANGE); - assertThat(context.getStatistics().getAll()).containsOnly( - entry("inserts", "1"), entry("updates", "0"), entry("merged", "0"), entry("untouched", "0")); + assertThat(context.getStatistics().getAll()).contains( + entry("inserts", "1"), entry("updates", "0"), entry("merged", "0")); } @Test @@ -175,23 +180,27 @@ public class PersistIssuesStepTest extends BaseStepTest { ComponentDto file = db.components().insertComponent(newFileDto(project, null)); when(system2.now()).thenReturn(NOW); - issueCache.newAppender().append(new DefaultIssue() + protoIssueCache.newAppender().append(new DefaultIssue() .setKey("ISSUE") .setType(RuleType.CODE_SMELL) .setRuleKey(rule.getKey()) .setComponentUuid(file.uuid()) + .setComponentKey(file.getKey()) .setProjectUuid(project.uuid()) + .setProjectKey(project.getKey()) .setSeverity(BLOCKER) .setStatus(STATUS_OPEN) .setNew(true) .setCopied(true) .setType(RuleType.BUG) + .setCreationDate(new Date(NOW)) .setSelectedAt(NOW) .addComment(new DefaultIssueComment() .setKey("COMMENT") .setIssueKey("ISSUE") .setUserUuid("john_uuid") .setMarkdownText("Some text") + .setUpdatedAt(new Date(NOW)) .setCreatedAt(new Date(NOW)) .setNew(true)) .setCurrentChange(new FieldDiffs() @@ -215,8 +224,8 @@ public class PersistIssuesStepTest extends BaseStepTest { List changes = dbClient.issueChangeDao().selectByIssueKeys(session, Arrays.asList("ISSUE")); assertThat(changes).extracting(IssueChangeDto::getChangeType).containsExactly(IssueChangeDto.TYPE_COMMENT, IssueChangeDto.TYPE_FIELD_CHANGE); - assertThat(context.getStatistics().getAll()).containsOnly( - entry("inserts", "1"), entry("updates", "0"), entry("merged", "0"), entry("untouched", "0")); + assertThat(context.getStatistics().getAll()).contains( + entry("inserts", "1"), entry("updates", "0"), entry("merged", "0")); } @Test @@ -233,7 +242,7 @@ public class PersistIssuesStepTest extends BaseStepTest { // simulate the issue has been updated after the analysis ran .setUpdatedAt(NOW + 1_000_000_000L)); issue = dbClient.issueDao().selectByKey(db.getSession(), issue.getKey()).get(); - DiskCache.DiskAppender issueCacheAppender = issueCache.newAppender(); + DiskCache.CacheAppender issueCacheAppender = protoIssueCache.newAppender(); when(system2.now()).thenReturn(NOW); DefaultIssue defaultIssue = issue.toDefaultIssue() @@ -250,8 +259,8 @@ public class PersistIssuesStepTest extends BaseStepTest { ArgumentCaptor issueDtoCaptor = ArgumentCaptor.forClass(IssueDto.class); verify(conflictResolver).resolve(eq(defaultIssue), issueDtoCaptor.capture(), any(IssueMapper.class)); assertThat(issueDtoCaptor.getValue().getId()).isEqualTo(issue.getId()); - assertThat(context.getStatistics().getAll()).containsOnly( - entry("inserts", "0"), entry("updates", "1"), entry("merged", "1"), entry("untouched", "0")); + assertThat(context.getStatistics().getAll()).contains( + entry("inserts", "0"), entry("updates", "1"), entry("merged", "1")); } @@ -264,14 +273,17 @@ public class PersistIssuesStepTest extends BaseStepTest { ComponentDto file = db.components().insertComponent(newFileDto(project, null)); session.commit(); - issueCache.newAppender().append(new DefaultIssue() + protoIssueCache.newAppender().append(new DefaultIssue() .setKey("ISSUE") .setType(RuleType.CODE_SMELL) .setRuleKey(rule.getKey()) .setComponentUuid(file.uuid()) + .setComponentKey(file.getKey()) .setProjectUuid(project.uuid()) + .setProjectKey(project.getKey()) .setSeverity(BLOCKER) .setStatus(STATUS_OPEN) + .setCreationDate(new Date(NOW)) .setNew(true) .setType(RuleType.BUG)).close(); @@ -286,8 +298,8 @@ public class PersistIssuesStepTest extends BaseStepTest { assertThat(result.getSeverity()).isEqualTo(BLOCKER); assertThat(result.getStatus()).isEqualTo(STATUS_OPEN); assertThat(result.getType()).isEqualTo(RuleType.BUG.getDbConstant()); - assertThat(context.getStatistics().getAll()).containsOnly( - entry("inserts", "1"), entry("updates", "0"), entry("merged", "0"), entry("untouched", "0")); + assertThat(context.getStatistics().getAll()).contains( + entry("inserts", "1"), entry("updates", "0"), entry("merged", "0")); } @Test @@ -300,7 +312,7 @@ public class PersistIssuesStepTest extends BaseStepTest { .setResolution(null) .setCreatedAt(NOW - 1_000_000_000L) .setUpdatedAt(NOW - 1_000_000_000L)); - DiskCache.DiskAppender issueCacheAppender = issueCache.newAppender(); + DiskCache.CacheAppender issueCacheAppender = protoIssueCache.newAppender(); issueCacheAppender.append( issue.toDefaultIssue() @@ -317,8 +329,8 @@ public class PersistIssuesStepTest extends BaseStepTest { IssueDto issueReloaded = db.getDbClient().issueDao().selectByKey(db.getSession(), issue.getKey()).get(); assertThat(issueReloaded.getStatus()).isEqualTo(STATUS_CLOSED); assertThat(issueReloaded.getResolution()).isEqualTo(RESOLUTION_FIXED); - assertThat(context.getStatistics().getAll()).containsOnly( - entry("inserts", "0"), entry("updates", "1"), entry("merged", "0"), entry("untouched", "0")); + assertThat(context.getStatistics().getAll()).contains( + entry("inserts", "0"), entry("updates", "1"), entry("merged", "0")); } @Test @@ -331,7 +343,7 @@ public class PersistIssuesStepTest extends BaseStepTest { .setResolution(null) .setCreatedAt(NOW - 1_000_000_000L) .setUpdatedAt(NOW - 1_000_000_000L)); - DiskCache.DiskAppender issueCacheAppender = issueCache.newAppender(); + DiskCache.CacheAppender issueCacheAppender = protoIssueCache.newAppender(); issueCacheAppender.append( issue.toDefaultIssue() @@ -346,6 +358,7 @@ public class PersistIssuesStepTest extends BaseStepTest { .setUserUuid("john_uuid") .setMarkdownText("Some text") .setCreatedAt(new Date(NOW)) + .setUpdatedAt(new Date(NOW)) .setNew(true))) .close(); @@ -357,8 +370,8 @@ public class PersistIssuesStepTest extends BaseStepTest { .extracting(IssueChangeDto::getChangeType, IssueChangeDto::getUserUuid, IssueChangeDto::getChangeData, IssueChangeDto::getIssueKey, IssueChangeDto::getIssueChangeCreationDate) .containsOnly(IssueChangeDto.TYPE_COMMENT, "john_uuid", "Some text", issue.getKey(), NOW); - assertThat(context.getStatistics().getAll()).containsOnly( - entry("inserts", "0"), entry("updates", "1"), entry("merged", "0"), entry("untouched", "0")); + assertThat(context.getStatistics().getAll()).contains( + entry("inserts", "0"), entry("updates", "1"), entry("merged", "0")); } @Test @@ -371,7 +384,7 @@ public class PersistIssuesStepTest extends BaseStepTest { .setResolution(null) .setCreatedAt(NOW - 1_000_000_000L) .setUpdatedAt(NOW - 1_000_000_000L)); - DiskCache.DiskAppender issueCacheAppender = issueCache.newAppender(); + DiskCache.CacheAppender issueCacheAppender = protoIssueCache.newAppender(); issueCacheAppender.append( issue.toDefaultIssue() @@ -395,8 +408,8 @@ public class PersistIssuesStepTest extends BaseStepTest { .extracting(IssueChangeDto::getChangeType, IssueChangeDto::getUserUuid, IssueChangeDto::getChangeData, IssueChangeDto::getIssueKey, IssueChangeDto::getIssueChangeCreationDate) .containsOnly(IssueChangeDto.TYPE_FIELD_CHANGE, "john_uuid", "technicalDebt=1", issue.getKey(), NOW); - assertThat(context.getStatistics().getAll()).containsOnly( - entry("inserts", "0"), entry("updates", "1"), entry("merged", "0"), entry("untouched", "0")); + assertThat(context.getStatistics().getAll()).contains( + entry("inserts", "0"), entry("updates", "1"), entry("merged", "0")); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java index 11be48ed072..91ddd0ada48 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java @@ -42,6 +42,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.sonar.api.notifications.Notification; +import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; import org.sonar.api.utils.System2; @@ -50,7 +51,7 @@ import org.sonar.ce.task.projectanalysis.analysis.Branch; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.DefaultBranchImpl; import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; -import org.sonar.ce.task.projectanalysis.issue.IssueCache; +import org.sonar.ce.task.projectanalysis.issue.ProtoIssueCache; import org.sonar.ce.task.projectanalysis.notification.NotificationFactory; import org.sonar.ce.task.projectanalysis.util.cache.DiskCache; import org.sonar.ce.task.step.ComputationStep; @@ -85,6 +86,7 @@ import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -150,13 +152,13 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { private NewIssuesNotification newIssuesNotificationMock = createNewIssuesNotificationMock(); private MyNewIssuesNotification myNewIssuesNotificationMock = createMyNewIssuesNotificationMock(); - private IssueCache issueCache; + private ProtoIssueCache protoIssueCache; private SendIssueNotificationsStep underTest; @Before public void setUp() throws Exception { - issueCache = new IssueCache(temp.newFile(), System2.INSTANCE); - underTest = new SendIssueNotificationsStep(issueCache, treeRootHolder, notificationService, analysisMetadataHolder, + protoIssueCache = new ProtoIssueCache(temp.newFile(), System2.INSTANCE); + underTest = new SendIssueNotificationsStep(protoIssueCache, treeRootHolder, notificationService, analysisMetadataHolder, notificationFactory, db.getDbClient()); when(notificationFactory.newNewIssuesNotification(any(assigneeCacheType))).thenReturn(newIssuesNotificationMock); when(notificationFactory.newMyNewIssuesNotification(any(assigneeCacheType))).thenReturn(myNewIssuesNotificationMock); @@ -178,8 +180,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { @Test public void send_global_new_issues_notification() { analysisMetadataHolder.setProject(new Project(PROJECT.getUuid(), PROJECT.getKey(), PROJECT.getName(), null, emptyList())); - issueCache.newAppender().append( - new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION) + protoIssueCache.newAppender().append( + createIssue().setType(randomRuleType).setEffort(ISSUE_DURATION) .setCreationDate(new Date(ANALYSE_DATE))) .close(); when(notificationService.hasProjectSubscribersForTypes(eq(PROJECT.getUuid()), any())).thenReturn(true); @@ -202,15 +204,16 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { Integer[] backDatedEfforts = IntStream.range(0, 1 + random.nextInt(10)).mapToObj(i -> 10 + random.nextInt(100)).toArray(Integer[]::new); Duration expectedEffort = Duration.create(stream(efforts).mapToInt(i -> i).sum()); List issues = concat(stream(efforts) - .map(effort -> new DefaultIssue().setType(randomRuleType).setEffort(Duration.create(effort)) + .map(effort -> createIssue().setType(randomRuleType).setEffort(Duration.create(effort)) .setCreationDate(new Date(ANALYSE_DATE))), stream(backDatedEfforts) - .map(effort -> new DefaultIssue().setType(randomRuleType).setEffort(Duration.create(effort)) + .map(effort -> createIssue().setType(randomRuleType).setEffort(Duration.create(effort)) .setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS)))) .collect(toList()); shuffle(issues); - DiskCache.DiskAppender issueCache = this.issueCache.newAppender(); + DiskCache.CacheAppender issueCache = this.protoIssueCache.newAppender(); issues.forEach(issueCache::append); + issueCache.close(); analysisMetadataHolder.setProject(new Project(PROJECT.getUuid(), PROJECT.getKey(), PROJECT.getName(), null, emptyList())); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); @@ -233,8 +236,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { @Test public void do_not_send_global_new_issues_notification_if_issue_has_been_backdated() { analysisMetadataHolder.setProject(new Project(PROJECT.getUuid(), PROJECT.getKey(), PROJECT.getName(), null, emptyList())); - issueCache.newAppender().append( - new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION) + protoIssueCache.newAppender().append( + createIssue().setType(randomRuleType).setEffort(ISSUE_DURATION) .setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS))) .close(); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); @@ -251,8 +254,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { public void send_global_new_issues_notification_on_branch() { ComponentDto project = newPrivateProjectDto(newOrganizationDto()); ComponentDto branch = setUpBranch(project, BRANCH); - issueCache.newAppender().append( - new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE))).close(); + protoIssueCache.newAppender().append( + createIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE))).close(); when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), NOTIF_TYPES)).thenReturn(true); analysisMetadataHolder.setProject(Project.from(project)); analysisMetadataHolder.setBranch(newBranch(BranchType.BRANCH)); @@ -272,8 +275,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { public void do_not_send_global_new_issues_notification_on_pull_request() { ComponentDto project = newPrivateProjectDto(newOrganizationDto()); ComponentDto branch = setUpBranch(project, PULL_REQUEST); - issueCache.newAppender().append( - new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE))).close(); + protoIssueCache.newAppender().append( + createIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE))).close(); when(notificationService.hasProjectSubscribersForTypes(project.uuid(), NOTIF_TYPES)).thenReturn(true); analysisMetadataHolder.setProject(Project.from(project)); analysisMetadataHolder.setBranch(newPullRequest()); @@ -285,12 +288,16 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { verifyZeroInteractions(notificationService, newIssuesNotificationMock); } + private DefaultIssue createIssue() { + return new DefaultIssue().setKey("k").setProjectKey("p").setStatus("OPEN").setProjectUuid("uuid").setComponentKey("c").setRuleKey(RuleKey.of("r", "r")); + } + @Test public void do_not_send_global_new_issues_notification_on_branch_if_issue_has_been_backdated() { ComponentDto project = newPrivateProjectDto(newOrganizationDto()); ComponentDto branch = setUpBranch(project, BRANCH); - issueCache.newAppender().append( - new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS))).close(); + protoIssueCache.newAppender().append( + createIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS))).close(); when(notificationService.hasProjectSubscribersForTypes(branch.uuid(), NOTIF_TYPES)).thenReturn(true); analysisMetadataHolder.setProject(Project.from(project)); analysisMetadataHolder.setBranch(newBranch(BranchType.BRANCH)); @@ -308,9 +315,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { UserDto user = db.users().insertUser(); analysisMetadataHolder.setProject(new Project(PROJECT.getUuid(), PROJECT.getKey(), PROJECT.getName(), null, emptyList())); - issueCache.newAppender().append( - new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setAssigneeUuid(user.getUuid()) - .setCreationDate(new Date(ANALYSE_DATE))) + protoIssueCache.newAppender().append( + createIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setAssigneeUuid(user.getUuid()).setCreationDate(new Date(ANALYSE_DATE))) .close(); when(notificationService.hasProjectSubscribersForTypes(eq(PROJECT.getUuid()), any())).thenReturn(true); @@ -340,19 +346,21 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { Integer[] assignedToOther = IntStream.range(0, 3).mapToObj(i -> 10).toArray(Integer[]::new); List issues = concat(stream(assigned) - .map(effort -> new DefaultIssue().setType(randomRuleType).setEffort(Duration.create(effort)) + .map(effort -> createIssue().setType(randomRuleType).setEffort(Duration.create(effort)) .setAssigneeUuid(perceval.getUuid()) + .setNew(true) .setCreationDate(new Date(ANALYSE_DATE))), stream(assignedToOther) - .map(effort -> new DefaultIssue().setType(randomRuleType).setEffort(Duration.create(effort)) + .map(effort -> createIssue().setType(randomRuleType).setEffort(Duration.create(effort)) .setAssigneeUuid(arthur.getUuid()) + .setNew(true) .setCreationDate(new Date(ANALYSE_DATE)))) .collect(toList()); shuffle(issues); - IssueCache issueCache = new IssueCache(temp.newFile(), System2.INSTANCE); - DiskCache.DiskAppender newIssueCache = issueCache.newAppender(); + ProtoIssueCache protoIssueCache = new ProtoIssueCache(temp.newFile(), System2.INSTANCE); + DiskCache.CacheAppender newIssueCache = protoIssueCache.newAppender(); issues.forEach(newIssueCache::append); - + newIssueCache.close(); analysisMetadataHolder.setProject(new Project(PROJECT.getUuid(), PROJECT.getKey(), PROJECT.getName(), null, emptyList())); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); @@ -363,12 +371,10 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { MyNewIssuesNotification myNewIssuesNotificationMock1 = createMyNewIssuesNotificationMock(); MyNewIssuesNotification myNewIssuesNotificationMock2 = createMyNewIssuesNotificationMock(); - when(notificationFactory.newMyNewIssuesNotification(any(assigneeCacheType))) - .thenReturn(myNewIssuesNotificationMock1) - .thenReturn(myNewIssuesNotificationMock2); + doReturn(myNewIssuesNotificationMock1).doReturn(myNewIssuesNotificationMock2).when(notificationFactory).newMyNewIssuesNotification(any(assigneeCacheType)); TestComputationStepContext context = new TestComputationStepContext(); - new SendIssueNotificationsStep(issueCache, treeRootHolder, notificationService, analysisMetadataHolder, notificationFactory, db.getDbClient()) + new SendIssueNotificationsStep(protoIssueCache, treeRootHolder, notificationService, analysisMetadataHolder, notificationFactory, db.getDbClient()) .execute(context); verify(notificationService).deliverEmails(ImmutableSet.of(myNewIssuesNotificationMock1, myNewIssuesNotificationMock2)); @@ -413,17 +419,18 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { Integer[] backDatedEfforts = IntStream.range(0, 1 + random.nextInt(10)).mapToObj(i -> 10 + random.nextInt(100)).toArray(Integer[]::new); Duration expectedEffort = Duration.create(stream(efforts).mapToInt(i -> i).sum()); List issues = concat(stream(efforts) - .map(effort -> new DefaultIssue().setType(randomRuleType).setEffort(Duration.create(effort)) + .map(effort -> createIssue().setType(randomRuleType).setEffort(Duration.create(effort)) .setAssigneeUuid(user.getUuid()) .setCreationDate(new Date(ANALYSE_DATE))), stream(backDatedEfforts) - .map(effort -> new DefaultIssue().setType(randomRuleType).setEffort(Duration.create(effort)) + .map(effort -> createIssue().setType(randomRuleType).setEffort(Duration.create(effort)) .setAssigneeUuid(user.getUuid()) .setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS)))) .collect(toList()); shuffle(issues); - DiskCache.DiskAppender issueCache = this.issueCache.newAppender(); + DiskCache.CacheAppender issueCache = this.protoIssueCache.newAppender(); issues.forEach(issueCache::append); + issueCache.close(); analysisMetadataHolder.setProject(new Project(PROJECT.getUuid(), PROJECT.getKey(), PROJECT.getName(), null, emptyList())); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); @@ -469,8 +476,8 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { public void do_not_send_new_issues_notification_to_user_if_issue_is_backdated() { analysisMetadataHolder.setProject(new Project(PROJECT.getUuid(), PROJECT.getKey(), PROJECT.getName(), null, emptyList())); UserDto user = db.users().insertUser(); - issueCache.newAppender().append( - new DefaultIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setAssigneeUuid(user.getUuid()) + protoIssueCache.newAppender().append( + createIssue().setType(randomRuleType).setEffort(ISSUE_DURATION).setAssigneeUuid(user.getUuid()) .setCreationDate(new Date(ANALYSE_DATE - FIVE_MINUTES_IN_MS))) .close(); when(notificationService.hasProjectSubscribersForTypes(PROJECT.getUuid(), NOTIF_TYPES)).thenReturn(true); @@ -543,7 +550,7 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { private DefaultIssue prepareIssue(long issueCreatedAt, UserDto user, ComponentDto project, ComponentDto file, RuleDefinitionDto ruleDefinitionDto, RuleType type) { DefaultIssue issue = newIssue(ruleDefinitionDto, project, file).setType(type).toDefaultIssue() .setNew(false).setChanged(true).setSendNotifications(true).setCreationDate(new Date(issueCreatedAt)).setAssigneeUuid(user.getUuid()); - issueCache.newAppender().append(issue).close(); + protoIssueCache.newAppender().append(issue).close(); when(notificationService.hasProjectSubscribersForTypes(project.projectUuid(), NOTIF_TYPES)).thenReturn(true); return issue; } @@ -572,7 +579,7 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { .setChanged(true) .setSendNotifications(true) .setCreationDate(new Date(issueCreatedAt)); - issueCache.newAppender().append(issue).close(); + protoIssueCache.newAppender().append(issue).close(); when(notificationService.hasProjectSubscribersForTypes(project.uuid(), NOTIF_TYPES)).thenReturn(true); IssuesChangesNotification issuesChangesNotification = mock(IssuesChangesNotification.class); when(notificationFactory.newIssuesChangesNotification(anySet(), anyMap())).thenReturn(issuesChangesNotification); @@ -601,9 +608,9 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { .mapToObj(i -> newIssue(ruleDefinitionDto, project, file).setKee("uuid_" + i).setType(randomTypeExceptHotspot).toDefaultIssue() .setNew(false).setChanged(true).setSendNotifications(true).setAssigneeUuid(user.getUuid())) .collect(toList()); - DiskCache.DiskAppender diskAppender = issueCache.newAppender(); - issues.forEach(diskAppender::append); - diskAppender.close(); + DiskCache.CacheAppender cacheAppender = protoIssueCache.newAppender(); + issues.forEach(cacheAppender::append); + cacheAppender.close(); analysisMetadataHolder.setProject(Project.from(project)); NewIssuesFactoryCaptor newIssuesFactoryCaptor = new NewIssuesFactoryCaptor(() -> mock(IssuesChangesNotification.class)); when(notificationFactory.newIssuesChangesNotification(anySet(), anyMap())).thenAnswer(newIssuesFactoryCaptor); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/DiskCacheTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/JavaSerializationDiskCacheTest.java similarity index 87% rename from server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/DiskCacheTest.java rename to server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/JavaSerializationDiskCacheTest.java index 5197ad323be..c9e712bfc09 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/DiskCacheTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/JavaSerializationDiskCacheTest.java @@ -19,26 +19,25 @@ */ package org.sonar.ce.task.projectanalysis.util.cache; +import java.io.ObjectOutputStream; +import java.io.Serializable; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.sonar.api.utils.System2; import org.sonar.core.util.CloseableIterator; -import java.io.ObjectOutputStream; -import java.io.Serializable; - import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; -public class DiskCacheTest { +public class JavaSerializationDiskCacheTest { @Rule public TemporaryFolder temp = new TemporaryFolder(); @Test public void write_and_read() throws Exception { - DiskCache cache = new DiskCache<>(temp.newFile(), System2.INSTANCE); + DiskCache cache = new JavaSerializationDiskCache<>(temp.newFile(), System2.INSTANCE); try (CloseableIterator traverse = cache.traverse()) { assertThat(traverse).isExhausted(); } @@ -55,7 +54,7 @@ public class DiskCacheTest { @Test public void fail_if_file_is_not_writable() throws Exception { try { - new DiskCache<>(temp.newFolder(), System2.INSTANCE); + new JavaSerializationDiskCache<>(temp.newFolder(), System2.INSTANCE); fail(); } catch (IllegalStateException e) { assertThat(e).hasMessageContaining("Fail to write into file"); @@ -69,7 +68,7 @@ public class DiskCacheTest { throw new UnsupportedOperationException("expected error"); } } - DiskCache cache = new DiskCache<>(temp.newFile(), System2.INSTANCE); + DiskCache cache = new JavaSerializationDiskCache<>(temp.newFile(), System2.INSTANCE); try { cache.newAppender().append(new Unserializable()); fail(); diff --git a/server/sonar-db-dao/build.gradle b/server/sonar-db-dao/build.gradle index 6877e8d4eba..2b0f2d86f6e 100644 --- a/server/sonar-db-dao/build.gradle +++ b/server/sonar-db-dao/build.gradle @@ -76,3 +76,8 @@ configurations { artifacts { tests testJar } + +jar { + // remove exclusion on proto files so that they can be included by other modules + setExcludes([]) +} diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java index 5a81de71b63..cd91a5d4fbe 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java @@ -213,11 +213,12 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. } @Override + @CheckForNull public String language() { return language; } - public DefaultIssue setLanguage(String l) { + public DefaultIssue setLanguage(@Nullable String l) { this.language = l; return this; } @@ -269,7 +270,7 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. } public DefaultIssue setLine(@Nullable Integer l) { - Preconditions.checkArgument(l == null || l > 0, "Line must be null or greater than zero (got %d)", l); + Preconditions.checkArgument(l == null || l > 0, "Line must be null or greater than zero (got %s)", l); this.line = l; return this; } @@ -507,6 +508,11 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. return this; } + public DefaultIssue setCurrentChangeWithoutAddChange(@Nullable FieldDiffs currentChange) { + this.currentChange = currentChange; + return this; + } + @CheckForNull public FieldDiffs currentChange() { return currentChange; @@ -583,7 +589,7 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. return false; } DefaultIssue that = (DefaultIssue) o; - return !(key != null ? !key.equals(that.key) : (that.key != null)); + return Objects.equals(key, that.key); } @Override diff --git a/sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java b/sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java index d560d79e174..8113a90c610 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/FieldDiffs.java @@ -85,11 +85,12 @@ public class FieldDiffs implements Serializable { return this; } + @CheckForNull public String issueKey() { return issueKey; } - public FieldDiffs setIssueKey(String issueKey) { + public FieldDiffs setIssueKey(@Nullable String issueKey) { this.issueKey = issueKey; return this; } -- 2.39.5