From c99a322d758137d3bc95df81b1c83a4e336076b0 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 1 Mar 2017 10:26:53 +0100 Subject: SONAR-8659 Bring back "Edit Permissions" to system administrators --- .../org/sonar/server/ui/ws/ComponentAction.java | 4 +- .../sonar/server/ui/ws/ComponentActionTest.java | 119 +++++++++++++-------- 2 files changed, 76 insertions(+), 47 deletions(-) (limited to 'server') diff --git a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/ComponentAction.java b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/ComponentAction.java index dd8839b82d5..4a525a30c6a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/ui/ws/ComponentAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/ui/ws/ComponentAction.java @@ -130,7 +130,9 @@ public class ComponentAction implements NavigationWsAction { String componentKey = request.mandatoryParam(PARAM_COMPONENT_KEY); try (DbSession session = dbClient.openSession(false)) { ComponentDto component = componentFinder.getByKey(session, componentKey); - if (!(userSession.hasComponentPermission(USER, component) || userSession.hasComponentPermission(ADMIN, component))) { + if (!userSession.hasComponentPermission(USER, component) && + !userSession.hasComponentPermission(ADMIN, component) && + !userSession.isSystemAdministrator()) { throw insufficientPrivilegesException(); } OrganizationDto org = componentFinder.getOrganization(session, component); diff --git a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java index 0a293bc8974..717adbd0279 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/ui/ws/ComponentActionTest.java @@ -47,7 +47,6 @@ import org.sonar.db.organization.OrganizationDto; import org.sonar.db.property.PropertyDbTester; import org.sonar.db.property.PropertyDto; import org.sonar.db.qualitygate.QualityGateDto; -import org.sonar.db.user.UserDbTester; import org.sonar.db.user.UserDto; import org.sonar.server.component.ComponentFinder; import org.sonar.server.exceptions.ForbiddenException; @@ -78,42 +77,28 @@ import static org.sonar.test.JsonAssert.assertJson; public class ComponentActionTest { - private static final String PROJECT_KEY = "polop"; - @Rule public ExpectedException expectedException = ExpectedException.none(); @Rule public DbTester dbTester = DbTester.create(System2.INSTANCE); @Rule - public UserSessionRule userSessionRule = UserSessionRule.standalone(); + public UserSessionRule userSession = UserSessionRule.standalone(); private DbClient dbClient = dbTester.getDbClient(); private ComponentDbTester componentDbTester = dbTester.components(); - private UserDbTester userDbTester = dbTester.users(); private PropertyDbTester propertyDbTester = new PropertyDbTester(dbTester); - private ResourceTypes resourceTypes = mock(ResourceTypes.class); - private ComponentDto project; private WsActionTester ws; - private static QualityProfile createQProfile(String qpKey, String qpName, String languageKey) { - return new QualityProfile(qpKey, qpName, languageKey, new Date()); - } - - private static String qualityProfilesToJson(QualityProfile... qps) { - List qualityProfiles = Arrays.asList(qps); - return QPMeasureData.toJson(new QPMeasureData(qualityProfiles)); - } - @Before public void before() { OrganizationDto organization = dbTester.organizations().insertForKey("my-org"); - project = newProjectDto(organization, "abcd") - .setKey(PROJECT_KEY) - .setName("Polop") - .setDescription("test project") - .setLanguage("xoo"); + project = newProjectDto(organization, "abcd") + .setKey("polop") + .setName("Polop") + .setDescription("test project") + .setLanguage("xoo"); } @Test @@ -133,19 +118,47 @@ public class ComponentActionTest { } @Test - public void fail_on_missing_permission() throws Exception { + public void throw_ForbiddenException_if_required_permission_is_not_granted() throws Exception { init(); componentDbTester.insertComponent(project); + userSession.logIn(); expectedException.expect(ForbiddenException.class); execute(project.key()); } + @Test + public void return_info_if_user_has_browse_permission_on_project() throws Exception { + init(); + componentDbTester.insertComponent(project); + userSession.logIn().addProjectUuidPermissions(UserRole.USER, project.uuid()); + + verifySuccess(project.key()); + } + + @Test + public void return_info_if_user_has_administration_permission_on_project() throws Exception { + init(); + componentDbTester.insertComponent(project); + userSession.logIn().addProjectUuidPermissions(UserRole.ADMIN, project.uuid()); + + verifySuccess(project.key()); + } + + @Test + public void return_info_if_user_is_system_administrator() throws Exception { + init(); + componentDbTester.insertComponent(project); + userSession.logIn().setSystemAdministrator(); + + verifySuccess(project.key()); + } + @Test public void return_component_info_when_anonymous_no_snapshot() throws Exception { init(); componentDbTester.insertComponent(project); - userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(project.key(), "return_component_info_when_anonymous_no_snapshot.json"); } @@ -153,10 +166,10 @@ public class ComponentActionTest { @Test public void return_component_info_with_favourite() throws Exception { init(); - UserDto user = userDbTester.insertUser("obiwan"); + UserDto user = dbTester.users().insertUser("obiwan"); componentDbTester.insertComponent(project); propertyDbTester.insertProperty(new PropertyDto().setKey("favourite").setResourceId(project.getId()).setUserId(user.getId())); - userSessionRule.logIn(user).addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.logIn(user).addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(project.key(), "return_component_info_with_favourite.json"); } @@ -169,7 +182,7 @@ public class ComponentActionTest { .setCreatedAt(DateUtils.parseDateTime("2015-04-22T11:44:00+0200").getTime()) .setVersion("3.14") .setLast(true)); - userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(project.key(), "return_component_info_when_snapshot.json"); } @@ -182,7 +195,7 @@ public class ComponentActionTest { addQualityProfiles(project, analysis, createQProfile("qp1", "Sonar Way Java", "java"), createQProfile("qp2", "Sonar Way Xoo", "xoo")); - userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(project.key(), "return_quality_profiles.json"); } @@ -191,7 +204,7 @@ public class ComponentActionTest { public void return_empty_quality_profiles_when_no_measure() throws Exception { init(); componentDbTester.insertComponent(project); - userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(project.key(), "return_empty_quality_profiles_when_no_measure.json"); } @@ -202,7 +215,7 @@ public class ComponentActionTest { componentDbTester.insertComponent(project); QualityGateDto qualityGateDto = dbTester.qualityGates().insertQualityGate("Sonar way"); dbTester.qualityGates().associateProjectToQualityGate(project, qualityGateDto); - userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(project.key(), "return_quality_gate.json"); } @@ -212,7 +225,7 @@ public class ComponentActionTest { init(); componentDbTester.insertComponent(project); dbTester.qualityGates().createDefaultQualityGate("Sonar way"); - userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(project.key(), "return_default_quality_gate.json"); } @@ -221,7 +234,7 @@ public class ComponentActionTest { public void return_no_quality_gate_when_not_defined_on_project_and_no_default_one() throws Exception { init(); componentDbTester.insertComponent(project); - userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.USER, project.uuid()); String json = execute(project.key()); assertThat(json).doesNotContain("qualityGate"); @@ -231,7 +244,7 @@ public class ComponentActionTest { public void return_extensions() throws Exception { init(createPages()); componentDbTester.insertProjectAndSnapshot(project); - userSessionRule.anonymous().addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.anonymous().addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(project.key(), "return_extensions.json"); } @@ -240,7 +253,7 @@ public class ComponentActionTest { public void return_extensions_for_admin() throws Exception { init(createPages()); componentDbTester.insertProjectAndSnapshot(project); - userSessionRule.anonymous() + userSession.anonymous() .addProjectUuidPermissions(UserRole.USER, project.uuid()) .addProjectUuidPermissions(UserRole.ADMIN, project.uuid()); @@ -249,9 +262,9 @@ public class ComponentActionTest { @Test public void return_configuration_for_admin() throws Exception { - UserDto user = userDbTester.insertUser(); + UserDto user = dbTester.users().insertUser(); componentDbTester.insertComponent(project); - userSessionRule.logIn(user) + userSession.logIn(user) .addProjectUuidPermissions(UserRole.USER, "abcd") .addProjectUuidPermissions(UserRole.ADMIN, "abcd"); @@ -276,7 +289,7 @@ public class ComponentActionTest { public void return_configuration_with_all_properties() throws Exception { init(); componentDbTester.insertComponent(project); - userSessionRule.anonymous() + userSession.anonymous() .addProjectUuidPermissions(UserRole.USER, "abcd") .addProjectUuidPermissions(UserRole.ADMIN, "abcd"); @@ -299,7 +312,7 @@ public class ComponentActionTest { init(); ComponentDto project = componentDbTester.insertComponent(this.project); ComponentDto module = componentDbTester.insertComponent(newModuleDto("bcde", project).setKey("palap").setName("Palap")); - userSessionRule.anonymous() + userSession.anonymous() .addProjectUuidPermissions(UserRole.USER, "abcd") .addProjectUuidPermissions(UserRole.ADMIN, "abcd"); @@ -310,7 +323,7 @@ public class ComponentActionTest { public void return_configuration_for_quality_profile_admin() throws Exception { init(); componentDbTester.insertComponent(project); - userSessionRule.logIn() + userSession.logIn() .addProjectUuidPermissions(UserRole.USER, project.uuid()) .addOrganizationPermission(project.getOrganizationUuid(), QUALITY_PROFILE_ADMIN); @@ -321,7 +334,7 @@ public class ComponentActionTest { public void return_configuration_for_quality_gate_admin() throws Exception { init(); componentDbTester.insertComponent(project); - userSessionRule.logIn() + userSession.logIn() .addProjectUuidPermissions(UserRole.USER, project.uuid()) .addOrganizationPermission(project.getOrganizationUuid(), QUALITY_GATE_ADMIN); @@ -337,7 +350,7 @@ public class ComponentActionTest { ComponentDto file = componentDbTester.insertComponent(newFileDto(directory, directory, "cdef").setName("Source.xoo") .setKey("palap:src/main/xoo/Source.xoo") .setPath(directory.path())); - userSessionRule.addProjectUuidPermissions(UserRole.USER, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.USER, project.uuid()); executeAndVerify(file.key(), "return_bread_crumbs_on_several_levels.json"); } @@ -346,7 +359,7 @@ public class ComponentActionTest { public void project_administrator_is_allowed_to_get_information() throws Exception { init(createPages()); componentDbTester.insertProjectAndSnapshot(project); - userSessionRule.addProjectUuidPermissions(UserRole.ADMIN, project.uuid()); + userSession.addProjectUuidPermissions(UserRole.ADMIN, project.uuid()); execute(project.key()); } @@ -366,14 +379,14 @@ public class ComponentActionTest { .setLast(true); componentDbTester.insertSnapshot(analysis); when(resourceTypes.get(project.qualifier())).thenReturn(DefaultResourceTypes.get().getRootType()); - UserDto user = userDbTester.insertUser("obiwan"); + UserDto user = dbTester.users().insertUser("obiwan"); propertyDbTester.insertProperty(new PropertyDto().setKey("favourite").setResourceId(project.getId()).setUserId(user.getId())); addQualityProfiles(project, analysis, createQProfile("qp1", "Sonar Way Java", "java"), createQProfile("qp2", "Sonar Way Xoo", "xoo")); QualityGateDto qualityGateDto = dbTester.qualityGates().insertQualityGate("Sonar way"); dbTester.qualityGates().associateProjectToQualityGate(project, qualityGateDto); - userSessionRule.logIn(user) + userSession.logIn(user) .addProjectUuidPermissions(UserRole.USER, project.uuid()) .addProjectUuidPermissions(UserRole.ADMIN, project.uuid()); @@ -387,12 +400,12 @@ public class ComponentActionTest { OrganizationDto org = dbTester.organizations().insert(); ComponentDto project = dbTester.components().insertProject(org); - userSessionRule.logIn() + userSession.logIn() .addProjectUuidPermissions(UserRole.ADMIN, project.uuid()) .addOrganizationPermission(org.getUuid(), GlobalPermissions.SYSTEM_ADMIN); assertJson(execute(project.key())).isSimilarTo("{\"configuration\": {\"canApplyPermissionTemplate\": true}}"); - userSessionRule.logIn() + userSession.logIn() .addProjectUuidPermissions(UserRole.ADMIN, project.uuid()); assertJson(execute(project.key())).isSimilarTo("{\"configuration\": {\"canApplyPermissionTemplate\": false}}"); @@ -408,7 +421,7 @@ public class ComponentActionTest { }}); pageRepository.start(); ws = new WsActionTester( - new ComponentAction(dbClient, pageRepository, resourceTypes, userSessionRule, new ComponentFinder(dbClient), + new ComponentAction(dbClient, pageRepository, resourceTypes, userSession, new ComponentFinder(dbClient), new QualityGateFinder(dbClient))); } @@ -453,4 +466,18 @@ public class ComponentActionTest { return new Page[] {page1, page2, adminPage}; } + + private void verifySuccess(String componentKey) { + String json = execute(componentKey); + assertJson(json).isSimilarTo("{\"key\":\"" + componentKey + "\"}"); + } + + private static QualityProfile createQProfile(String qpKey, String qpName, String languageKey) { + return new QualityProfile(qpKey, qpName, languageKey, new Date()); + } + + private static String qualityProfilesToJson(QualityProfile... qps) { + List qualityProfiles = Arrays.asList(qps); + return QPMeasureData.toJson(new QPMeasureData(qualityProfiles)); + } } -- cgit v1.2.3 From f2647505fb3cb4bd76a7bdad538beb8f35baece1 Mon Sep 17 00:00:00 2001 From: Stas Vilchik Date: Wed, 1 Mar 2017 13:23:55 +0100 Subject: SONAR-8451 fix bad state on measures page --- server/sonar-web/src/main/js/apps/component-measures/app/App.js | 7 ++++--- .../sonar-web/src/main/js/apps/component-measures/home/actions.js | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) (limited to 'server') diff --git a/server/sonar-web/src/main/js/apps/component-measures/app/App.js b/server/sonar-web/src/main/js/apps/component-measures/app/App.js index f60a5aa0798..2381ba36011 100644 --- a/server/sonar-web/src/main/js/apps/component-measures/app/App.js +++ b/server/sonar-web/src/main/js/apps/component-measures/app/App.js @@ -21,15 +21,16 @@ import React from 'react'; import Spinner from './../components/Spinner'; export default class App extends React.Component { + state = { componentSet: false }; + componentDidMount () { this.props.setComponent(this.props.component); this.props.fetchMetrics(); + this.setState({ componentSet: true }); } render () { - const { metrics } = this.props; - - if (metrics == null) { + if (this.props.metrics == null || !this.state.componentSet) { return ; } diff --git a/server/sonar-web/src/main/js/apps/component-measures/home/actions.js b/server/sonar-web/src/main/js/apps/component-measures/home/actions.js index b516d2e7bbb..a44cd01f8a9 100644 --- a/server/sonar-web/src/main/js/apps/component-measures/home/actions.js +++ b/server/sonar-web/src/main/js/apps/component-measures/home/actions.js @@ -41,6 +41,8 @@ export function fetchMeasures () { dispatch(startFetching()); const state = getState(); + /* eslint-disable no-console */ + console.log(state); const component = getMeasuresAppComponent(state); const metrics = getMeasuresAppAllMetrics(state); -- cgit v1.2.3 From 187e582f89cc4ba48e7078abe16e8869e69b9e36 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 1 Mar 2017 15:24:02 +0100 Subject: SONAR-8835 file move detection does not need to compute source hash --- .../task/projectanalysis/filemove/FileMoveDetectionStep.java | 7 ++----- .../task/projectanalysis/filemove/FileSimilarity.java | 9 +-------- .../task/projectanalysis/filemove/FileMoveDetectionStepTest.java | 6 +----- .../task/projectanalysis/filemove/MatchesByScoreTest.java | 2 +- 4 files changed, 5 insertions(+), 19 deletions(-) (limited to 'server') diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStep.java index 1a53493123a..182e4232455 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStep.java @@ -37,7 +37,6 @@ import javax.annotation.concurrent.Immutable; import org.sonar.api.resources.Qualifiers; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; -import org.sonar.core.hash.SourceHashComputer; import org.sonar.core.hash.SourceLinesHashesComputer; import org.sonar.core.util.CloseableIterator; import org.sonar.db.DbClient; @@ -185,15 +184,13 @@ public class FileMoveDetectionStep implements ComputationStep { // SourceHashRepository Component component = reportFilesByKey.get(fileKey); SourceLinesHashesComputer linesHashesComputer = new SourceLinesHashesComputer(); - SourceHashComputer sourceHashComputer = new SourceHashComputer(); try (CloseableIterator lineIterator = sourceLinesRepository.readLines(component)) { while (lineIterator.hasNext()) { String line = lineIterator.next(); linesHashesComputer.addLine(line); - sourceHashComputer.addLine(line, lineIterator.hasNext()); } } - builder.put(fileKey, new File(component.getReportAttributes().getPath(), sourceHashComputer.getHash(), linesHashesComputer.getLineHashes())); + builder.put(fileKey, new File(component.getReportAttributes().getPath(), linesHashesComputer.getLineHashes())); } return builder.build(); } @@ -236,7 +233,7 @@ public class FileMoveDetectionStep implements ComputationStep { if (fileSourceDto == null) { return null; } - return new File(dbComponent.getPath(), fileSourceDto.getSrcHash(), LINES_HASHES_SPLITTER.splitToList(fileSourceDto.getLineHashes())); + return new File(dbComponent.getPath(), LINES_HASHES_SPLITTER.splitToList(fileSourceDto.getLineHashes())); } private static void printIfDebug(ScoreMatrix scoreMatrix) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarity.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarity.java index d234f35292c..86743b06906 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarity.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarity.java @@ -29,12 +29,10 @@ public interface FileSimilarity { final class File { private final String path; - private final String srcHash; private final List lineHashes; - public File(String path, @Nullable String srcHash, @Nullable List lineHashes) { + public File(String path, @Nullable List lineHashes) { this.path = requireNonNull(path, "path can not be null"); - this.srcHash = srcHash; this.lineHashes = lineHashes; } @@ -42,11 +40,6 @@ public interface FileSimilarity { return path; } - @CheckForNull - public String getSrcHash() { - return srcHash; - } - @CheckForNull public List getLineHashes() { return lineHashes; diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStepTest.java index f6fd10c8e01..6f67fd94847 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStepTest.java @@ -35,7 +35,6 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.mockito.ArgumentCaptor; -import org.sonar.core.hash.SourceHashComputer; import org.sonar.core.hash.SourceLinesHashesComputer; import org.sonar.db.DbClient; import org.sonar.db.DbSession; @@ -499,18 +498,15 @@ public class FileMoveDetectionStepTest { private void mockContentOfFileInDb(String key, String[] content) { SourceLinesHashesComputer linesHashesComputer = new SourceLinesHashesComputer(); - SourceHashComputer sourceHashComputer = new SourceHashComputer(); Iterator lineIterator = Arrays.asList(content).iterator(); while (lineIterator.hasNext()) { String line = lineIterator.next(); linesHashesComputer.addLine(line); - sourceHashComputer.addLine(line, lineIterator.hasNext()); } when(fileSourceDao.selectSourceByFileUuid(dbSession, componentUuidOf(key))) .thenReturn(new FileSourceDto() - .setLineHashes(on('\n').join(linesHashesComputer.getLineHashes())) - .setSrcHash(sourceHashComputer.getHash())); + .setLineHashes(on('\n').join(linesHashesComputer.getLineHashes()))); } private void setFilesInReport(Component... files) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MatchesByScoreTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MatchesByScoreTest.java index 1f2644a69c3..cbd96aac878 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MatchesByScoreTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MatchesByScoreTest.java @@ -78,6 +78,6 @@ public class MatchesByScoreTest { } private static FileSimilarity.File fileOf(String key) { - return new FileSimilarity.File("path of " + key, null, null); + return new FileSimilarity.File("path of " + key, null); } } -- cgit v1.2.3 From 72ac5447a32d6537e805374116cbf369f7a65038 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 1 Mar 2017 15:33:22 +0100 Subject: SONAR-8835 support NULL in column file_sources.line_hashes The column can be null on Oracle if source content is empty. Other databases do not store NULL but empty string. --- .../filemove/FileMoveDetectionStep.java | 4 ++- .../projectanalysis/filemove/FileSimilarity.java | 11 ++++---- .../filemove/FileSimilarityImpl.java | 15 +++-------- .../filemove/SourceSimilarityImpl.java | 7 +++-- .../filemove/FileMoveDetectionStepTest.java | 31 ++++++++++++++-------- .../filemove/MatchesByScoreTest.java | 6 +++-- .../filemove/SourceSimilarityImplTest.java | 8 +++++- 7 files changed, 49 insertions(+), 33 deletions(-) (limited to 'server') diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStep.java index 182e4232455..df6c144f05f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStep.java @@ -56,6 +56,7 @@ import org.sonar.server.computation.task.projectanalysis.filemove.FileSimilarity import org.sonar.server.computation.task.projectanalysis.source.SourceLinesRepository; import org.sonar.server.computation.task.step.ComputationStep; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.base.Splitter.on; import static com.google.common.collect.FluentIterable.from; import static java.util.Arrays.asList; @@ -233,7 +234,8 @@ public class FileMoveDetectionStep implements ComputationStep { if (fileSourceDto == null) { return null; } - return new File(dbComponent.getPath(), LINES_HASHES_SPLITTER.splitToList(fileSourceDto.getLineHashes())); + String lineHashes = firstNonNull(fileSourceDto.getLineHashes(), ""); + return new File(dbComponent.getPath(), LINES_HASHES_SPLITTER.splitToList(lineHashes)); } private static void printIfDebug(ScoreMatrix scoreMatrix) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarity.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarity.java index 86743b06906..7f3547b904e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarity.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarity.java @@ -20,8 +20,6 @@ package org.sonar.server.computation.task.projectanalysis.filemove; import java.util.List; -import javax.annotation.CheckForNull; -import javax.annotation.Nullable; import static java.util.Objects.requireNonNull; @@ -31,16 +29,19 @@ public interface FileSimilarity { private final String path; private final List lineHashes; - public File(String path, @Nullable List lineHashes) { + public File(String path, List lineHashes) { this.path = requireNonNull(path, "path can not be null"); - this.lineHashes = lineHashes; + this.lineHashes = requireNonNull(lineHashes, "lineHashes can not be null"); } public String getPath() { return path; } - @CheckForNull + /** + * List of hash of each line. An empty list is returned + * if file content is empty. + */ public List getLineHashes() { return lineHashes; } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarityImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarityImpl.java index c34bf8c1564..d059c0ac182 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarityImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/FileSimilarityImpl.java @@ -19,8 +19,6 @@ */ package org.sonar.server.computation.task.projectanalysis.filemove; -import java.util.List; - public class FileSimilarityImpl implements FileSimilarity { private final SourceSimilarity sourceSimilarity; @@ -31,15 +29,10 @@ public class FileSimilarityImpl implements FileSimilarity { @Override public int score(File file1, File file2) { - int score = 0; - - // TODO check filenames + // Algorithm could be improved by increasing score + // depending on filename similarity + // Current implementation relies on file content only. - List lineHashes1 = file1.getLineHashes(); - List lineHashes2 = file2.getLineHashes(); - if (lineHashes1 != null && lineHashes2 != null) { - score += sourceSimilarity.score(lineHashes1, lineHashes2); - } - return score; + return sourceSimilarity.score(file1.getLineHashes(), file2.getLineHashes()); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/SourceSimilarityImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/SourceSimilarityImpl.java index bb9da71e5ef..bdbc91d3b25 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/SourceSimilarityImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/filemove/SourceSimilarityImpl.java @@ -27,12 +27,15 @@ import static java.lang.Math.min; public class SourceSimilarityImpl implements SourceSimilarity { @Override - public int score(List left, List right) { + public int score(List left, List right) { + if (left.isEmpty() && right.isEmpty()) { + return 0; + } int distance = levenshteinDistance(left, right); return (int) (100 * (1.0 - ((double) distance) / (max(left.size(), right.size())))); } - int levenshteinDistance(List left, List right) { + private static int levenshteinDistance(List left, List right) { int len0 = left.size() + 1; int len1 = right.size() + 1; diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStepTest.java index 6f67fd94847..68af5bc8837 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/FileMoveDetectionStepTest.java @@ -23,13 +23,12 @@ import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.function.Function; +import javax.annotation.Nullable; import org.apache.commons.io.FileUtils; import org.junit.Before; import org.junit.Rule; @@ -409,6 +408,18 @@ public class FileMoveDetectionStepTest { assertThat(movedFilesRepository.getComponentsWithOriginal()).isEmpty(); } + @Test + public void execute_detects_no_move_if_two_files_are_empty() { + analysisMetadataHolder.setBaseAnalysis(ANALYSIS); + mockComponents(FILE_1.getKey(), FILE_2.getKey()); + mockContentOfFileInDb(FILE_1.getKey(), null); + mockContentOfFileInDb(FILE_2.getKey(), null); + + underTest.execute(); + + assertThat(movedFilesRepository.getComponentsWithOriginal()).isEmpty(); + } + @Test public void execute_detects_several_moves() { // testing: @@ -496,17 +507,15 @@ public class FileMoveDetectionStepTest { sourceLinesRepository.addLines(ref, content); } - private void mockContentOfFileInDb(String key, String[] content) { - SourceLinesHashesComputer linesHashesComputer = new SourceLinesHashesComputer(); - Iterator lineIterator = Arrays.asList(content).iterator(); - while (lineIterator.hasNext()) { - String line = lineIterator.next(); - linesHashesComputer.addLine(line); + private void mockContentOfFileInDb(String key, @Nullable String[] content) { + FileSourceDto dto = new FileSourceDto(); + if (content != null) { + SourceLinesHashesComputer linesHashesComputer = new SourceLinesHashesComputer(); + stream(content).forEach(linesHashesComputer::addLine); + dto.setLineHashes(on('\n').join(linesHashesComputer.getLineHashes())); } - when(fileSourceDao.selectSourceByFileUuid(dbSession, componentUuidOf(key))) - .thenReturn(new FileSourceDto() - .setLineHashes(on('\n').join(linesHashesComputer.getLineHashes()))); + when(fileSourceDao.selectSourceByFileUuid(dbSession, componentUuidOf(key))).thenReturn(dto); } private void setFilesInReport(Component... files) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MatchesByScoreTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MatchesByScoreTest.java index cbd96aac878..4007a357a7c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MatchesByScoreTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/MatchesByScoreTest.java @@ -30,6 +30,8 @@ import java.util.Set; import org.junit.Test; import static com.google.common.collect.ImmutableSet.of; +import static java.util.Collections.emptyList; +import static java.util.Collections.emptySet; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.server.computation.task.projectanalysis.filemove.FileMoveDetectionStep.MIN_REQUIRED_SCORE; @@ -39,7 +41,7 @@ public class MatchesByScoreTest { @Test public void creates_returns_always_the_same_instance_of_maxScore_is_less_than_min_required_score() { - Set doesNotMatterDbFileKeys = Collections.emptySet(); + Set doesNotMatterDbFileKeys = emptySet(); Map doesNotMatterReportFiles = Collections.emptyMap(); int[][] doesNotMatterScores = new int[0][0]; @@ -78,6 +80,6 @@ public class MatchesByScoreTest { } private static FileSimilarity.File fileOf(String key) { - return new FileSimilarity.File("path of " + key, null); + return new FileSimilarity.File("path of " + key, emptyList()); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/SourceSimilarityImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/SourceSimilarityImplTest.java index 1c50bddbdb5..11ae0481655 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/SourceSimilarityImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/filemove/SourceSimilarityImplTest.java @@ -25,6 +25,7 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; import static org.assertj.core.api.Assertions.assertThat; public class SourceSimilarityImplTest { @@ -32,7 +33,7 @@ public class SourceSimilarityImplTest { @Rule public ExpectedException expectedException = ExpectedException.none(); - SourceSimilarityImpl underTest = new SourceSimilarityImpl(); + private SourceSimilarityImpl underTest = new SourceSimilarityImpl(); @Test public void zero_if_fully_different() { @@ -53,4 +54,9 @@ public class SourceSimilarityImplTest { assertThat(underTest.score(asList("a"), asList("a", "b", "c"))).isEqualTo(33); assertThat(underTest.score(asList("a", "b", "c"), asList("a"))).isEqualTo(33); } + + @Test + public void two_empty_lists_are_not_considered_as_equal() { + assertThat(underTest.score(emptyList(), emptyList())).isEqualTo(0); + } } -- cgit v1.2.3