Deprecated SerializedCpg APIs. (#1781)

This PR does not change behaviour. The serialization feature was internally not doing anything anymore for quite a while. Time to deprecated the API to eventually removed them.
ShiftLeftSecurity · Aug 2, 2024 · 5804e66 · 5804e66
1 parent 88d9845
commit 5804e66
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 59 deletions.
diff --git a/codepropertygraph/src/main/scala/io/shiftleft/SerializedCpg.scala b/codepropertygraph/src/main/scala/io/shiftleft/SerializedCpg.scala
@@ -7,6 +7,7 @@ import java.net.{URI, URISyntaxException}
 import java.nio.file.{FileSystem, FileSystems, Files}
 import java.util
 
+@deprecated("New API functions do not use this class anymore")
 class SerializedCpg extends AutoCloseable {
 
   /** We allow creating a dummy serialized CPG that does not do anything.

diff --git a/codepropertygraph/src/main/scala/io/shiftleft/passes/CpgPass.scala b/codepropertygraph/src/main/scala/io/shiftleft/passes/CpgPass.scala
@@ -62,7 +62,43 @@ abstract class ForkJoinParallelCpgPass[T <: AnyRef](cpg: Cpg, @nowarn outName: S
   // Override this to disable parallelism of passes. Useful for debugging.
   def isParallel: Boolean = true
 
-  override def createAndApply(): Unit = createApplySerializeAndStore(null)
+  override def createAndApply(): Unit = {
+    baseLogger.info(s"Start of pass: $name")
+    val nanosStart = System.nanoTime()
+    var nParts     = 0
+    var nanosBuilt = -1L
+    var nDiff      = -1
+    var nDiffT     = -1
+    try {
+      val diffGraph = Cpg.newDiffGraphBuilder
+      nParts = runWithBuilder(diffGraph)
+      nanosBuilt = System.nanoTime()
+      nDiff = diffGraph.size
+
+      // TODO how about `nDiffT` which seems to count the number of modifications..
+      //      nDiffT = overflowdb.BatchedUpdate
+      //        .applyDiff(cpg.graph, diffGraph, null)
+      //        .transitiveModifications()
+
+      flatgraph.DiffGraphApplier.applyDiff(cpg.graph, diffGraph)
+    } catch {
+      case exc: Exception =>
+        baseLogger.error(s"Pass ${name} failed", exc)
+        throw exc
+    } finally {
+      try {
+        finish()
+      } finally {
+        // the nested finally is somewhat ugly -- but we promised to clean up with finish(), we want to include finish()
+        // in the reported timings, and we must have our final log message if finish() throws
+        val nanosStop = System.nanoTime()
+        val fracRun   = if (nanosBuilt == -1) 0.0 else (nanosStop - nanosBuilt) * 100.0 / (nanosStop - nanosStart + 1)
+        baseLogger.info(
+          f"Pass $name completed in ${(nanosStop - nanosStart) * 1e-6}%.0f ms (${fracRun}%.0f%% on mutations). ${nDiff}%d + ${nDiffT - nDiff}%d changes committed from ${nParts}%d parts."
+        )
+      }
+    }
+  }
 
   override def runWithBuilder(externalBuilder: DiffGraphBuilder): Int = {
     try {
@@ -105,45 +141,9 @@ abstract class ForkJoinParallelCpgPass[T <: AnyRef](cpg: Cpg, @nowarn outName: S
     }
   }
 
+  @deprecated("Please use createAndApply")
   override def createApplySerializeAndStore(serializedCpg: SerializedCpg, prefix: String = ""): Unit = {
-    baseLogger.info(s"Start of pass: $name")
-    val nanosStart = System.nanoTime()
-    var nParts     = 0
-    var nanosBuilt = -1L
-    var nDiff      = -1
-    var nDiffT     = -1
-    try {
-      val diffGraph = Cpg.newDiffGraphBuilder
-      nParts = runWithBuilder(diffGraph)
-      nanosBuilt = System.nanoTime()
-      nDiff = diffGraph.size
-
-      // TODO how about `nDiffT` which seems to count the number of modifications..
-//      nDiffT = overflowdb.BatchedUpdate
-//        .applyDiff(cpg.graph, diffGraph, null)
-//        .transitiveModifications()
-
-      flatgraph.DiffGraphApplier.applyDiff(cpg.graph, diffGraph)
-    } catch {
-      case exc: Exception =>
-        baseLogger.error(s"Pass ${name} failed", exc)
-        throw exc
-    } finally {
-      try {
-        finish()
-      } finally {
-        // the nested finally is somewhat ugly -- but we promised to clean up with finish(), we want to include finish()
-        // in the reported timings, and we must have our final log message if finish() throws
-        val nanosStop = System.nanoTime()
-        val fracRun   = if (nanosBuilt == -1) 0.0 else (nanosStop - nanosBuilt) * 100.0 / (nanosStop - nanosStart + 1)
-        val serializationString = if (serializedCpg != null && !serializedCpg.isEmpty) {
-          " Diff serialized and stored."
-        } else ""
-        baseLogger.info(
-          f"Pass $name completed in ${(nanosStop - nanosStart) * 1e-6}%.0f ms (${fracRun}%.0f%% on mutations). ${nDiff}%d + ${nDiffT - nDiff}%d changes committed from ${nParts}%d parts.${serializationString}%s"
-        )
-      }
-    }
+    createAndApply()
   }
 
 }
@@ -154,6 +154,7 @@ trait CpgPassBase {
 
   def createAndApply(): Unit
 
+  @deprecated("Please use createAndApply")
   def createApplySerializeAndStore(serializedCpg: SerializedCpg, prefix: String = ""): Unit
 
   /** Name of the pass. By default it is inferred from the name of the class, override if needed.
@@ -196,11 +197,8 @@ trait CpgPassBase {
     prefix + "_" + outputName + "_" + index
   }
 
-  protected def store(overlay: GeneratedMessageV3, name: String, serializedCpg: SerializedCpg): Unit = {
-    if (overlay.getSerializedSize > 0) {
-      serializedCpg.addOverlay(overlay, name)
-    }
-  }
+  @deprecated
+  protected def store(overlay: GeneratedMessageV3, name: String, serializedCpg: SerializedCpg): Unit = {}
 
   protected def withStartEndTimesLogged[A](fun: => A): A = {
     baseLogger.info(s"Running pass: $name")

diff --git a/codepropertygraph/src/test/scala/io/shiftleft/passes/CpgPassNewTests.scala b/codepropertygraph/src/test/scala/io/shiftleft/passes/CpgPassNewTests.scala
@@ -2,7 +2,6 @@ package io.shiftleft.passes
 
 import better.files.File
 import flatgraph.SchemaViolationException
-import io.shiftleft.SerializedCpg
 import io.shiftleft.codepropertygraph.generated.Cpg
 import io.shiftleft.codepropertygraph.generated.nodes.NewFile
 import io.shiftleft.codepropertygraph.generated.language.*
@@ -16,7 +15,7 @@ class CpgPassNewTests extends AnyWordSpec with Matchers {
   private object Fixture {
     def apply(f: (Cpg, CpgPassBase) => Unit): Unit = {
       val cpg = Cpg.empty
-      class MyPass(cpg: Cpg) extends SimpleCpgPass(cpg, "MyPass") {
+      class MyPass(cpg: Cpg) extends CpgPass(cpg, "MyPass") {
         override def run(builder: DiffGraphBuilder): Unit = {
           val builder2 = Cpg.newDiffGraphBuilder
           builder.addNode(NewFile().name("foo"))
@@ -35,21 +34,9 @@ class CpgPassNewTests extends AnyWordSpec with Matchers {
       cpg.all.label.toSet shouldBe Set("FILE")
     }
 
-    "produce a serialized CPG file" in Fixture { (_, pass) =>
-      File.usingTemporaryFile("pass", ".zip") { file =>
-        file.delete()
-        val filename      = file.path.toString
-        val serializedCpg = new SerializedCpg(filename)
-        pass.createApplySerializeAndStore(serializedCpg)
-        serializedCpg.close()
-        file.exists shouldBe true
-        Files.size(file.path) should not be 0
-      }
-    }
-
     "fail for schema violations" in {
       val cpg = Cpg.empty
-      val pass = new SimpleCpgPass(cpg, "pass1") {
+      val pass = new CpgPass(cpg, "pass1") {
         override def run(dst: DiffGraphBuilder): Unit = {
           val file1 = NewFile().name("foo")
           val file2 = NewFile().name("bar")