From d1f3d97a9ecc50164c4522adc2ce2ad36c60c334 Mon Sep 17 00:00:00 2001 From: Richard Otte Date: Thu, 16 Nov 2017 14:40:36 -0500 Subject: [PATCH] Cleaning up code to address pr comments --- .../hub/detect/bomtool/HexBomTool.groovy | 36 +++--- .../bomtool/hex/Rebar3TreeParser.groovy | 115 +++++++++--------- .../DependencyGraphResourceTestUtil.groovy | 52 ++++---- 3 files changed, 101 insertions(+), 102 deletions(-) diff --git a/src/main/groovy/com/blackducksoftware/integration/hub/detect/bomtool/HexBomTool.groovy b/src/main/groovy/com/blackducksoftware/integration/hub/detect/bomtool/HexBomTool.groovy index df514f841..abfb2bd40 100644 --- a/src/main/groovy/com/blackducksoftware/integration/hub/detect/bomtool/HexBomTool.groovy +++ b/src/main/groovy/com/blackducksoftware/integration/hub/detect/bomtool/HexBomTool.groovy @@ -40,52 +40,52 @@ import groovy.transform.TypeChecked @Component @TypeChecked class HexBomTool extends BomTool { - private final Logger logger = LoggerFactory.getLogger(HexBomTool.class) + private final Logger logger = LoggerFactory.getLogger(HexBomTool.class); - public static final String REBAR_CONFIG = 'rebar.config' + public static final String REBAR_CONFIG = 'rebar.config'; @Autowired - DetectConfiguration detectConfiguration + DetectConfiguration detectConfiguration; @Autowired - Rebar3TreeParser rebarTreeParser + Rebar3TreeParser rebarTreeParser; - private String rebarExePath - private boolean rebarApplies + private String rebarExePath; + private boolean rebarApplies; @Override public BomToolType getBomToolType() { - return BomToolType.HEX + return BomToolType.HEX; } @Override public boolean isBomToolApplicable() { - boolean hasRebarConfig = detectFileManager.containsAllFiles(sourcePath, REBAR_CONFIG) + boolean hasRebarConfig = detectFileManager.containsAllFiles(sourcePath, REBAR_CONFIG); if (hasRebarConfig) { - logger.info('Rebar3 build tool applies for HEX given the current configuration.') - rebarExePath = findExecutablePath(ExecutableType.REBAR3, true, detectConfiguration.getHexRebar3Path()) + logger.info('Rebar3 build tool applies for HEX given the current configuration.'); + rebarExePath = findExecutablePath(ExecutableType.REBAR3, true, detectConfiguration.getHexRebar3Path()); if (!rebarExePath) { - logger.warn('Could not find a rebar3 executable.') + logger.warn('Could not find a rebar3 executable.'); } } - rebarApplies = rebarExePath && hasRebarConfig + rebarApplies = rebarExePath && hasRebarConfig; - return rebarApplies + return rebarApplies; } @Override public List extractDetectCodeLocations(DetectProject detectProject) { if (rebarApplies) { - Executable rebar3TreeExe = new Executable(new File(sourcePath), ['REBAR_COLOR': 'none'], rebarExePath, ['tree']) - List output = executableRunner.execute(rebar3TreeExe).standardOutputAsList - DetectCodeLocation projectCodeLocation = rebarTreeParser.parseRebarTreeOutput(output, detectProject, sourcePath) + Executable rebar3TreeExe = new Executable(new File(sourcePath), ['REBAR_COLOR': 'none'], rebarExePath, ['tree']); + List output = executableRunner.execute(rebar3TreeExe).standardOutputAsList; + DetectCodeLocation projectCodeLocation = rebarTreeParser.parseRebarTreeOutput(output, detectProject, sourcePath); - return [projectCodeLocation] + return [projectCodeLocation]; } - [] + return []; } } diff --git a/src/main/groovy/com/blackducksoftware/integration/hub/detect/bomtool/hex/Rebar3TreeParser.groovy b/src/main/groovy/com/blackducksoftware/integration/hub/detect/bomtool/hex/Rebar3TreeParser.groovy index 1189c8a5e..a2f6271a9 100644 --- a/src/main/groovy/com/blackducksoftware/integration/hub/detect/bomtool/hex/Rebar3TreeParser.groovy +++ b/src/main/groovy/com/blackducksoftware/integration/hub/detect/bomtool/hex/Rebar3TreeParser.groovy @@ -33,7 +33,6 @@ import com.blackducksoftware.integration.hub.bdio.model.Forge import com.blackducksoftware.integration.hub.bdio.model.dependency.Dependency import com.blackducksoftware.integration.hub.bdio.model.externalid.ExternalId import com.blackducksoftware.integration.hub.bdio.model.externalid.ExternalIdFactory -import com.blackducksoftware.integration.hub.detect.DetectConfiguration import com.blackducksoftware.integration.hub.detect.model.BomToolType import com.blackducksoftware.integration.hub.detect.model.DetectCodeLocation import com.blackducksoftware.integration.hub.detect.model.DetectProject @@ -43,117 +42,117 @@ import groovy.transform.TypeChecked @Component @TypeChecked class Rebar3TreeParser { - private final Logger logger = LoggerFactory.getLogger(Rebar3TreeParser.class) + private final Logger logger = LoggerFactory.getLogger(Rebar3TreeParser.class); - public static final String LAST_DEPENDENCY_CHARACTER = '└' - public static final String NTH_DEPENDENCY_CHARACTER = '├' - public static final String HORIZONTAL_SEPARATOR_CHARACTER = '─' - public static final String INNER_LEVEL_CHARACTER = '│' - public static final String INNER_LEVEL_PREFIX = INNER_LEVEL_CHARACTER + ' '.multiply(2) - public static final String OUTER_LEVEL_PREFIX = ' '.multiply(3) - public static final String PROJECT_FORGE_STRING = '(project app)' + public static final String LAST_DEPENDENCY_CHARACTER = '└'; + public static final String NTH_DEPENDENCY_CHARACTER = '├'; + public static final String HORIZONTAL_SEPARATOR_CHARACTER = '─'; + public static final String INNER_LEVEL_CHARACTER = '│'; + public static final String INNER_LEVEL_PREFIX = INNER_LEVEL_CHARACTER + ' '.multiply(2); + public static final String OUTER_LEVEL_PREFIX = ' '.multiply(3); + public static final String PROJECT_IDENTIFIER = '(project app)'; @Autowired - DetectConfiguration detectConfiguration + ExternalIdFactory externalIdFactory; - @Autowired - ExternalIdFactory externalIdFactory - - public DetectCodeLocation parseRebarTreeOutput(List dependencyTreeOutput, DetectProject detectProject, String sourcePath) { - MutableDependencyGraph graph = new MutableMapDependencyGraph() - String projectName = '' - String projectVersionName = '' - Stack dependencyStack = new Stack() - int previousTreeLevel = 0 - Dependency previousDependency + public DetectCodeLocation parseRebarTreeOutput(final List dependencyTreeOutput, final DetectProject detectProject, final String sourcePath) { + MutableDependencyGraph graph = new MutableMapDependencyGraph(); + String projectName = ''; + String projectVersionName = ''; + Stack dependencyStack = new Stack(); + int previousTreeLevel = 0; + Dependency previousDependency; try { for (String line: dependencyTreeOutput) { if (line.contains(HORIZONTAL_SEPARATOR_CHARACTER)) { - Dependency currentDependency = createDependencyFromLine(line) + Dependency currentDependency = createDependencyFromLine(line); - final int currentTreeLevel = getDependencyLevelFromLine(line) + final int currentTreeLevel = getDependencyLevelFromLine(line); if (currentTreeLevel == previousTreeLevel + 1) { - dependencyStack.push(previousDependency) + dependencyStack.push(previousDependency); } else if (currentTreeLevel < previousTreeLevel) { - int levelDelta = (previousTreeLevel - currentTreeLevel) + int levelDelta = (previousTreeLevel - currentTreeLevel); for (int levels = 0; levels < levelDelta; levels++) { - dependencyStack.pop() + dependencyStack.pop(); } } else if (currentTreeLevel != previousTreeLevel) { - logger.error(String.format('The tree level (%s) and this line (%s) with count %s can\'t be reconciled.', previousTreeLevel, line, currentTreeLevel)) + logger.error(String.format('The tree level (%s) and this line (%s) with count %s can\'t be reconciled.', previousTreeLevel, line, currentTreeLevel)); } if (dependencyStack.size() == 0) { if ( isProject(line) ) { - projectName = currentDependency.name - projectVersionName = currentDependency.version + projectName = currentDependency.name; + projectVersionName = currentDependency.version; } else { - graph.addChildToRoot(currentDependency) + graph.addChildToRoot(currentDependency); } } else if (dependencyStack.size() == 1 && dependencyStack.peek().name.equals(projectName) && dependencyStack.peek().version.equals(projectVersionName)) { - graph.addChildToRoot(currentDependency) + graph.addChildToRoot(currentDependency); } else { - graph.addChildWithParents(currentDependency, dependencyStack.peek()) + graph.addChildWithParents(currentDependency, dependencyStack.peek()); } - previousDependency = currentDependency - previousTreeLevel = currentTreeLevel + previousDependency = currentDependency; + previousTreeLevel = currentTreeLevel; } } } catch (final Exception e) { - logger.error('Exception parsing rebar output: ' + e.getMessage()) + logger.error('Exception parsing rebar output: ' + e.getMessage()); } - detectProject.setProjectNameIfNotSet(projectName) - detectProject.setProjectVersionNameIfNotSet(projectVersionName) + detectProject.setProjectNameIfNotSet(projectName); + detectProject.setProjectVersionNameIfNotSet(projectVersionName); - final ExternalId id = externalIdFactory.createNameVersionExternalId(Forge.HEX, projectName, projectVersionName) + final ExternalId id = externalIdFactory.createNameVersionExternalId(Forge.HEX, projectName, projectVersionName); - return new DetectCodeLocation(BomToolType.HEX, sourcePath, projectName, projectVersionName, id, graph) + return new DetectCodeLocation(BomToolType.HEX, sourcePath, projectName, projectVersionName, id, graph); } - private Dependency createDependencyFromLine(String line) { - String nameVersionLine = reduceLineToNameVersion(line) - String name = nameVersionLine.substring(0, nameVersionLine.lastIndexOf(HORIZONTAL_SEPARATOR_CHARACTER)) - String version = nameVersionLine.substring(nameVersionLine.lastIndexOf(HORIZONTAL_SEPARATOR_CHARACTER) + 1) - ExternalId externalId = externalIdFactory.createNameVersionExternalId(Forge.HEX, name, version) + private Dependency createDependencyFromLine(final String line) { + String nameVersionLine = reduceLineToNameVersion(line); + String name = nameVersionLine.substring(0, nameVersionLine.lastIndexOf(HORIZONTAL_SEPARATOR_CHARACTER)); + String version = nameVersionLine.substring(nameVersionLine.lastIndexOf(HORIZONTAL_SEPARATOR_CHARACTER) + 1); + ExternalId externalId = externalIdFactory.createNameVersionExternalId(Forge.HEX, name, version); - return new Dependency(name, version, externalId) + return new Dependency(name, version, externalId); } private String reduceLineToNameVersion(String line) { - for (String specialCharacter : [ + String[] ignoredSpecialCharacters = [ LAST_DEPENDENCY_CHARACTER, NTH_DEPENDENCY_CHARACTER, INNER_LEVEL_CHARACTER - ]) { - line = line.replaceAll(specialCharacter, '') + ]; + for (final String specialCharacter : ignoredSpecialCharacters) { + line = line.replaceAll(specialCharacter, ''); } - line = line.replaceFirst(HORIZONTAL_SEPARATOR_CHARACTER, '') + + line = line.replaceFirst(HORIZONTAL_SEPARATOR_CHARACTER, ''); + if (line.endsWith(')')) { - line = line.substring(0, line.lastIndexOf('(')) + line = line.substring(0, line.lastIndexOf('(')); } - return line.trim() + return line.trim(); } private int getDependencyLevelFromLine(String line) { - int level = 0 + int level = 0; while(line.startsWith(INNER_LEVEL_PREFIX) || line.startsWith(OUTER_LEVEL_PREFIX)) { - line = line.substring(3) - level++ + line = line.substring(3); + level++; } - return level + return level; } - private boolean isProject(String line) { - String forgeString = '' + private boolean isProject(final String line) { + String forgeString = ''; if (line.endsWith(')')) { - forgeString = line.substring(line.lastIndexOf('(')) + forgeString = line.substring(line.lastIndexOf('(')); } - return PROJECT_FORGE_STRING.equals(forgeString) + return PROJECT_IDENTIFIER.equals(forgeString); } } diff --git a/src/test/groovy/com/blackducksoftware/integration/hub/detect/testutils/DependencyGraphResourceTestUtil.groovy b/src/test/groovy/com/blackducksoftware/integration/hub/detect/testutils/DependencyGraphResourceTestUtil.groovy index 568dc9a35..4ed473bf7 100644 --- a/src/test/groovy/com/blackducksoftware/integration/hub/detect/testutils/DependencyGraphResourceTestUtil.groovy +++ b/src/test/groovy/com/blackducksoftware/integration/hub/detect/testutils/DependencyGraphResourceTestUtil.groovy @@ -10,54 +10,54 @@ import com.google.gson.GsonBuilder class DependencyGraphResourceTestUtil { public static void assertGraph(String expectedResourceFile, DependencyGraph actualGraph) { - DependencyGraphSummarizer summarizer = new DependencyGraphSummarizer(new Gson()) + DependencyGraphSummarizer summarizer = new DependencyGraphSummarizer(new Gson()); - TestUtil testUtil = new TestUtil() - String json = testUtil.getResourceAsUTF8String(expectedResourceFile) + TestUtil testUtil = new TestUtil(); + String json = testUtil.getResourceAsUTF8String(expectedResourceFile); - GraphSummary expected = summarizer.fromJson(json) - GraphSummary actual = summarizer.fromGraph(actualGraph) - println new GsonBuilder().setPrettyPrinting().create().toJson(actual) - assertSummarries(expected, actual) + GraphSummary expected = summarizer.fromJson(json); + GraphSummary actual = summarizer.fromGraph(actualGraph); + println new GsonBuilder().setPrettyPrinting().create().toJson(actual); + assertSummarries(expected, actual); } public static void assertGraph(DependencyGraph expectedGraph, DependencyGraph actualGraph) { - DependencyGraphSummarizer summarizer = new DependencyGraphSummarizer(new Gson()) - GraphSummary expected = summarizer.fromGraph(expectedGraph) - GraphSummary actual = summarizer.fromGraph(actualGraph) - println new GsonBuilder().setPrettyPrinting().create().toJson(actual) - assertSummarries(expected, actual) + DependencyGraphSummarizer summarizer = new DependencyGraphSummarizer(new Gson()); + GraphSummary expected = summarizer.fromGraph(expectedGraph); + GraphSummary actual = summarizer.fromGraph(actualGraph); + println new GsonBuilder().setPrettyPrinting().create().toJson(actual); + assertSummarries(expected, actual); } public static void assertSummarries(GraphSummary expected, GraphSummary actual) { - assertSet(expected.rootExternalDataIds, actual.rootExternalDataIds, "Root external ids") - assertSet(expected.dependencySummaries.keySet(), actual.dependencySummaries.keySet(), "Dependencies in graph") + assertSet(expected.rootExternalDataIds, actual.rootExternalDataIds, "Root external ids"); + assertSet(expected.dependencySummaries.keySet(), actual.dependencySummaries.keySet(), "Dependencies in graph"); - def expectedRelationshipIds = expected.externalDataIdRelationships.keySet() + def expectedRelationshipIds = expected.externalDataIdRelationships.keySet(); def expectedExistingRelationshipsIds = expectedRelationshipIds.findAll{ key -> expected.externalDataIdRelationships.get(key) != null && expected.externalDataIdRelationships.get(key).size() > 0} - def actualRelationshipIds = actual.externalDataIdRelationships.keySet() + def actualRelationshipIds = actual.externalDataIdRelationships.keySet(); def actualExistingRelationshipsIds = actualRelationshipIds.findAll{ key -> actual.externalDataIdRelationships.get(key) != null && actual.externalDataIdRelationships.get(key).size() > 0} - assertSet(expectedExistingRelationshipsIds, actualExistingRelationshipsIds, "Existing relationships") + assertSet(expectedExistingRelationshipsIds, actualExistingRelationshipsIds, "Existing relationships"); for (String key : expected.dependencySummaries.keySet()){ - assertEquals(expected.dependencySummaries.get(key).name, actual.dependencySummaries.get(key).name) - assertEquals(expected.dependencySummaries.get(key).version, actual.dependencySummaries.get(key).version) + assertEquals(expected.dependencySummaries.get(key).name, actual.dependencySummaries.get(key).name); + assertEquals(expected.dependencySummaries.get(key).version, actual.dependencySummaries.get(key).version); } for (String key : expectedExistingRelationshipsIds){ - assertSet(expected.externalDataIdRelationships.get(key), actual.externalDataIdRelationships.get(key), "External data id relationships for " + key) + assertSet(expected.externalDataIdRelationships.get(key), actual.externalDataIdRelationships.get(key), "External data id relationships for " + key); } } public static void assertSet (Set expected, Set actual, String title) { - Set missingExpected = new HashSet<>(expected) - missingExpected.removeAll(actual) + Set missingExpected = new HashSet<>(expected); + missingExpected.removeAll(actual); - Set extraActual = new HashSet<>(actual) - extraActual.removeAll(expected) + Set extraActual = new HashSet<>(actual); + extraActual.removeAll(expected); - assertTrue(title + ": Found missing expected " + missingExpected.toString(), missingExpected.size() == 0) - assertTrue(title + ": Found extra actual " + extraActual.toString(), extraActual.size() == 0) + assertTrue(title + ": Found missing expected " + missingExpected.toString(), missingExpected.size() == 0); + assertTrue(title + ": Found extra actual " + extraActual.toString(), extraActual.size() == 0); } }