DotNet 3 #176

gkillough · 2020-05-27T19:26:47Z

Description

Added functionality to determine available dotnet runtimes and programmatically select the appropriate nuget inspector.
Added several abstractions for duplicate nuget functionality.

…ic to determine which runtime to use

…nd clean up TODOs

blackduck-copilot · 2020-05-27T19:26:49Z

Black Duck Security Report

Merging #176 into master will not change security risk.

Added Components

Clean: 1

Removed Components

Clean: 1

Click here to see full report

taikuukaits · 2020-05-27T20:12:35Z

...ava/com/synopsys/integration/detect/tool/detector/inspectors/nuget/NugetRuntimeResolver.java

+import com.synopsys.integration.detectable.detectable.executable.ExecutableRunner;
+import com.synopsys.integration.detectable.detectable.executable.ExecutableRunnerException;
+
+public class NugetRuntimeResolver {


I'd rather we separated the invocation of the command from the parsing and matching - it would make the unit tests simpler.

…nto separate classes

taikuukaits · 2020-05-28T18:49:39Z

...sys/integration/detect/tool/detector/inspectors/nuget/DotNetRuntimeAvailabilityVerifier.java

+    }
+
+    public boolean isRuntimeAvailable(Integer... versionTokens) throws DetectableException {
+        List<String> runtimes = dotNetRuntimeFinder.listAvailableRuntimes();


I think this is improved, but I think we could isolate more if you take in a List runtimes instead of taking the DotNetRuntimeFinder.

The reason I was hesitant about doing something like that is that it would allow you to pass in an arbitrary list of Strings. Even if we wrapped those Strings in a new type of object (e.g. DotNetRuntimeModel or something), I think these are concerns that are already correctly coupled. Decoupling further would essentially make this a semVer regex util that just so happens to have a constant pattern related to dotnet runtimes. If we want to go that route, that's fine (obviously we would move the runtime pattern constant somewhere else), but I think that is what we're really talking about if we decouple this further.

Gotcha, I can say it is very typical for parsers to take in a list of strings, such as NpmParser. And it is convention we know you can't just pass an arbitrary list of strings. That said I would not be opposed to a small data class like DotNetRuntimeResult(List runtimes).

Personally not having to create a mock runtime executable and eliminating the explicit dependency outweigh protecting someone from passing wrong strings. But I would also be fine with what you have. Depends on the type of change that comes in next.

JakeMathews · 2020-05-28T21:37:24Z

src/main/java/com/synopsys/integration/detect/RunBeanConfiguration.java

@@ -139,7 +141,7 @@ public AirGapPathFinder airGapPathFinder() {

    @Bean
    public CodeLocationNameGenerator codeLocationNameService() {
-        final String codeLocationNameOverride = detectConfiguration.getValueOrEmpty(DetectProperties.Companion.getDETECT_CODE_LOCATION_NAME()).orElse(null);


Not in love with removing all the finals, but since we don't have an official rule on it, it's fine.

My save actions are configured based on the Engineer Onboarding on Confluence, but if you guys have a different preference in Detect, I can update the settings for the project accordingly. I assumed the finals were just left over from when we still had that convention mandated.

If you could add them back for now that would be great.

.../synopsys/integration/detect/tool/detector/inspectors/nuget/AirgapNugetInspectorLocator.java

...sys/integration/detect/tool/detector/inspectors/nuget/DotNetRuntimeAvailabilityVerifier.java

.../synopsys/integration/detect/tool/detector/inspectors/nuget/OnlineNugetInspectorLocator.java

...integration/detect/tool/detector/inspectors/nuget/DotNetRuntimeAvailabilityVerifierTest.java

...ynopsys/integration/detect/tool/detector/inspectors/nuget/LocatorNugetInspectorResolver.java

…erbose implementation

JakeMathews · 2020-06-02T15:42:32Z

src/main/java/com/synopsys/integration/detect/RunBeanConfiguration.java

@@ -139,7 +141,7 @@ public AirGapPathFinder airGapPathFinder() {

    @Bean
    public CodeLocationNameGenerator codeLocationNameService() {
-        final String codeLocationNameOverride = detectConfiguration.getValueOrEmpty(DetectProperties.Companion.getDETECT_CODE_LOCATION_NAME()).orElse(null);


If you could add them back for now that would be great.

JakeMathews · 2020-06-02T15:45:52Z

...ynopsys/integration/detect/tool/detector/inspectors/nuget/LocatorNugetInspectorResolver.java

-            final String inspectorExe = foundExe.get().getAbsolutePath();
-            logger.debug("Found nuget inspector: " + inspectorExe);
-            return new ExeNugetInspector(executableRunner, inspectorExe);
+        Optional<File> foundExecutable = fileFinder.findFiles(toolsFolder, inspectorName, 3).stream().findFirst();


Suggested change

Optional<File> foundExecutable = fileFinder.findFiles(toolsFolder, inspectorName, 3).stream().findFirst();

File foundExecutable = fileFinder.findFiles(toolsFolder, inspectorName, 3).stream().

.filter(File::exists)

.findFirst()

.orElseThrow(() -> new DetectableException(String.format("Unable to find nuget inspector, looking for %s in %s", inspectorName, toolsFolder.toString())));

I think we don't need the if/else if we do the filtering in the Stream.

JakeMathews · 2020-06-02T16:00:34Z

Also the battery tests seem to be failing on Travis. Not sure if it is a result of the changes you made, or if the changes on master just need to be merged with this branch.

gkillough · 2020-06-02T16:03:50Z

Also the battery tests seem to be failing on Travis. Not sure if it is a result of the changes you made, or if the changes on master just need to be merged with this branch.

@JakeMathews It's because the new inspector isn't released on artifactory yet.

sonarcloud · 2020-06-02T18:56:02Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
1 Code Smell

15.5% Coverage
0.0% Duplication

The version of Java (1.8.0_151) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

Gavin Killough added 6 commits May 22, 2020 10:26

feat(nuget): Begin implementing support for dotnet 3.1 runtime

a8d1717

feat(nuget): Add semantic version parsing for dotnet runtimes and log…

8bf0ca5

…ic to determine which runtime to use

feat(nuget): Make version parsing more generic

71e8082

Add artifactory constants for dotnet3

54d35c2

feat(nuget): Create additional abstraction for redundant nuget code a…

971dcc6

…nd clean up TODOs

Merge branch 'master' into gk_support_dotnet_3

e7d2ae7

gkillough requested review from ekerwin, stevebillings, taikuukaits and JakeMathews May 27, 2020 19:26

taikuukaits reviewed May 27, 2020

View reviewed changes

fix: resolve sonar code smells

4b58ca7

taikuukaits approved these changes May 27, 2020

View reviewed changes

refactor(nuget): Split dotnet runtime finding and runtime verifying i…

fc0d762

…nto separate classes

gkillough marked this pull request as ready for review May 28, 2020 12:48

taikuukaits reviewed May 28, 2020

View reviewed changes

JakeMathews suggested changes May 28, 2020

View reviewed changes

Gavin Killough added 7 commits May 29, 2020 09:46

fix: Resolve several issues discovered in code review

9284bef

fix: Replace regex functionality with java code

71449bc

refactor: Replace POC of dotnet runtime regex alternative with less v…

4bb964f

…erbose implementation

refactor: Clean up dotnet runtime parsing logic

b2c90ba

Merge branch 'gk_support_dotnet_3_no_regex' into gk_support_dotnet_3

fe6b978

Merge branch 'master' into gk_support_dotnet_3

63f520d

refactor: Make method name plural

fcd5c3f

gkillough requested review from JakeMathews and taikuukaits June 2, 2020 12:46

chore: Add copyright header to new files

5cde737

JakeMathews approved these changes Jun 2, 2020

View reviewed changes

taikuukaits approved these changes Jun 2, 2020

View reviewed changes

Gavin Killough added 2 commits June 2, 2020 12:15

fix: Add finals to local variables based on project preference

3bf080b

Merge branch 'master' into gk_support_dotnet_3

f50c14e

gkillough merged commit fa3da79 into master Jun 2, 2020

JakeMathews mentioned this pull request Jun 4, 2020

reducing the noise #178

Closed

gkillough deleted the gk_support_dotnet_3 branch June 10, 2020 16:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DotNet 3 #176

DotNet 3 #176

gkillough commented May 27, 2020

blackduck-copilot bot commented May 27, 2020 •

edited

taikuukaits May 27, 2020

taikuukaits May 28, 2020

gkillough May 28, 2020

taikuukaits May 29, 2020

JakeMathews May 28, 2020

gkillough May 29, 2020

JakeMathews Jun 2, 2020

JakeMathews Jun 2, 2020

JakeMathews Jun 2, 2020

JakeMathews Jun 2, 2020

JakeMathews commented Jun 2, 2020

gkillough commented Jun 2, 2020

sonarcloud bot commented Jun 2, 2020

-        Optional<File> foundExecutable = fileFinder.findFiles(toolsFolder, inspectorName, 3).stream().findFirst();
+        File foundExecutable = fileFinder.findFiles(toolsFolder, inspectorName, 3).stream().
+            .filter(File::exists)
+            .findFirst()
+            .orElseThrow(() -> new DetectableException(String.format("Unable to find nuget inspector, looking for %s in %s", inspectorName, toolsFolder.toString())));

DotNet 3 #176

DotNet 3 #176

Conversation

gkillough commented May 27, 2020

Description

blackduck-copilot bot commented May 27, 2020 • edited

Black Duck Security Report

Added Components

Removed Components

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JakeMathews commented Jun 2, 2020

gkillough commented Jun 2, 2020

sonarcloud bot commented Jun 2, 2020

blackduck-copilot bot commented May 27, 2020 •

edited