From c3670e8b01b59617e7077be186147a4ff32d32b4 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Tue, 3 May 2016 18:50:10 +0200 Subject: [PATCH] SONAR-7387 Log flooding in web service api/tests/covered_files --- .../sonar/server/test/index/TestIndex.java | 21 +++++--- .../server/test/ws/CoveredFilesAction.java | 13 +++-- .../test/ws/CoveredFilesActionTest.java | 49 ++++++++++++++----- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/test/index/TestIndex.java b/server/sonar-server/src/main/java/org/sonar/server/test/index/TestIndex.java index b1f7793ad26..5dbbb8bd7c9 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/test/index/TestIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/test/index/TestIndex.java @@ -20,6 +20,10 @@ package org.sonar.server.test.index; import com.google.common.base.Function; +import com.google.common.base.Optional; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.index.query.FilterBuilders; import org.elasticsearch.index.query.QueryBuilders; @@ -30,10 +34,6 @@ import org.sonar.server.es.EsClient; import org.sonar.server.es.SearchOptions; import org.sonar.server.es.SearchResult; -import java.util.ArrayList; -import java.util.List; -import java.util.Map; - import static org.sonar.server.test.index.TestIndexDefinition.FIELD_COVERED_FILES; import static org.sonar.server.test.index.TestIndexDefinition.FIELD_COVERED_FILE_LINES; import static org.sonar.server.test.index.TestIndexDefinition.FIELD_COVERED_FILE_UUID; @@ -89,15 +89,24 @@ public class TestIndex extends BaseIndex { } public TestDoc searchByTestUuid(String testUuid) { + Optional testDoc = getNullableByTestUuid(testUuid); + if (testDoc.isPresent()) { + return testDoc.get(); + } + + throw new IllegalStateException(String.format("Test id '%s' not found", testUuid)); + } + + public Optional getNullableByTestUuid(String testUuid) { for (SearchHit hit : getClient().prepareSearch(TestIndexDefinition.INDEX) .setTypes(TestIndexDefinition.TYPE) .setSize(1) .setQuery(QueryBuilders.filteredQuery(QueryBuilders.matchAllQuery(), FilterBuilders.termFilter(FIELD_TEST_UUID, testUuid))) .get().getHits().getHits()) { - return new TestDoc(hit.sourceAsMap()); + return Optional.of(new TestDoc(hit.sourceAsMap())); } - throw new IllegalStateException(String.format("Test id '%s' not found", testUuid)); + return Optional.absent(); } public SearchResult searchByTestUuid(String testUuid, SearchOptions searchOptions) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/test/ws/CoveredFilesAction.java b/server/sonar-server/src/main/java/org/sonar/server/test/ws/CoveredFilesAction.java index f8a51f4cb00..406deaa8659 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/test/ws/CoveredFilesAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/test/ws/CoveredFilesAction.java @@ -33,15 +33,17 @@ import org.sonar.api.web.UserRole; import org.sonar.core.util.Uuids; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonar.db.MyBatis; import org.sonar.db.component.ComponentDto; import org.sonar.db.component.ComponentDtoFunctions; import org.sonar.server.test.index.CoveredFileDoc; +import org.sonar.server.test.index.TestDoc; import org.sonar.server.test.index.TestIndex; import org.sonar.server.user.UserSession; -import org.sonar.server.ws.WsUtils; import org.sonarqube.ws.WsTests; +import static org.sonar.server.ws.WsUtils.checkFoundWithOptional; +import static org.sonar.server.ws.WsUtils.writeProtobuf; + public class CoveredFilesAction implements TestsWsAction { public static final String TEST_ID = "testId"; @@ -75,7 +77,8 @@ public class CoveredFilesAction implements TestsWsAction { @Override public void handle(Request request, Response response) throws Exception { String testId = request.mandatoryParam(TEST_ID); - userSession.checkComponentUuidPermission(UserRole.CODEVIEWER, index.searchByTestUuid(testId).fileUuid()); + TestDoc testDoc = checkFoundWithOptional(index.getNullableByTestUuid(testId), "Test with id '%s' is not found", testId); + userSession.checkComponentUuidPermission(UserRole.CODEVIEWER, testDoc.fileUuid()); List coveredFiles = index.coveredFiles(testId); Map componentsByUuid = buildComponentsByUuid(coveredFiles); @@ -95,7 +98,7 @@ public class CoveredFilesAction implements TestsWsAction { responseBuilder.addFiles(fileBuilder); } } - WsUtils.writeProtobuf(responseBuilder.build(), request, response); + writeProtobuf(responseBuilder.build(), request, response); } private Map buildComponentsByUuid(List coveredFiles) { @@ -105,7 +108,7 @@ public class CoveredFilesAction implements TestsWsAction { try { components = dbClient.componentDao().selectByUuids(dbSession, sourceFileUuids); } finally { - MyBatis.closeQuietly(dbSession); + dbClient.closeSession(dbSession); } return Maps.uniqueIndex(components, ComponentDtoFunctions.toUuid()); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/test/ws/CoveredFilesActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/test/ws/CoveredFilesActionTest.java index 6fc1ea7d66d..e422141a965 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/test/ws/CoveredFilesActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/test/ws/CoveredFilesActionTest.java @@ -19,17 +19,22 @@ */ package org.sonar.server.test.ws; +import com.google.common.base.Optional; import java.util.Arrays; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.sonar.api.web.UserRole; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.test.index.CoveredFileDoc; +import org.sonar.server.test.index.TestDoc; import org.sonar.server.test.index.TestIndex; import org.sonar.server.tester.UserSessionRule; -import org.sonar.server.ws.WsTester; +import org.sonar.server.ws.TestRequest; +import org.sonar.server.ws.WsActionTester; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyList; @@ -40,6 +45,7 @@ import static org.mockito.Mockito.when; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.component.ComponentTesting.newProjectDto; import static org.sonar.server.test.ws.CoveredFilesAction.TEST_ID; +import static org.sonar.test.JsonAssert.assertJson; public class CoveredFilesActionTest { @@ -48,34 +54,55 @@ public class CoveredFilesActionTest { @Rule public UserSessionRule userSessionRule = UserSessionRule.standalone(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); - WsTester ws; - private DbClient dbClient; - private TestIndex testIndex; + WsActionTester ws; + DbClient dbClient; + TestIndex testIndex; @Before public void setUp() { dbClient = mock(DbClient.class, RETURNS_DEEP_STUBS); testIndex = mock(TestIndex.class, RETURNS_DEEP_STUBS); - ws = new WsTester(new TestsWs(new CoveredFilesAction(dbClient, testIndex, userSessionRule))); + + ws = new WsActionTester(new CoveredFilesAction(dbClient, testIndex, userSessionRule)); + } + + @Test + public void covered_files() { + userSessionRule.addComponentUuidPermission(UserRole.CODEVIEWER, "SonarQube", "test-file-uuid"); + + when(testIndex.getNullableByTestUuid(anyString())).thenReturn(Optional.of(new TestDoc().setFileUuid("test-file-uuid"))); + when(testIndex.coveredFiles("test-uuid")).thenReturn(Arrays.asList( + new CoveredFileDoc().setFileUuid(FILE_1_ID).setCoveredLines(Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)), + new CoveredFileDoc().setFileUuid(FILE_2_ID).setCoveredLines(Arrays.asList(1, 2, 3)))); + when(dbClient.componentDao().selectByUuids(any(DbSession.class), anyList())).thenReturn( + Arrays.asList( + newFileDto(newProjectDto(), FILE_1_ID).setKey("org.foo.Bar.java").setLongName("src/main/java/org/foo/Bar.java"), + newFileDto(newProjectDto(), FILE_2_ID).setKey("org.foo.File.java").setLongName("src/main/java/org/foo/File.java"))); + + TestRequest request = ws.newRequest().setParam(TEST_ID, "test-uuid"); + + assertJson(request.execute().getInput()).isSimilarTo(getClass().getResource("CoveredFilesActionTest/tests-covered-files.json")); } @Test - public void covered_files() throws Exception { + public void fail_when_test_uuid_is_unknown() { userSessionRule.addComponentUuidPermission(UserRole.CODEVIEWER, "SonarQube", "test-file-uuid"); - when(testIndex.searchByTestUuid(anyString()).fileUuid()).thenReturn("test-file-uuid"); + when(testIndex.getNullableByTestUuid(anyString())).thenReturn(Optional.absent()); when(testIndex.coveredFiles("test-uuid")).thenReturn(Arrays.asList( new CoveredFileDoc().setFileUuid(FILE_1_ID).setCoveredLines(Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9, 10)), - new CoveredFileDoc().setFileUuid(FILE_2_ID).setCoveredLines(Arrays.asList(1, 2, 3)) - )); + new CoveredFileDoc().setFileUuid(FILE_2_ID).setCoveredLines(Arrays.asList(1, 2, 3)))); when(dbClient.componentDao().selectByUuids(any(DbSession.class), anyList())).thenReturn( Arrays.asList( newFileDto(newProjectDto(), FILE_1_ID).setKey("org.foo.Bar.java").setLongName("src/main/java/org/foo/Bar.java"), newFileDto(newProjectDto(), FILE_2_ID).setKey("org.foo.File.java").setLongName("src/main/java/org/foo/File.java"))); - WsTester.TestRequest request = ws.newGetRequest("api/tests", "covered_files").setParam(TEST_ID, "test-uuid"); + expectedException.expect(NotFoundException.class); + expectedException.expectMessage("Test with id 'test-uuid' is not found"); - request.execute().assertJson(getClass(), "tests-covered-files.json"); + ws.newRequest().setParam(TEST_ID, "test-uuid").execute(); } } -- 2.39.5