Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jarviz Updates #4

Merged
merged 3 commits into from
Mar 12, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Jarviz CLI is a command-line tool designed for \*nix systems to perform dependen

```json
{
"appSetName": "FooPortfolio",
"applicationName": "MyApp",
"artifactFileName": "foo-product-1.2.1.jar",
"artifactId": "foo-product",
Expand Down
2 changes: 2 additions & 0 deletions jarviz-cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ The output of the `analyze` step is a [JSON Lines (.jsonl)](http://jsonlines.org

```json
{
"appSetName": "FooPortfolio",
"applicationName": "MyApp",
"artifactFileName": "foo-product-1.2.1.jar",
"artifactId": "foo-product",
Expand All @@ -214,6 +215,7 @@ The output of the `analyze` step is a [JSON Lines (.jsonl)](http://jsonlines.org

#### Fields

- `appSetName` - Optional name for the application set.
- `applicationName` - The human-readable name of the application.
- `artifactFileName` - The file name of the artifact (e.g. `"foo-product-1.2.1.jar"`).
- `artifactId` - The id of the artifact (e.g. `"foo-product"`).
Expand Down
3 changes: 2 additions & 1 deletion jarviz-cli/samples/config.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"artifactDirectory": "/tmp/jarviz/artifacts"
"artifactDirectory": "/tmp/jarviz/artifacts",
"continueOnMavenError": false
}
2 changes: 2 additions & 0 deletions jarviz-lib/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ This Java library scans the Java [bytecode](https://docs.oracle.com/javase/specs

```json
{
"appSetName": "FooPortfolio",
"applicationName": "MyApp",
"artifactFileName": "foo-product-1.2.1.jar",
"artifactId": "foo-product",
Expand Down Expand Up @@ -240,6 +241,7 @@ The output of `analyze` is a JSON Lines ([`.jsonl`](http://jsonlines.org/)) file

#### Fields

- `appSetName` - Optional name for the application set.
- `applicationName` - The human readable name of the application.
- `artifactFileName` - The file name of the artifact (e.g. `"foo-product-1.2.1.jar"`).
- `artifactId` - The id of the artifact (e.g. `"foo-product"`).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@ public interface JarvizConfig {
*/
String getArtifactDirectory();

/**
* When processing artifacts in a given appSet, should Jarviz ignore any maven
* resolution or download errors for any given artifact and continue processing
* remaining ones?
*
* @return Should Jarviz ignore maven errors and continue processing remaining artifacts?
*/
boolean getContinueOnMavenError();

/**
* Creates a default config with artifactDirectory set to "/tmp/jarviz/artifacts"
*
Expand Down
31 changes: 26 additions & 5 deletions jarviz-lib/src/main/java/com/vrbo/jarviz/model/Artifact.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.vrbo.jarviz.model;

import java.util.Arrays;
import java.util.List;
import java.util.Optional;

import org.immutables.value.Value;
Expand All @@ -37,6 +39,8 @@
@JsonInclude(value = JsonInclude.Include.NON_EMPTY)
public interface Artifact {

List<String> NON_SPECIFIC_VERSIONS = Arrays.asList("LATEST", "RELEASE");

/**
* The packaging type of the artifact, default "jar".
* Eg: "jar" or "war"
Expand Down Expand Up @@ -93,16 +97,23 @@ default String getPackaging() {
* For releases: "foo-bar.239.1.jar"
* With classifier: "foo-bar.239.1-test.jar"
* Snapshot: "foo-bar-2.0.1-20200708.191052-38.jar"
* For unresolvable versions LATEST or RELEASE: "foo-bar.jar"
*
* @return The file name.
*/
@JsonIgnore
default String toFileName() {
return String.format("%s-%s%s.%s",
getArtifactId(),
getVersion(),
getClassifier().map(s -> "-" + s).orElse(""),
getPackaging());
if (isVersionLatestOrRelease()) {
return String.format("%s.%s",
getArtifactId(),
getPackaging());
} else {
return String.format("%s-%s%s.%s",
getArtifactId(),
getVersion(),
getClassifier().map(s -> "-" + s).orElse(""),
getPackaging());
}
}

/**
Expand Down Expand Up @@ -144,5 +155,15 @@ default void check() {
}
}

/**
* Is Version LATEST or RELEASE?
*
* @return true if version is LATEST or RELEASE, false otherwise.
*/
@Value.Default
default boolean isVersionLatestOrRelease() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this contain 2, we can keep it as a List, else we can make a it a Set.

return NON_SPECIFIC_VERSIONS.contains(getVersion());
}

class Builder extends ImmutableArtifact.Builder {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
@JsonInclude(value = JsonInclude.Include.NON_EMPTY)
public interface CouplingRecord {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a Java doc pls?

String getAppSetName();

/**
* The human readable name of the application.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ private int analyzeApplicationSet(final ApplicationSet appSet,

final CouplingRecordWriter writer = new CouplingRecordWriter(reportFile);
for (Application application : appSet.getApplications()) {
appSetCouplingCount += analyzeApplication(application, filterConfig, classLoaderService, writer);
appSetCouplingCount += analyzeApplication(appSet, application, filterConfig, classLoaderService, writer);
}

log.info("ApplicationSet={}, TotalClassesAnalyzed={}, TotalCouplingsFound={}",
Expand All @@ -120,7 +120,8 @@ private int analyzeApplicationSet(final ApplicationSet appSet,
* @param writer Coupling record writer.
* @return Coupling count for the application.
*/
private int analyzeApplication(final Application app,
private int analyzeApplication(final ApplicationSet appSet,
final Application app,
final CouplingFilterConfig filterConfig,
final ClassLoaderService classLoaderService,
final CouplingRecordWriter writer) {
Expand All @@ -131,7 +132,7 @@ private int analyzeApplication(final Application app,
for (Artifact artifact : app.getArtifacts()) {
log.info("Analyzing artifact: {}", artifact.toFileName());

appCouplingCount += analyzeArtifact(app, artifact, filterConfig, classLoaderService, writer);
appCouplingCount += analyzeArtifact(appSet, app, artifact, filterConfig, classLoaderService, writer);
}

log.info("Application={}, TotalClassesAnalyzed={}, TotalCouplingsFound={}",
Expand All @@ -150,7 +151,8 @@ private int analyzeApplication(final Application app,
* @param writer Coupling record writer.
* @return Coupling count for the artifact.
*/
private int analyzeArtifact(final Application app,
private int analyzeArtifact(final ApplicationSet appSet,
final Application app,
final Artifact artifact,
final CouplingFilterConfig filterConfig,
final ClassLoaderService classLoaderService,
Expand All @@ -171,16 +173,18 @@ private int analyzeArtifact(final Application app,

// Write the CouplingRecord as Json
couplings.stream()
.map(c -> toCouplingRecord(app, artifact, c))
.map(c -> toCouplingRecord(appSet, app, artifact, c))
.forEach(writer::writeAsJson);

return couplings.size();
}

private static CouplingRecord toCouplingRecord(final Application app,
private static CouplingRecord toCouplingRecord(final ApplicationSet appSet,
final Application app,
final Artifact artifact,
final MethodCoupling methodCoupling) {
return new CouplingRecord.Builder()
.appSetName(appSet.getAppSetName().orElse(""))
.applicationName(app.getAppName())
.artifactFileName(artifact.toFileName())
.artifactId(artifact.getArtifactId())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ public class MavenArtifactDiscoveryService implements ArtifactDiscoveryService {
private final Logger log = LoggerFactory.getLogger(MavenArtifactDiscoveryService.class);

private final String localRepoPath;
private final boolean continueOnMavenError;

@Inject
public MavenArtifactDiscoveryService(final JarvizConfig config) {
this.localRepoPath = config.getArtifactDirectory();
this.continueOnMavenError = config.getContinueOnMavenError();
}

@Override
Expand All @@ -59,18 +61,23 @@ private Process runMavenCopy(final Artifact artifact) throws ArtifactNotFoundExc
try {
final String artifactMavenId = artifact.toMavenId();
log.info("Maven: fetching artifact {}", artifactMavenId);
final String mvnCommand = String.format("mvn dependency:copy -DoutputDirectory=%s -Dartifact=%s",
localRepoPath, artifactMavenId);

final String stripVersionSwitch = artifact.isVersionLatestOrRelease() ? "true" : "false";
final String mvnCommand = String.format("mvn dependency:copy -DoutputDirectory=%s -Dartifact=%s -Dmdep.stripVersion=%s",
localRepoPath, artifactMavenId, stripVersionSwitch);
final Process process = Runtime.getRuntime().exec(mvnCommand);
int exitCode = process.waitFor();

drainInputStream(process.getInputStream()).forEach(s -> log.info("{}", s));
if (exitCode != 0) {
// In case, there is an error, let's log and throw exception
drainInputStream(process.getInputStream()).forEach(s -> log.info("{}", s));
drainInputStream(process.getErrorStream()).forEach(s -> log.error("{}", s));
log.info("Maven command failed: {}", mvnCommand);
throw new ArtifactNotFoundException(
String.format("Unable to fetch the artifact %s from Maven repository", artifactMavenId));
log.error("Maven command failed: {}", mvnCommand);

if (!continueOnMavenError) {
throw new ArtifactNotFoundException(
String.format("Unable to fetch the artifact %s from Maven repository", artifactMavenId));
}
}

return process;
Expand Down
46 changes: 46 additions & 0 deletions jarviz-lib/src/test/java/com/vrbo/jarviz/model/ArtifactTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,52 @@ public void testToFileName() {
.isEqualTo("foo-bar-2.0.1-SNAPSHOT.jar");
}

@Test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

public void testToFileName_WhenVersionIsUnspecified() {

final Artifact releaseVerArtifact =
new Artifact.Builder()
.artifactId("api-service")
.groupId("com.vrbo.api")
.version("RELEASE")
.build();

assertThat(releaseVerArtifact.toFileName())
.isEqualTo("api-service.jar");

assertThat(new Artifact.Builder()
.from(releaseVerArtifact)
.classifier("logic")
.build()
.toFileName())
.isEqualTo("api-service.jar");

assertThat(new Artifact.Builder()
.from(releaseVerArtifact)
.packaging("war")
.build()
.toFileName())
.isEqualTo("api-service.war");

final Artifact latestVerArtifact =
new Artifact.Builder()
.artifactId("foo-bar")
.groupId("abc.xyz")
.version("LATEST")
.build();

assertThat(latestVerArtifact.toFileName())
.isEqualTo("foo-bar.jar");

assertThat(new Artifact.Builder()
.from(latestVerArtifact)
.version("LATEST")
.baseVersion(Optional.empty())
.build()
.toFileName())
.isEqualTo("foo-bar.jar");
}

@Test
public void testToMavenId() {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public void setup() {
artifactDiscoveryService = new MavenArtifactDiscoveryService(
new JarvizConfig.Builder()
.artifactDirectory("/tmp/jarviz/artifacts")
.continueOnMavenError(false)
.build());
}

Expand All @@ -59,4 +60,19 @@ public void testDiscoverArtifact_Fail() throws ArtifactNotFoundException {
artifactDiscoveryService.discoverArtifact(artifact);
}

@Test
Copy link
Member

@lasanthak lasanthak Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this test to be correct shouldn't we remove "throws ArtifactNotFoundException" from the signature?

public void testDiscoverArtifact_WhenContinueOnMavenError_IsTrue() throws ArtifactNotFoundException {
final MavenArtifactDiscoveryService artifactDiscoveryService2 = new MavenArtifactDiscoveryService(
new JarvizConfig.Builder()
.artifactDirectory("/tmp/jarviz/artifacts")
.continueOnMavenError(true)
.build());
final Artifact artifact = new Artifact.Builder()
.groupId("__my_invalid_group__")
.artifactId("__my_invalid_artifact__")
.version("0")
.build();
final File file = artifactDiscoveryService2.discoverArtifact(artifact);
assertThat(file).doesNotExist();
}
}