Skip to content

Commit

Permalink
🔊 improved logging structure with more info pops to the user in case …
Browse files Browse the repository at this point in the history
…of not-complete dependency tree
  • Loading branch information
noyshabtay committed Nov 23, 2023
1 parent 9805bf6 commit 9d3788e
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 35 deletions.
31 changes: 22 additions & 9 deletions src/main/java/com/jfrog/ide/common/yarn/YarnDriver.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectReader;
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.apache.commons.lang3.StringUtils;
import org.jfrog.build.api.util.Log;
import org.jfrog.build.api.util.NullLog;
import org.jfrog.build.extractor.executor.CommandExecutor;
import org.jfrog.build.extractor.executor.CommandResults;

Expand All @@ -20,9 +21,16 @@
public class YarnDriver {
private static final ObjectReader jsonReader = new ObjectMapper().reader();
private final CommandExecutor commandExecutor;
private final Log log;

public YarnDriver(Map<String, String> env) {
this.commandExecutor = new CommandExecutor("yarn", env);
this.log = new NullLog();
}

public YarnDriver(Map<String, String> env, Log log) {
this.commandExecutor = new CommandExecutor("yarn", env);
this.log = log;
}

@SuppressWarnings("unused")
Expand Down Expand Up @@ -50,8 +58,9 @@ public JsonNode list(File workingDirectory, List<String> extraArgs) throws IOExc
String res = StringUtils.isBlank(commandRes.getRes()) ? "{}" : commandRes.getRes();
JsonNode jsonResults = jsonReader.readTree(res);

if (!commandRes.isOk() && !jsonResults.has("problems")) {
((ObjectNode) jsonResults).put("problems", commandRes.getErr());
if (!commandRes.isOk()) {
log.error("Errors occurred during Yarn list command. " +
"The dependency tree may be incomplete:\n" + commandRes.getErr());
}
return jsonResults;
} catch (IOException | InterruptedException e) {
Expand All @@ -76,17 +85,21 @@ public JsonNode[] why(File workingDirectory, String componentName) throws IOExce
String[] args = {"why", componentName, "--json", "--no-progress"};
try {
CommandResults commandRes = runCommand(workingDirectory, args);
String res = StringUtils.isBlank(commandRes.getRes()) ? "{}" : commandRes.getRes();

// Note that although the command may succeed (commandRes.isOk() == true), the result may still contain errors (such as no match found)
String err = commandRes.getErr();
if (!StringUtils.isBlank(err)) {
log.error("Errors occurred during Yarn why command for dependency '" + componentName + "'. " +
"The dependency tree may be incomplete:\n" + err);
return new JsonNode[0];
}

String res = commandRes.getRes();
String[] splittedResults = res.split("\n");
JsonNode[] yarnWhyResults = new JsonNode[splittedResults.length];
for (int i = 0; i < splittedResults.length; i++) {
yarnWhyResults[i] = jsonReader.readTree(splittedResults[i]);
}
// note that although the command may succeed (commandRes.isOk() == true), the result may still contain errors (such as no match found)
String err = commandRes.getErr();
if (!StringUtils.isBlank(err)) {
((ObjectNode) yarnWhyResults[0]).put("problems", commandRes.getErr());
}

return yarnWhyResults;

Expand Down
21 changes: 6 additions & 15 deletions src/main/java/com/jfrog/ide/common/yarn/YarnTreeBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,26 @@ public class YarnTreeBuilder {
private final YarnDriver yarnDriver;
private final Path projectDir;
private final String descriptorFilePath;
private final Log log;

public YarnTreeBuilder(Path projectDir, String descriptorFilePath, Map<String, String> env) {
public YarnTreeBuilder(Path projectDir, String descriptorFilePath, Map<String, String> env, Log log) {
this.projectDir = projectDir;
this.descriptorFilePath = descriptorFilePath;
this.yarnDriver = new YarnDriver(env);
this.yarnDriver = new YarnDriver(env, log);
this.log = log;
}

/**
* Build the yarn dependency tree.
*
* @param logger - The logger.
* @return full dependency tree without Xray scan results.
* @throws IOException in case of I/O error.
*/
public DepTree buildTree(Log logger) throws IOException {
public DepTree buildTree() throws IOException {
if (!yarnDriver.isYarnInstalled()) {
throw new IOException("Could not scan Yarn project dependencies, because Yarn is not in the PATH.");
}
JsonNode listResults = yarnDriver.list(projectDir.toFile());
if (listResults.get("problems") != null) {
logger.warn("Errors occurred during building the yarn dependency tree. " +
"The dependency tree may be incomplete:\n" + listResults.get("problems").toString());
}
return buildYarnDependencyTree(listResults);
}

Expand Down Expand Up @@ -162,21 +159,15 @@ List<List<String>> extractMultiplePaths(String projectRootId, String packageFull
* Example 2 (List):
* {"type":"list","data":{"type":"reasons","items":["Specified in \"dependencies\"","Hoisted from \"jest-cli#node-notifier#minimist\"","Hoisted from \"jest-cli#sane#minimist\""]}}
*
* @param logger - The logger.
* @param projectRootId - The name of the project to display in the root of the impact tree.
* @param packageName - The package name (without version).
* Example: "minimist".
* @param packageVersions - The package versions.
* @return A map of package full name (<NAME>:<VERSION>) to a list of dependency paths.
* @throws IOException in case of I/O error returned from the running "yarn why" command in the yarnDriver.
*/
public Map<String, List<List<String>>> findDependencyImpactPaths(Log logger, String projectRootId, String packageName, Set<String> packageVersions) throws IOException {
public Map<String, List<List<String>>> findDependencyImpactPaths(String projectRootId, String packageName, Set<String> packageVersions) throws IOException {
JsonNode[] yarnWhyItem = yarnDriver.why(projectDir.toFile(), packageName);
if (yarnWhyItem[0].has("problems")) {
logger.warn("Errors occurred during building the Yarn dependency tree. " +
"The dependency tree may be incomplete:\n" + yarnWhyItem[0].get("problems").toString());

}

// Parse "yarn why" results and generate the dependency paths
String packageFullName = packageName;
Expand Down
8 changes: 1 addition & 7 deletions src/test/java/com/jfrog/ide/common/yarn/YarnDriverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,6 @@ public void yarnWhyDependencyTest(Project project, String componentName) {
assertNotNull(whyResults);
assertTrue(whyResults.length > 0);

for (JsonNode result : whyResults) {
assertNotNull(result);
assertFalse(result.has("problems"), "Unexpected problems in yarn why command result:\n" + result.get("problems"));
}
} catch (IOException e) {
fail("Exception during yarn why command: " + e.getMessage(), e);
}
Expand All @@ -86,9 +82,7 @@ public void yarnWhyEmptyTest(Project project, String componentName) {
try {
JsonNode[] whyResults = yarnDriver.why(tempProject, componentName);
assertNotNull(whyResults);
assertTrue(whyResults.length > 0);

assertTrue(whyResults[0].has("problems"), "Yarn why command result should contain problems:\n" + whyResults[0].toString());
assertFalse(whyResults.length > 0);

} catch (IOException e) {
fail("Exception during yarn why command: " + e.getMessage(), e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ private YarnTreeBuilder createYarnTreeBuilder(Project project) throws IOExceptio
FileUtils.copyDirectory((project).path.toFile(), tempProject);
Path projectDir = tempProject.toPath();
descriptorFilePath = projectDir.resolve("package.json").toString();
return new YarnTreeBuilder(projectDir, descriptorFilePath, null);
return new YarnTreeBuilder(projectDir, descriptorFilePath, null, new NullLog());
}

@Test(dataProvider = "yarnTreeBuilderProvider")
public void yarnTreeBuilderTest(Project project, int expectedChildren) throws IOException {
YarnTreeBuilder yarnTreeBuilder = createYarnTreeBuilder(project);
depTree = yarnTreeBuilder.buildTree(new NullLog());
depTree = yarnTreeBuilder.buildTree();
assertNotNull(depTree);
String expectedProjectName = project.name;
checkDependencyTree(expectedProjectName, expectedChildren);
Expand Down Expand Up @@ -142,7 +142,7 @@ public void extractMultiplePathsTest() {
List.of(projectRootId, "jest-cli", "istanbul-api", "mkdirp", packageFullName),
List.of(projectRootId, packageFullName)
);
YarnTreeBuilder yarnTreeBuilder = new YarnTreeBuilder(Paths.get(""), "", null);
YarnTreeBuilder yarnTreeBuilder = new YarnTreeBuilder(Paths.get(""), "", null, new NullLog());
List<List<String>> paths = yarnTreeBuilder.extractMultiplePaths(projectRootId, packageFullName, rawDependencyPaths);

assertNotNull(paths);
Expand All @@ -166,7 +166,7 @@ public void findDependencyImpactPathsTest(Project project, String packageName, S
String projectRootId = project.name;

YarnTreeBuilder yarnTreeBuilder = createYarnTreeBuilder(project);
Map<String, List<List<String>>> pathsMap = yarnTreeBuilder.findDependencyImpactPaths(new NullLog(), projectRootId, packageName, packageVersions);
Map<String, List<List<String>>> pathsMap = yarnTreeBuilder.findDependencyImpactPaths(projectRootId, packageName, packageVersions);

assertNotNull(pathsMap);
for (String packageVersion : packageVersions) {
Expand Down

0 comments on commit 9d3788e

Please sign in to comment.