Skip to content

Commit

Permalink
FIX (PMD): @W-15708421@: Read PMD results from outfile instead of std…
Browse files Browse the repository at this point in the history
…out.
  • Loading branch information
jfeingold35 committed Jul 24, 2024
1 parent 9d9c90d commit cb5c1c0
Show file tree
Hide file tree
Showing 15 changed files with 131 additions and 107 deletions.
8 changes: 2 additions & 6 deletions sfge/src/main/java/com/salesforce/cli/CacheCreator.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
package com.salesforce.cli;

import java.io.File;
import com.salesforce.config.SfgeConfigProvider;
import java.io.FileWriter;
import java.io.IOException;
import java.nio.file.Path;

import com.salesforce.config.SfgeConfig;
import com.salesforce.config.SfgeConfigProvider;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

Expand All @@ -32,8 +29,7 @@ public CacheCreator() {
public void create(Result result) {
if (SfgeConfigProvider.get().isCachingDisabled()) {
if (LOGGER.isInfoEnabled()) {
LOGGER.info(
"Skipping to cache information since it has been disabled.");
LOGGER.info("Skipping to cache information since it has been disabled.");
}
return;
}
Expand Down
6 changes: 3 additions & 3 deletions sfge/src/main/java/com/salesforce/cli/FilesToEntriesMap.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package com.salesforce.cli;

import com.google.common.annotations.VisibleForTesting;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;

import com.google.common.annotations.VisibleForTesting;
import org.json.simple.JSONArray;
import org.json.simple.JSONObject;

Expand Down Expand Up @@ -109,7 +108,8 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Entry entry = (Entry) o;
return Objects.equals(filename, entry.filename) && Objects.equals(methodName, entry.methodName);
return Objects.equals(filename, entry.filename)
&& Objects.equals(methodName, entry.methodName);
}

@Override
Expand Down
13 changes: 7 additions & 6 deletions sfge/src/main/java/com/salesforce/config/EnvUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

import com.google.common.annotations.VisibleForTesting;
import com.salesforce.graph.ops.registry.RegistryDataLimitCalculator;

import java.io.File;
import java.nio.file.Files;
import java.util.concurrent.TimeUnit;

public final class EnvUtil {
Expand All @@ -16,7 +14,8 @@ public final class EnvUtil {
private static final String ENV_PROGRESS_INCREMENTS = "SFGE_PROGRESS_INCREMENTS";
private static final String ENV_STACK_DEPTH_LIMIT = "SFGE_STACK_DEPTH_LIMIT";
private static final String ENV_PATH_EXPANSION_LIMIT = "SFGE_PATH_EXPANSION_LIMIT";
private static final String ENV_FILES_TO_ENTRIES_CACHE_LOCATION = "SFGE_FILES_TO_ENTRIES_CACHE_LOCATION";
private static final String ENV_FILES_TO_ENTRIES_CACHE_LOCATION =
"SFGE_FILES_TO_ENTRIES_CACHE_LOCATION";
private static final String ENV_DISABLE_CACHING = "SFGE_DISABLE_CACHING";

// TODO: These should move to SfgeConfigImpl and this class should return Optionals
Expand All @@ -39,11 +38,12 @@ public final class EnvUtil {
static final int DEFAULT_PATH_EXPANSION_LIMIT =
RegistryDataLimitCalculator.getApexPathExpanderRegistryLimit();

@VisibleForTesting static final String DEFAULT_FILES_TO_ENTRIES_CACHE_LOCATION = ".sfge-cache" + File.separator + "fileToEntryMapData.json";
@VisibleForTesting
static final String DEFAULT_FILES_TO_ENTRIES_CACHE_LOCATION =
".sfge-cache" + File.separator + "fileToEntryMapData.json";

@VisibleForTesting static final boolean DEFAULT_DISABLE_CACHING = true;


/**
* Returns the value of the {@link #ENV_RULE_THREAD_COUNT} environment variable if set, else
* {@link #DEFAULT_RULE_THREAD_COUNT}. Should be used to set the number of threads that can be
Expand Down Expand Up @@ -111,7 +111,8 @@ static int getPathExpansionLimit() {
}

static String getFilesToEntriesCacheLocation() {
return getStringOrDefault(ENV_FILES_TO_ENTRIES_CACHE_LOCATION, DEFAULT_FILES_TO_ENTRIES_CACHE_LOCATION);
return getStringOrDefault(
ENV_FILES_TO_ENTRIES_CACHE_LOCATION, DEFAULT_FILES_TO_ENTRIES_CACHE_LOCATION);
}

static boolean isCachingDisabled() {
Expand Down
4 changes: 2 additions & 2 deletions sfge/src/main/java/com/salesforce/config/SfgeConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ public interface SfgeConfig {
/** Limit to control the growth of path expansion to help alleviate OutOfMemoryError. */
int getPathExpansionLimit();

/** Filename of data that stores Files to Entries **/
/** Filename of data that stores Files to Entries * */
String getFilesToEntriesCacheLocation();

/** Indicates if caching should be disabled in the current run **/
/** Indicates if caching should be disabled in the current run * */
boolean isCachingDisabled();
}
1 change: 0 additions & 1 deletion sfge/src/main/java/com/salesforce/graph/ApexPath.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.salesforce.graph;

import com.google.common.collect.ImmutableList;
import com.salesforce.Collectible;
import com.salesforce.NullCollectible;
import com.salesforce.collections.CollectionUtil;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@
import com.salesforce.graph.ApexPath;
import com.salesforce.graph.ops.expander.PathExpansionObserver;
import com.salesforce.graph.vertex.MethodVertex;

import java.util.Collections;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;

/**
* Tracks apex class files that were encountered while expanding paths.
*/
/** Tracks apex class files that were encountered while expanding paths. */
public class TaintedFileTracker implements PathExpansionObserver {
final HashSet<String> taintedFiles;

Expand All @@ -30,5 +27,4 @@ public void onPathVisit(ApexPath path) {
public Set<String> getTaintedFiles() {
return Collections.unmodifiableSet(taintedFiles);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ public Result runRules() {
return result;
}

private void extractFileEntryMapping(TaintedFileTracker taintedFileTracker, MethodVertex entryMethod, Result result) {
private void extractFileEntryMapping(
TaintedFileTracker taintedFileTracker, MethodVertex entryMethod, Result result) {
final String entryFile = entryMethod.getFileName();
final String entryMethodName = entryMethod.getName();

Expand Down Expand Up @@ -239,12 +240,14 @@ private ApexPathRetrievalSummary getPathSummary(ApexPathExpanderConfig expanderC
return ApexPathUtil.summarizeForwardPaths(g, methodVertex, expanderConfig);
}

private ApexPathExpanderConfig getApexPathExpanderConfig(TaintedFileTracker taintedFileTracker) {
private ApexPathExpanderConfig getApexPathExpanderConfig(
TaintedFileTracker taintedFileTracker) {
ApexPathExpanderConfig.Builder expanderConfigBuilder =
ApexPathUtil.getFullConfiguredPathExpanderConfigBuilder();
// Add default PathExpansionObservers

// We need a new instance of TaintedFileTracker since the tainted files are unique to each path expansion.
// We need a new instance of TaintedFileTracker since the tainted files are unique to each
// path expansion.
expanderConfigBuilder = expanderConfigBuilder.withPathExpansionObserver(taintedFileTracker);

// Add observers that the rules request
Expand Down
54 changes: 29 additions & 25 deletions sfge/src/test/java/com/salesforce/cli/CacheCreatorTest.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
package com.salesforce.cli;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.*;

import com.salesforce.config.SfgeConfigTestProvider;
import com.salesforce.config.TestSfgeConfig;
import java.io.File;
import java.io.IOException;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
Expand All @@ -12,31 +18,27 @@
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;

import java.io.File;
import java.io.IOException;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.*;

public class CacheCreatorTest {
private static final String DUMMY_FILE_NAME = ".test-sfge-cache" + File.separator + "myFileToEntriesDummy.json";
private static final String DUMMY_FILE_NAME =
".test-sfge-cache" + File.separator + "myFileToEntriesDummy.json";
@Mock CacheCreator.Dependencies dependencies;

AutoCloseable openMocks;

@BeforeEach
void setUp() {
openMocks = MockitoAnnotations.openMocks(this);
SfgeConfigTestProvider.set(new TestSfgeConfig() {
@Override
public String getFilesToEntriesCacheLocation() {
return DUMMY_FILE_NAME;
}
public boolean isCachingDisabled() {
return false;
}
});
SfgeConfigTestProvider.set(
new TestSfgeConfig() {
@Override
public String getFilesToEntriesCacheLocation() {
return DUMMY_FILE_NAME;
}

public boolean isCachingDisabled() {
return false;
}
});
}

@AfterEach
Expand All @@ -57,12 +59,13 @@ public void testFilesToEntriesDataCreationForFileCreation() throws IOException {

String actualFilename = fileNameCaptor.getValue();
MatcherAssert.assertThat(actualFilename, Matchers.containsString(DUMMY_FILE_NAME));

}

@Test
public void testIOExceptionDoesNotDisrupt() throws IOException {
doThrow(new IOException("dummy exception")).when(dependencies).writeFile(anyString(), anyString());
doThrow(new IOException("dummy exception"))
.when(dependencies)
.writeFile(anyString(), anyString());

Result result = new Result();
CacheCreator cacheCreator = new CacheCreator(dependencies);
Expand All @@ -74,12 +77,13 @@ public void testIOExceptionDoesNotDisrupt() throws IOException {
public void testNoFileWrittenWhenCachingIsDisabled() throws IOException {

// Disable caching
SfgeConfigTestProvider.set(new TestSfgeConfig() {
@Override
public boolean isCachingDisabled() {
return true;
}
});
SfgeConfigTestProvider.set(
new TestSfgeConfig() {
@Override
public boolean isCachingDisabled() {
return true;
}
});

try {
Result result = new Result();
Expand Down
27 changes: 15 additions & 12 deletions sfge/src/test/java/com/salesforce/cli/FilesToEntriesMapTest.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
package com.salesforce.cli;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;

import java.util.HashMap;
import java.util.HashSet;
import org.hamcrest.Matchers;
import org.json.simple.JSONArray;
import org.json.simple.JSONObject;
import org.json.simple.parser.JSONParser;
import org.json.simple.parser.ParseException;
import org.junit.jupiter.api.Test;

import java.util.HashMap;
import java.util.HashSet;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.*;

public class FilesToEntriesMapTest {

private static final String FILE_1 = "file1";
Expand All @@ -24,7 +23,6 @@ public class FilesToEntriesMapTest {
private static final String ENTRY_METHOD_2 = "entryMethod2";
private static final String ENTRY_METHOD_3 = "entryMethod3";


@Test
public void testEntryCreation() {
FilesToEntriesMap filesToEntriesMap = new FilesToEntriesMap();
Expand All @@ -36,7 +34,11 @@ public void testEntryCreation() {
assertThat(actualMap.keySet(), containsInAnyOrder(FILE_1, FILE_2));

HashSet<FilesToEntriesMap.Entry> entries1 = actualMap.get(FILE_1);
assertThat(entries1, containsInAnyOrder(new FilesToEntriesMap.Entry(ENTRY_FILE_1, ENTRY_METHOD_1), new FilesToEntriesMap.Entry(ENTRY_FILE_2, ENTRY_METHOD_3)));
assertThat(
entries1,
containsInAnyOrder(
new FilesToEntriesMap.Entry(ENTRY_FILE_1, ENTRY_METHOD_1),
new FilesToEntriesMap.Entry(ENTRY_FILE_2, ENTRY_METHOD_3)));

HashSet<FilesToEntriesMap.Entry> entries2 = actualMap.get(FILE_2);
assertThat(entries2, contains(new FilesToEntriesMap.Entry(ENTRY_FILE_1, ENTRY_METHOD_2)));
Expand All @@ -57,8 +59,11 @@ public void testMerge_appendingToExistingFilename() {
HashMap<String, HashSet<FilesToEntriesMap.Entry>> actualMap = filesToEntriesMap1.getMap();

HashSet<FilesToEntriesMap.Entry> entries2 = actualMap.get(FILE_2);
assertThat(entries2, contains(new FilesToEntriesMap.Entry(ENTRY_FILE_1, ENTRY_METHOD_2), new FilesToEntriesMap.Entry(ENTRY_FILE_2, ENTRY_METHOD_3)));

assertThat(
entries2,
contains(
new FilesToEntriesMap.Entry(ENTRY_FILE_1, ENTRY_METHOD_2),
new FilesToEntriesMap.Entry(ENTRY_FILE_2, ENTRY_METHOD_3)));
}

@Test
Expand Down Expand Up @@ -99,7 +104,5 @@ public void testJsonString() throws ParseException {
assertThat(file2Data.get(FilesToEntriesMap.FIELD_FILENAME), equalTo(FILE_2));
JSONArray entries2 = (JSONArray) file2Data.get(FilesToEntriesMap.FIELD_ENTRIES);
assertThat(entries2.size(), equalTo(1));


}
}
Loading

0 comments on commit cb5c1c0

Please sign in to comment.