SONARPHP-1344 Verify file status by file hashes before restoring from…

… cache

php-frontend/src/main/java/org/sonar/php/cache/Cache.java

@@ -29,6 +29,7 @@ public class Cache {
 
   public static final String DATA_CACHE_PREFIX = "php.projectSymbolData.data:";
   public static final String STRING_TABLE_CACHE_PREFIX = "php.projectSymbolData.stringTable:";
+  public static final String CONTENT_HASHES_KEY = "php.contentHashes:";
   private final CacheContext cacheContext;
   private final String pluginVersion;
 
@@ -37,7 +38,7 @@ public Cache(CacheContext cacheContext) {
     this.pluginVersion = cacheContext.pluginVersion();
   }
 
-  public void write(InputFile file, SymbolTableImpl symbolTable) {
+  public void writeFileSymbolTable(InputFile file, SymbolTableImpl symbolTable) {
     if (cacheContext.isCacheEnabled()) {
       SymbolTableSerializationInput serializationInput = new SymbolTableSerializationInput(symbolTable, pluginVersion);
       SerializationResult serializationData = SymbolTableSerializer.toBinary(serializationInput);
@@ -48,6 +49,11 @@ public void write(InputFile file, SymbolTableImpl symbolTable) {
     }
   }
 
+  public void writeFileContentHash(InputFile file, byte[] hash) {
+    String cacheKey = cacheKey(CONTENT_HASHES_KEY, file.key());
+    cacheContext.getWriteCache().writeBytes(cacheKey, hash);
+  }
+
   @CheckForNull
   public SymbolTableImpl read(InputFile file) {
     if (cacheContext.isCacheEnabled()) {
@@ -60,7 +66,13 @@ public SymbolTableImpl read(InputFile file) {
     return null;
   }
 
+  public byte[] readFileContentHash(InputFile file) {
+    String cacheKey = cacheKey(CONTENT_HASHES_KEY, file.key());
+    return cacheContext.getReadCache().readBytes(cacheKey);
+  }
+
   private static String cacheKey(String prefix, String file) {
     return prefix + file.replace('\\', '/');
   }
+
 }

php-frontend/src/test/java/org/sonar/php/cache/CacheTest.java

@@ -34,14 +34,15 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 public class CacheTest {
 
   private static final InputFile DEFAULT_INPUT_FILE = inputFile("default");
   private static final String CACHE_KEY_DATA = "php.projectSymbolData.data:" + DEFAULT_INPUT_FILE.key();
   private static final String CACHE_KEY_STRING_TABLE = "php.projectSymbolData.stringTable:" + DEFAULT_INPUT_FILE.key();
+  private static final String CACHE_KEY_HASH = "php.contentHashes:" + DEFAULT_INPUT_FILE.key();
   private static final String PLUGIN_VERSION = "1.2.3";
 
   private final PhpWriteCache writeCache = mock(PhpWriteCache.class);
@@ -52,7 +53,7 @@ public void shouldWriteToCacheOnlyIfItsEnabled() {
     CacheContext context = new CacheContextImpl(true, writeCache, readCache, PLUGIN_VERSION);
     Cache cache = new Cache(context);
     SymbolTableImpl data = exampleSymbolTable();
-    cache.write(DEFAULT_INPUT_FILE, data);
+    cache.writeFileSymbolTable(DEFAULT_INPUT_FILE, data);
 
     SerializationResult binary = SymbolTableSerializer.toBinary(new SymbolTableSerializationInput(data, PLUGIN_VERSION));
 
@@ -66,9 +67,9 @@ public void shouldNotWriteToCacheIfItsDisabled() {
     Cache cache = new Cache(context);
     SymbolTableImpl data = emptySymbolTable();
 
-    cache.write(DEFAULT_INPUT_FILE, data);
+    cache.writeFileSymbolTable(DEFAULT_INPUT_FILE, data);
 
-    verifyZeroInteractions(writeCache);
+    verifyNoInteractions(writeCache);
   }
 
   @Test
@@ -129,6 +130,27 @@ public void shouldReturnNullWhenCacheDisabled() {
     assertThat(actual).isNull();
   }
 
+  @Test
+  public void readFileContentHash() {
+    CacheContext context = new CacheContextImpl(true, writeCache, readCache, PLUGIN_VERSION);
+    Cache cache = new Cache(context);
+    byte[] hash = "hash".getBytes();
+
+    when(readCache.readBytes(CACHE_KEY_HASH)).thenReturn(hash);
+
+    assertThat(cache.readFileContentHash(DEFAULT_INPUT_FILE)).isEqualTo(hash);
+  }
+
+  @Test
+  public void writeFileContentHash() {
+    CacheContext context = new CacheContextImpl(true, writeCache, readCache, PLUGIN_VERSION);
+    Cache cache = new Cache(context);
+    byte[] hash = "hash".getBytes();
+    cache.writeFileContentHash(DEFAULT_INPUT_FILE, hash);
+
+    verify(writeCache).writeBytes(CACHE_KEY_HASH, hash);
+  }
+
   void warmupReadCache(SymbolTableImpl data) {
     SymbolTableSerializationInput serializationInput = new SymbolTableSerializationInput(data, PLUGIN_VERSION);
     SerializationResult serializationData = SymbolTableSerializer.toBinary(serializationInput);

pom.xml

@@ -68,7 +68,7 @@
   </issueManagement>
 
   <properties>
-    <revision>3.27.0-SNAPSHOT</revision>
+    <revision>3.27.1-SNAPSHOT</revision>
     <version.enforcer.plugin>3.0.0-M3</version.enforcer.plugin>
     <gitRepositoryName>sonar-php</gitRepositoryName>
     <license.title>SonarQube PHP Plugin</license.title>
@@ -82,7 +82,7 @@
     <version.commons-lang>3.10</version.commons-lang>
     <version.junit>4.13.1</version.junit>
     <version.maven-project>2.0.6</version.maven-project>
-    <version.mockito>2.21.0</version.mockito>
+    <version.mockito>4.11.0</version.mockito>
     <version.sonar>9.7.1.62043</version.sonar>
     <version.staxmate>2.0.1</version.staxmate>
     <version.sonar-orchestrator>3.40.0.183</version.sonar-orchestrator>
@@ -210,6 +210,12 @@
         <version>${version.mockito}</version>
         <scope>test</scope>
       </dependency>
+      <dependency>
+        <groupId>org.mockito</groupId>
+        <artifactId>mockito-inline</artifactId>
+        <version>${version.mockito}</version>
+        <scope>test</scope>
+      </dependency>
       <dependency>
         <groupId>org.assertj</groupId>
         <artifactId>assertj-core</artifactId>

sonar-php-plugin/pom.xml

@@ -76,6 +76,10 @@
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-inline</artifactId>
+    </dependency>
     <dependency>
       <groupId>org.codehaus.staxmate</groupId>
       <artifactId>staxmate</artifactId>

sonar-php-plugin/src/main/java/org/sonar/plugins/php/AnalysisScanner.java

@@ -41,6 +41,7 @@
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.php.PHPAnalyzer;
+import org.sonar.php.cache.Cache;
 import org.sonar.php.checks.ParsingErrorCheck;
 import org.sonar.php.checks.UncatchableExceptionCheck;
 import org.sonar.php.checks.utils.PhpUnitCheck;
@@ -82,7 +83,7 @@ public AnalysisScanner(SensorContext context,
                          ProjectSymbolData projectSymbolData,
                          DurationStatistics statistics,
                          CacheContext cacheContext) {
-    super(context, statistics);
+    super(context, statistics, new Cache(cacheContext));
     this.checks = checks;
     this.fileLinesContextFactory = fileLinesContextFactory;
     this.noSonarFilter = noSonarFilter;

sonar-php-plugin/src/main/java/org/sonar/plugins/php/FileHashingUtils.java

@@ -0,0 +1,40 @@
+/*
+ * SonarQube PHP Plugin
+ * Copyright (C) 2010-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.plugins.php;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import org.sonar.api.batch.fs.InputFile;
+
+public class FileHashingUtils {
+
+  private FileHashingUtils() {
+  }
+
+  private static final String HASH_ALGORITHM = "MD5";
+
+  public static byte[] inputFileContentHash(InputFile inputFile) throws IOException, NoSuchAlgorithmException {
+    byte[] contentBytes = inputFile.contents().getBytes(StandardCharsets.UTF_8);
+    MessageDigest messageDigest = MessageDigest.getInstance(HASH_ALGORITHM);
+    return messageDigest.digest(contentBytes);
+  }
+}

sonar-php-plugin/src/main/java/org/sonar/plugins/php/Scanner.java

@@ -19,6 +19,9 @@
  */
 package org.sonar.plugins.php;
 
+import java.io.IOException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
@@ -28,6 +31,7 @@
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.sonar.php.cache.Cache;
 import org.sonarsource.analyzer.commons.ProgressReport;
 
 abstract class Scanner {
@@ -42,12 +46,14 @@ abstract class Scanner {
   private static final String FAIL_FAST_PROPERTY_NAME = "sonar.internal.analysis.failFast";
   protected final SensorContext context;
   protected final DurationStatistics statistics;
+  protected final Cache cache;
   protected boolean optimizedAnalysis;
 
 
-  Scanner(SensorContext context, DurationStatistics statistics) {
+  Scanner(SensorContext context, DurationStatistics statistics, Cache cache) {
     this.context = context;
     this.statistics = statistics;
+    this.cache = cache;
     optimizedAnalysis = shouldOptimizeAnalysis();
   }
 
@@ -86,7 +92,23 @@ private boolean shouldOptimizeAnalysis() {
       (context.canSkipUnchangedFiles() || context.config().getBoolean(SONAR_CAN_SKIP_UNCHANGED_FILES_KEY).orElse(false));
   }
   protected boolean fileCanBeSkipped(InputFile file) {
-    return optimizedAnalysis && file.status() != null && file.status().equals(InputFile.Status.SAME);
+    return optimizedAnalysis && fileIsUnchanged(file);
+  }
+
+  private boolean fileIsUnchanged(InputFile inputFile) {
+    if (inputFile.status() != null && !inputFile.status().equals(InputFile.Status.SAME)) {
+      return false;
+    }
+    byte[] fileHash = cache.readFileContentHash(inputFile);
+    // InputFile.Status is not reliable in some cases
+    // We use the hash of the file's content to double-check the content is the same.
+    try {
+      byte[] bytes = FileHashingUtils.inputFileContentHash(inputFile);
+      return MessageDigest.isEqual(fileHash, bytes);
+    } catch (IOException | NoSuchAlgorithmException e) {
+      LOG.debug("Failed to compute content hash for file {}", inputFile.key());
+      return false;
+    }
   }
 
   private void processFile(InputFile file) {

sonar-php-plugin/src/main/java/org/sonar/plugins/php/SymbolScanner.java

@@ -21,6 +21,8 @@
 
 import com.sonar.sslr.api.RecognitionException;
 import com.sonar.sslr.api.typed.ActionParser;
+import java.io.IOException;
+import java.security.NoSuchAlgorithmException;
 import java.util.List;
 import javax.annotation.CheckForNull;
 import org.sonar.DurationStatistics;
@@ -44,12 +46,10 @@ public class SymbolScanner extends Scanner {
 
   private final ActionParser<Tree> parser = PHPParserBuilder.createParser();
   private final ProjectSymbolData projectSymbolData = new ProjectSymbolData();
-  private final Cache cache;
   private int symbolTablesFromCache = 0;
 
   SymbolScanner(SensorContext context, DurationStatistics statistics, Cache cache) {
-    super(context, statistics);
-    this.cache = cache;
+    super(context, statistics, cache);
   }
 
   @Override
@@ -90,7 +90,15 @@ void scanFile(InputFile file) {
     fileSymbolTable.classSymbolDatas().forEach(projectSymbolData::add);
     fileSymbolTable.functionSymbolDatas().forEach(projectSymbolData::add);
 
-    cache.write(file, fileSymbolTable);
+    byte[] contentHash;
+    try {
+      contentHash = FileHashingUtils.inputFileContentHash(file);
+    } catch (IOException | NoSuchAlgorithmException e) {
+      LOG.debug("Failed to compute content hash for file {}", file.key());
+      return;
+    }
+    cache.writeFileContentHash(file, contentHash);
+    cache.writeFileSymbolTable(file, fileSymbolTable);
   }
 
   @CheckForNull

sonar-php-plugin/src/test/java/org/sonar/plugins/php/FileHashingUtilsTest.java

@@ -0,0 +1,41 @@
+/*
+ * SonarQube PHP Plugin
+ * Copyright (C) 2010-2022 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.plugins.php;
+
+import java.io.File;
+import java.io.IOException;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import org.junit.Test;
+import org.sonar.api.batch.fs.InputFile;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.plugins.php.PhpTestUtils.inputFile;
+
+public class FileHashingUtilsTest {
+
+  @Test
+  public void hashing() throws IOException, NoSuchAlgorithmException {
+    InputFile file1 = inputFile("hash/main.php", InputFile.Type.MAIN, InputFile.Status.SAME);
+    InputFile file2 = inputFile("hash/modified.php", InputFile.Type.MAIN, InputFile.Status.CHANGED);
+    assertThat(MessageDigest.isEqual(FileHashingUtils.inputFileContentHash(file1), FileHashingUtils.inputFileContentHash(file1))).isTrue();
+    assertThat(MessageDigest.isEqual(FileHashingUtils.inputFileContentHash(file1), FileHashingUtils.inputFileContentHash(file2))).isFalse();
+  }
+}