From: Teryk Bellahsene Date: Thu, 26 Mar 2015 14:18:10 +0000 (+0100) Subject: cancel analysis when the snapshot is not found - SONAR-6340 X-Git-Tag: 5.1~10 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=7ebe878335698392cd48fef42b23517cb250b469;p=sonarqube.git cancel analysis when the snapshot is not found - SONAR-6340 --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/ComputationService.java b/server/sonar-server/src/main/java/org/sonar/server/computation/ComputationService.java index bc83b093cac..f8ff6fb8f23 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/ComputationService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/ComputationService.java @@ -32,6 +32,7 @@ import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Profiler; import org.sonar.batch.protocol.output.BatchReportReader; import org.sonar.core.component.ComponentDto; +import org.sonar.core.component.SnapshotDto; import org.sonar.core.computation.db.AnalysisReportDto; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; @@ -41,11 +42,13 @@ import org.sonar.server.computation.step.ComputationStep; import org.sonar.server.computation.step.ComputationSteps; import org.sonar.server.db.DbClient; +import javax.annotation.Nullable; import java.io.File; import java.io.IOException; import static org.sonar.api.utils.DateUtils.formatDateTimeNullSafe; import static org.sonar.api.utils.DateUtils.longToDate; +import static org.sonar.core.computation.db.AnalysisReportDto.Status.*; public class ComputationService implements ServerComponent { @@ -70,10 +73,15 @@ public class ComputationService implements ServerComponent { Profiler profiler = Profiler.create(LOG).startDebug(String.format( "Analysis of project %s (report %d)", item.dto.getProjectKey(), item.dto.getId())); - ComponentDto project = loadProject(item); + ComponentDto project = null; + try { + project = loadProject(item); File reportDir = extractReportInDir(item); BatchReportReader reader = new BatchReportReader(reportDir); + if (isSnapshotMissing(item, reader.readMetadata().getSnapshotId())) { + return; + } ComputationContext context = new ComputationContext(reader, project); for (ComputationStep step : steps.orderedSteps()) { if (ArrayUtils.contains(step.supportedProjectQualifiers(), context.getProject().qualifier())) { @@ -82,10 +90,9 @@ public class ComputationService implements ServerComponent { stepProfiler.stopDebug(); } } - item.dto.succeed(); - + item.dto.setStatus(SUCCESS); } catch (Throwable e) { - item.dto.fail(); + item.dto.setStatus(FAILED); throw Throwables.propagate(e); } finally { item.dto.setFinishedAt(system.now()); @@ -110,28 +117,47 @@ public class ComputationService implements ServerComponent { } } - private ComponentDto loadProject(ReportQueue.Item queueItem) { + private ComponentDto loadProject(ReportQueue.Item item) { DbSession session = dbClient.openSession(false); try { - return dbClient.componentDao().getByKey(session, queueItem.dto.getProjectKey()); + return dbClient.componentDao().getByKey(session, item.dto.getProjectKey()); } finally { MyBatis.closeQuietly(session); } } - private void saveActivity(AnalysisReportDto report, ComponentDto project) { + private boolean isSnapshotMissing(ReportQueue.Item item, long snapshotId) { + DbSession session = dbClient.openSession(false); + try { + SnapshotDto snapshot = dbClient.snapshotDao().getNullableByKey(session, snapshotId); + if (snapshot == null) { + item.dto.setStatus(CANCELLED); + LOG.info("Processing of report #{} is canceled because it was submitted while another report of the same project was already being processed.", item.dto.getId()); + LOG.debug("The snapshot ID #{} provided by the report #{} does not exist anymore.", snapshotId, item.dto.getId()); + } + return snapshot==null; + } finally { + MyBatis.closeQuietly(session); + } + + } + + private void saveActivity(AnalysisReportDto report, @Nullable ComponentDto project) { Activity activity = new Activity(); activity.setType(Activity.Type.ANALYSIS_REPORT); activity.setAction("LOG_ANALYSIS_REPORT"); activity .setData("key", String.valueOf(report.getId())) - .setData("projectKey", project.key()) - .setData("projectName", project.name()) - .setData("projectUuid", project.uuid()) + .setData("projectKey", report.getProjectKey()) .setData("status", String.valueOf(report.getStatus())) .setData("submittedAt", formatDateTimeNullSafe(longToDate(report.getCreatedAt()))) .setData("startedAt", formatDateTimeNullSafe(longToDate(report.getStartedAt()))) .setData("finishedAt", formatDateTimeNullSafe(longToDate(report.getFinishedAt()))); + if (project != null) { + activity + .setData("projectName", project.name()) + .setData("projectUuid", project.uuid()); + } activityService.save(activity); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/ReportQueue.java b/server/sonar-server/src/main/java/org/sonar/server/computation/ReportQueue.java index 8139db0d050..ebddc02357c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/ReportQueue.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/ReportQueue.java @@ -29,6 +29,7 @@ import org.sonar.core.computation.db.AnalysisReportDto; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; import org.sonar.process.ProcessConstants; +import org.sonar.server.computation.db.AnalysisReportDao; import org.sonar.server.db.DbClient; import javax.annotation.CheckForNull; @@ -41,17 +42,6 @@ import java.util.List; import static org.sonar.core.computation.db.AnalysisReportDto.Status.PENDING; public class ReportQueue implements ServerComponent { - - public static class Item { - public final AnalysisReportDto dto; - public final File zipFile; - - public Item(AnalysisReportDto dto, File zipFile) { - this.dto = dto; - this.zipFile = zipFile; - } - } - private final DbClient dbClient; private final Settings settings; @@ -68,18 +58,10 @@ public class ReportQueue implements ServerComponent { try { checkThatProjectExistsInDatabase(projectKey, session); - // save report data on file. Directory is created if it does not exist yet. - FileUtils.copyInputStreamToFile(reportData, file); + saveReportOnDisk(reportData, file); + AnalysisReportDto dto = saveReportMetadataInDatabase(projectKey, uuid, session); - // add report metadata to the queue - AnalysisReportDto dto = new AnalysisReportDto() - .setProjectKey(projectKey) - .setStatus(PENDING) - .setUuid(uuid); - dbClient.analysisReportDao().insert(session, dto); - session.commit(); return new Item(dto, file); - } catch (Exception e) { FileUtils.deleteQuietly(file); throw new IllegalStateException("Fail to store analysis report of project " + projectKey, e); @@ -88,6 +70,24 @@ public class ReportQueue implements ServerComponent { } } + private AnalysisReportDto saveReportMetadataInDatabase(String projectKey, String uuid, DbSession session) { + AnalysisReportDto dto = new AnalysisReportDto() + .setProjectKey(projectKey) + .setStatus(PENDING) + .setUuid(uuid); + dao().insert(session, dto); + session.commit(); + return dto; + } + + private AnalysisReportDao dao() { + return dbClient.analysisReportDao(); + } + + private void saveReportOnDisk(InputStream reportData, File file) throws IOException { + FileUtils.copyInputStreamToFile(reportData, file); + } + private void checkThatProjectExistsInDatabase(String projectKey, DbSession session) { dbClient.componentDao().getByKey(session, projectKey); } @@ -96,7 +96,7 @@ public class ReportQueue implements ServerComponent { DbSession session = dbClient.openSession(false); try { FileUtils.deleteQuietly(item.zipFile); - dbClient.analysisReportDao().delete(session, item.dto.getId()); + dao().delete(session, item.dto.getId()); session.commit(); } finally { MyBatis.closeQuietly(session); @@ -107,14 +107,14 @@ public class ReportQueue implements ServerComponent { public Item pop() { DbSession session = dbClient.openSession(false); try { - AnalysisReportDto dto = dbClient.analysisReportDao().pop(session); + AnalysisReportDto dto = dao().pop(session); if (dto != null) { File file = reportFileForUuid(dto.getUuid()); if (file.exists()) { return new Item(dto, file); } Loggers.get(getClass()).error("Analysis report not found: " + file.getAbsolutePath()); - dbClient.analysisReportDao().delete(session, dto.getId()); + dao().delete(session, dto.getId()); session.commit(); } return null; @@ -123,10 +123,10 @@ public class ReportQueue implements ServerComponent { } } - public List findByProjectKey(String projectKey) { + public List selectByProjectKey(String projectKey) { DbSession session = dbClient.openSession(false); try { - return dbClient.analysisReportDao().selectByProjectKey(session, projectKey); + return dao().selectByProjectKey(session, projectKey); } finally { MyBatis.closeQuietly(session); } @@ -145,7 +145,7 @@ public class ReportQueue implements ServerComponent { DbSession session = dbClient.openSession(false); try { - dbClient.analysisReportDao().truncate(session); + dao().truncate(session); session.commit(); } finally { MyBatis.closeQuietly(session); @@ -155,7 +155,7 @@ public class ReportQueue implements ServerComponent { public void resetToPendingStatus() { DbSession session = dbClient.openSession(false); try { - dbClient.analysisReportDao().resetAllToPendingStatus(session); + dao().resetAllToPendingStatus(session); session.commit(); } finally { MyBatis.closeQuietly(session); @@ -168,7 +168,7 @@ public class ReportQueue implements ServerComponent { public List all() { DbSession session = dbClient.openSession(false); try { - return dbClient.analysisReportDao().selectAll(session); + return dao().selectAll(session); } finally { MyBatis.closeQuietly(session); } @@ -185,4 +185,14 @@ public class ReportQueue implements ServerComponent { private File reportFileForUuid(String uuid) { return new File(reportsDir(), String.format("%s.zip", uuid)); } + + public static class Item { + public final AnalysisReportDto dto; + public final File zipFile; + + public Item(AnalysisReportDto dto, File zipFile) { + this.dto = dto; + this.zipFile = zipFile; + } + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/ComputationServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/ComputationServiceTest.java index adeecf0d06f..dbbd25a4e55 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/ComputationServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/ComputationServiceTest.java @@ -50,13 +50,15 @@ import org.sonar.api.utils.ZipUtils; import org.sonar.api.utils.internal.JUnitTempFolder; import org.sonar.api.utils.log.LogTester; import org.sonar.api.utils.log.LoggerLevel; -import org.sonar.batch.protocol.output.BatchReportWriter; import org.sonar.batch.protocol.output.BatchReport; +import org.sonar.batch.protocol.output.BatchReportWriter; import org.sonar.core.computation.db.AnalysisReportDto; +import org.sonar.core.computation.db.AnalysisReportDto.Status; import org.sonar.core.persistence.DbTester; import org.sonar.server.activity.Activity; import org.sonar.server.activity.ActivityService; import org.sonar.server.component.db.ComponentDao; +import org.sonar.server.component.db.SnapshotDao; import org.sonar.server.computation.step.ComputationStep; import org.sonar.server.computation.step.ComputationSteps; import org.sonar.server.db.DbClient; @@ -72,15 +74,13 @@ import static org.mockito.Mockito.*; public class ComputationServiceTest { + private static final long ANY_SNAPSHOT_ID = 54987654231L; @ClassRule public static DbTester dbTester = new DbTester(); - @Rule public JUnitTempFolder tempFolder = new JUnitTempFolder(); - @Rule public LogTester logTester = new LogTester(); - ComputationStep projectStep1 = mockStep(Qualifiers.PROJECT); ComputationStep projectStep2 = mockStep(Qualifiers.PROJECT); ComputationStep viewStep = mockStep(Qualifiers.VIEW); @@ -91,7 +91,7 @@ public class ComputationServiceTest { @Before public void setUp() throws Exception { - DbClient dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), new ComponentDao()); + DbClient dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), new ComponentDao(), new SnapshotDao(system)); sut = new ComputationService(dbClient, steps, activityService, tempFolder, system); // db contains project with key "P1" @@ -104,13 +104,13 @@ public class ComputationServiceTest { // view step is not supposed to be executed when(steps.orderedSteps()).thenReturn(Arrays.asList(projectStep1, projectStep2, viewStep)); - AnalysisReportDto dto = AnalysisReportDto.newForTests(1L).setProjectKey("P1").setUuid("U1"); + AnalysisReportDto dto = newDefaultReport(); File zip = generateZip(); sut.process(new ReportQueue.Item(dto, zip)); // report is integrated -> status is set to SUCCESS - assertThat(dto.getStatus()).isEqualTo(AnalysisReportDto.Status.SUCCESS); + assertThat(dto.getStatus()).isEqualTo(Status.SUCCESS); assertThat(dto.getFinishedAt()).isNotNull(); // one info log at the end @@ -124,11 +124,15 @@ public class ComputationServiceTest { verify(activityService).save(any(Activity.class)); } + private AnalysisReportDto newDefaultReport() { + return AnalysisReportDto.newForTests(1L).setProjectKey("P1").setUuid("U1").setStatus(Status.PENDING); + } + @Test public void debug_logs() throws Exception { logTester.setLevel(LoggerLevel.DEBUG); - AnalysisReportDto dto = AnalysisReportDto.newForTests(1L).setProjectKey("P1").setUuid("U1"); + AnalysisReportDto dto = newDefaultReport(); File zip = generateZip(); sut.process(new ReportQueue.Item(dto, zip)); @@ -137,7 +141,7 @@ public class ComputationServiceTest { @Test public void fail_if_corrupted_zip() throws Exception { - AnalysisReportDto dto = AnalysisReportDto.newForTests(1L).setProjectKey("P1").setUuid("U1"); + AnalysisReportDto dto = newDefaultReport(); File zip = tempFolder.newFile(); FileUtils.write(zip, "not a file"); @@ -146,7 +150,7 @@ public class ComputationServiceTest { fail(); } catch (IllegalStateException e) { assertThat(e.getMessage()).startsWith("Fail to unzip " + zip.getAbsolutePath() + " into "); - assertThat(dto.getStatus()).isEqualTo(AnalysisReportDto.Status.FAILED); + assertThat(dto.getStatus()).isEqualTo(Status.FAILED); assertThat(dto.getFinishedAt()).isNotNull(); } } @@ -156,7 +160,7 @@ public class ComputationServiceTest { when(steps.orderedSteps()).thenReturn(Arrays.asList(projectStep1)); doThrow(new IllegalStateException("pb")).when(projectStep1).execute(any(ComputationContext.class)); - AnalysisReportDto dto = AnalysisReportDto.newForTests(1L).setProjectKey("P1").setUuid("U1"); + AnalysisReportDto dto = newDefaultReport(); File zip = generateZip(); try { @@ -164,11 +168,25 @@ public class ComputationServiceTest { fail(); } catch (IllegalStateException e) { assertThat(e.getMessage()).isEqualTo("pb"); - assertThat(dto.getStatus()).isEqualTo(AnalysisReportDto.Status.FAILED); + assertThat(dto.getStatus()).isEqualTo(Status.FAILED); assertThat(dto.getFinishedAt()).isNotNull(); } } + @Test + public void analysis_cancelled_when_snapshot_not_found() throws Exception { + AnalysisReportDto report = newDefaultReport(); + File zip = generateZip(ANY_SNAPSHOT_ID); + logTester.setLevel(LoggerLevel.DEBUG); + + sut.process(new ReportQueue.Item(report, zip)); + + assertThat(report.getStatus()).isEqualTo(Status.CANCELLED); + assertThat(logTester.logs()).contains( + String.format("Processing of report #%s is canceled because it was submitted while another report of the same project was already being processed.", report.getId())); + assertThat(logTester.logs()).contains(String.format("The snapshot ID #%s provided by the report #%s does not exist anymore.", ANY_SNAPSHOT_ID, report.getId())); + } + private ComputationStep mockStep(String... qualifiers) { ComputationStep step = mock(ComputationStep.class); when(step.supportedProjectQualifiers()).thenReturn(qualifiers); @@ -177,12 +195,17 @@ public class ComputationServiceTest { } private File generateZip() throws IOException { + return generateZip(110L); + } + + private File generateZip(long snapshotId) throws IOException { File dir = tempFolder.newDir(); BatchReportWriter writer = new BatchReportWriter(dir); writer.writeMetadata(BatchReport.Metadata.newBuilder() .setRootComponentRef(1) .setProjectKey("PROJECT_KEY") .setAnalysisDate(150000000L) + .setSnapshotId(snapshotId) .build()); File zip = tempFolder.newFile(); ZipUtils.zipDir(dir, zip); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/ReportQueueTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/ReportQueueTest.java index 6db7d0c9eb8..54d29d63e68 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/ReportQueueTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/ReportQueueTest.java @@ -40,7 +40,6 @@ import org.sonar.server.db.DbClient; import org.sonar.test.DbTests; import java.io.File; -import java.io.IOException; import java.io.InputStream; import java.util.List; @@ -56,7 +55,7 @@ public class ReportQueueTest { static final long NOW = 1_500_000_000_000L; @Rule - public DbTester dbTester = new DbTester(); + public DbTester db = new DbTester(); @Rule public TemporaryFolder temp = new TemporaryFolder(); @@ -72,7 +71,7 @@ public class ReportQueueTest { settings.setProperty(ProcessConstants.PATH_DATA, dataDir.getAbsolutePath()); when(system.now()).thenReturn(NOW); - DbClient dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), new ComponentDao(), new AnalysisReportDao(system)); + DbClient dbClient = new DbClient(db.database(), db.myBatis(), new ComponentDao(), new AnalysisReportDao(system)); sut = new ReportQueue(dbClient, settings); try (DbSession session = dbClient.openSession(false)) { @@ -84,7 +83,7 @@ public class ReportQueueTest { } @Test - public void add_report_to_queue() throws IOException { + public void add_report_to_queue() throws Exception { // must: // 1. insert metadata in db // 2. copy report content to directory /data/analysis @@ -95,7 +94,7 @@ public class ReportQueueTest { assertThat(item.dto.getUuid()).isNotEmpty(); assertThat(item.dto.getId()).isGreaterThan(0L); - List reports = sut.findByProjectKey("P1"); + List reports = sut.selectByProjectKey("P1"); assertThat(reports).hasSize(1); AnalysisReportDto report = reports.get(0); @@ -115,8 +114,8 @@ public class ReportQueueTest { @Test public void find_by_project_key() throws Exception { sut.add("P1", generateData()); - assertThat(sut.findByProjectKey("P1")).hasSize(1).extracting("projectKey").containsExactly("P1"); - assertThat(sut.findByProjectKey("P2")).isEmpty(); + assertThat(sut.selectByProjectKey("P1")).hasSize(1).extracting("projectKey").containsExactly("P1"); + assertThat(sut.selectByProjectKey("P2")).isEmpty(); } @Test @@ -147,10 +146,10 @@ public class ReportQueueTest { @Test public void remove() { ReportQueue.Item item = sut.add("P1", generateData()); - assertThat(dbTester.countRowsOfTable("analysis_reports")).isEqualTo(1); + assertThat(db.countRowsOfTable("analysis_reports")).isEqualTo(1); sut.remove(item); - assertThat(dbTester.countRowsOfTable("analysis_reports")).isEqualTo(0); + assertThat(db.countRowsOfTable("analysis_reports")).isEqualTo(0); assertThat(item.zipFile).doesNotExist(); } @@ -164,7 +163,7 @@ public class ReportQueueTest { assertThat(sut.pop()).isNull(); // table sanitized - assertThat(dbTester.countRowsOfTable("analysis_reports")).isEqualTo(0); + assertThat(db.countRowsOfTable("analysis_reports")).isEqualTo(0); } @Test @@ -175,10 +174,21 @@ public class ReportQueueTest { sut.clear(); - assertThat(dbTester.countRowsOfTable("analysis_reports")).isEqualTo(0); + assertThat(db.countRowsOfTable("analysis_reports")).isEqualTo(0); assertThat(analysisDir()).doesNotExist(); } + @Test + public void clear_do_not_fail_when_directory_do_not_exist() throws Exception { + sut.clear(); + sut.clear(); + } + + @Test(expected = IllegalStateException.class) + public void add_on_non_existent_project() throws Exception { + sut.add("UNKNOWN_PROJECT_KEY", generateData()); + } + @Test public void reset_to_pending_status() throws Exception { // 2 pending diff --git a/server/sonar-server/src/test/resources/org/sonar/server/computation/ComputationServiceTest/shared.xml b/server/sonar-server/src/test/resources/org/sonar/server/computation/ComputationServiceTest/shared.xml index 66bd86524aa..bdf91f3f145 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/computation/ComputationServiceTest/shared.xml +++ b/server/sonar-server/src/test/resources/org/sonar/server/computation/ComputationServiceTest/shared.xml @@ -1,3 +1,11 @@ + diff --git a/sonar-core/src/main/java/org/sonar/core/computation/db/AnalysisReportDto.java b/sonar-core/src/main/java/org/sonar/core/computation/db/AnalysisReportDto.java index d5cfc718ccf..d290959407e 100644 --- a/sonar-core/src/main/java/org/sonar/core/computation/db/AnalysisReportDto.java +++ b/sonar-core/src/main/java/org/sonar/core/computation/db/AnalysisReportDto.java @@ -42,10 +42,6 @@ public class AnalysisReportDto { return report; } - public void setId(Long id) { - this.id = id; - } - public String getProjectKey() { return projectKey; } @@ -64,14 +60,6 @@ public class AnalysisReportDto { return this; } - public void fail() { - this.status = Status.FAILED; - } - - public void succeed() { - this.status = Status.SUCCESS; - } - public String getUuid() { return uuid; } @@ -85,6 +73,10 @@ public class AnalysisReportDto { return id; } + public void setId(Long id) { + this.id = id; + } + @Override public String toString() { return Objects.toStringHelper(this) @@ -137,10 +129,6 @@ public class AnalysisReportDto { } public static enum Status { - PENDING, WORKING, SUCCESS, FAILED; - - public boolean isInFinalState() { - return SUCCESS.equals(this) || FAILED.equals(this); - } + PENDING, WORKING, SUCCESS, FAILED, CANCELLED } }