Skip to content

Commit

Permalink
SONARPHP-1344 Do not try to cache file hashes when cache is disabled (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
nils-werner-sonarsource authored Jan 12, 2023
1 parent bfebc3f commit 3ddc5a0
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 16 deletions.
14 changes: 10 additions & 4 deletions php-frontend/src/main/java/org/sonar/php/cache/Cache.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ public void writeFileSymbolTable(InputFile file, SymbolTableImpl symbolTable) {
}

public void writeFileContentHash(InputFile file, byte[] hash) {
String cacheKey = cacheKey(CONTENT_HASHES_KEY, file.key());
cacheContext.getWriteCache().writeBytes(cacheKey, hash);
if (cacheContext.isCacheEnabled()) {
String cacheKey = cacheKey(CONTENT_HASHES_KEY, file.key());
cacheContext.getWriteCache().writeBytes(cacheKey, hash);
}
}

@CheckForNull
Expand All @@ -66,9 +68,13 @@ public SymbolTableImpl read(InputFile file) {
return null;
}

@CheckForNull
public byte[] readFileContentHash(InputFile file) {
String cacheKey = cacheKey(CONTENT_HASHES_KEY, file.key());
return cacheContext.getReadCache().readBytes(cacheKey);
if (cacheContext.isCacheEnabled()) {
String cacheKey = cacheKey(CONTENT_HASHES_KEY, file.key());
return cacheContext.getReadCache().readBytes(cacheKey);
}
return null;
}

private static String cacheKey(String prefix, String file) {
Expand Down
26 changes: 22 additions & 4 deletions php-frontend/src/test/java/org/sonar/php/cache/CacheTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ public void shouldReturnNullWhenCacheDisabled() {
}

@Test
public void readFileContentHash() {
CacheContext context = new CacheContextImpl(true, writeCache, readCache, PLUGIN_VERSION);
public void readFileContentHashWhenCacheIsEnabled() {
CacheContext context = new CacheContextImpl(true, null, readCache, PLUGIN_VERSION);
Cache cache = new Cache(context);
byte[] hash = "hash".getBytes();

Expand All @@ -142,15 +142,33 @@ public void readFileContentHash() {
}

@Test
public void writeFileContentHash() {
CacheContext context = new CacheContextImpl(true, writeCache, readCache, PLUGIN_VERSION);
public void readFileContentHashWhenCacheIsDisabled() {
CacheContext context = new CacheContextImpl(false, null, readCache, PLUGIN_VERSION);
Cache cache = new Cache(context);

assertThat(cache.readFileContentHash(DEFAULT_INPUT_FILE)).isNull();
}

@Test
public void writeFileContentHashWhenCacheIsEnabled() {
CacheContext context = new CacheContextImpl(true, writeCache, null, PLUGIN_VERSION);
Cache cache = new Cache(context);
byte[] hash = "hash".getBytes();
cache.writeFileContentHash(DEFAULT_INPUT_FILE, hash);

verify(writeCache).writeBytes(CACHE_KEY_HASH, hash);
}

@Test
public void writeFileContentHashWhenCacheIsDisabled() {
CacheContext context = new CacheContextImpl(false, writeCache, null, PLUGIN_VERSION);
Cache cache = new Cache(context);
byte[] hash = "hash".getBytes();
cache.writeFileContentHash(DEFAULT_INPUT_FILE, hash);

verifyNoInteractions(writeCache);
}

void warmupReadCache(SymbolTableImpl data) {
SymbolTableSerializationInput serializationInput = new SymbolTableSerializationInput(data, PLUGIN_VERSION);
SerializationResult serializationData = SymbolTableSerializer.toBinary(serializationInput);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.security.NoSuchAlgorithmException;
import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -100,7 +101,7 @@ public class PHPSensorTest {
@org.junit.Rule
public LogTester logTester = new LogTester();

private final SensorContextTester context = SensorContextTester.create(new File("src/test/resources").getAbsoluteFile());
private SensorContextTester context = SensorContextTester.create(new File("src/test/resources").getAbsoluteFile());

private CheckFactory checkFactory = new CheckFactory(mock(ActiveRules.class));

Expand All @@ -122,6 +123,8 @@ public class PHPSensorTest {

private final Set<File> tempReportFiles = new HashSet<>();

private Path tmpFolderPath;

@org.junit.Rule
public TemporaryFolder tmpFolder = new TemporaryFolder();

Expand Down Expand Up @@ -157,10 +160,16 @@ public boolean scanWithoutParsing(PhpInputFileContext phpInputFileContext) {

@Before
public void before() throws IOException {
context.fileSystem().setWorkDir(tmpFolder.newFolder().toPath());
tmpFolderPath = tmpFolder.newFolder().toPath();
resetContext();
disableCache();
}

private void resetContext() {
context = SensorContextTester.create(new File("src/test/resources").getAbsoluteFile());
context.fileSystem().setWorkDir(tmpFolderPath);
}

private void disableCache() {
context.setCacheEnabled(false);
context.setCanSkipUnchangedFiles(false);
Expand Down Expand Up @@ -689,8 +698,8 @@ public void should_use_test_file_checks() {
@Test
public void should_not_analyze_unchanged_file_if_setting_is_enabled() {
checkFactory = new CheckFactory(getActiveRules());
context.fileSystem().add(inputFile(ANALYZED_FILE, Type.MAIN, InputFile.Status.SAME));
context.setCanSkipUnchangedFiles(true);
InputFile inputFile = inputFile(ANALYZED_FILE, Type.MAIN, InputFile.Status.SAME);
analyzeBaseCommit(inputFile);
createSensor().execute(context);
assertThat(context.allIssues()).isEmpty();
}
Expand All @@ -717,8 +726,11 @@ public void should_analyze_changed_file_if_setting_is_enabled() {
@Test
public void should_not_analyze_unchanged_file_if_enabled_by_property() {
checkFactory = new CheckFactory(getActiveRules());
context.fileSystem().add(inputFile(ANALYZED_FILE, Type.MAIN, InputFile.Status.SAME));
context.setSettings(new MapSettings().setProperty("sonar.php.skipUnchanged", "true"));

InputFile inputFile = inputFile(ANALYZED_FILE, Type.MAIN, InputFile.Status.SAME);
analyzeBaseCommit(inputFile);
context.fileSystem().add(inputFile);

createSensor().execute(context);
assertThat(context.allIssues()).isEmpty();
}
Expand All @@ -741,8 +753,10 @@ public void should_not_raise_issue_on_same_file_when_no_check_requires_parsing()
.build();
checkFactory = new CheckFactory(rules);

context.fileSystem().add(inputFile(ANALYZED_FILE, Type.MAIN, InputFile.Status.SAME));
context.setSettings(new MapSettings().setProperty("sonar.php.skipUnchanged", "true"));
InputFile inputFile = inputFile(ANALYZED_FILE, Type.MAIN, InputFile.Status.SAME);
analyzeBaseCommit(inputFile);

context.fileSystem().add(inputFile);
createSensor().execute(context);
assertThat(context.allIssues()).isEmpty();
}
Expand Down Expand Up @@ -836,6 +850,17 @@ private void createReportWithAbsolutePath(String generatedReportRelativePath, St
}
}

private void analyzeBaseCommit(InputFile inputFile) {
enableCache();
context.fileSystem().add(inputFile);
context.setCanSkipUnchangedFiles(true);
createSensor().execute(context);
resetContext();
context.setCacheEnabled(true);
context.setCanSkipUnchangedFiles(true);
setCacheFromPreviousAnalysis();
}

private static class ExceptionRaisingCheck extends PHPVisitorCheck {

private final RuntimeException exception;
Expand Down

0 comments on commit 3ddc5a0

Please sign in to comment.