From 788784f1f7c9390d3abbb3b1e4594e19f59b4834 Mon Sep 17 00:00:00 2001 From: David Baker Effendi Date: Thu, 25 Feb 2021 14:45:22 +0200 Subject: [PATCH] :sparkles: Quality of Life Changes (#73) * :fire: Removed log4j2 properties as to avoid them being accidentally not overridden * :lock: Changed visibility of driver constructors * :lock: AuthKey never null and now is just blank if not set * Documented changes * Fixed constructor visibility incorrectly modified and now connect calls return their object instance * Fixed scala tests * Removed a println --- CHANGELOG.md | 5 +++ .../github/plume/oss/drivers/GremlinDriver.kt | 2 +- .../plume/oss/drivers/JanusGraphDriver.kt | 4 +-- .../github/plume/oss/drivers/Neo4jDriver.kt | 4 +-- .../github/plume/oss/drivers/NeptuneDriver.kt | 4 +-- .../plume/oss/drivers/OverflowDbDriver.kt | 36 +++++++++++++++++-- .../plume/oss/drivers/TigerGraphDriver.kt | 6 ++-- .../plume/oss/drivers/TinkerGraphDriver.kt | 4 ++- .../github/plume/oss/util/SootToPlumeUtil.kt | 1 - plume/src/main/resources/log4j2.properties | 30 ---------------- .../plume/oss/drivers/Neo4jDriverIntTest.kt | 15 ++++---- .../plume/oss/drivers/NeptuneDriverIntTest.kt | 11 +++--- .../oss/drivers/OverflowDbDriverIntTest.kt | 13 ++++--- .../oss/drivers/TinkerGraphDriverIntTest.kt | 4 +-- .../plume/oss/PlumeCodeToCpgSuite.scala | 2 +- 15 files changed, 72 insertions(+), 69 deletions(-) delete mode 100644 plume/src/main/resources/log4j2.properties diff --git a/CHANGELOG.md b/CHANGELOG.md index 417039ce..54030733 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Added ### Changed +- `TigerGraphDriver::authKey` never null and now just blank if not set +- Removed `log4f2.properties` under the main artifact +- Made the visibility of driver constructors module specific so that users are forced +to use the `DriverFactory` +- `connect` methods on drivers now return the driver instead of nothing. ### Fixed diff --git a/plume/src/main/kotlin/io/github/plume/oss/drivers/GremlinDriver.kt b/plume/src/main/kotlin/io/github/plume/oss/drivers/GremlinDriver.kt index cdcdec65..6f640ecf 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/drivers/GremlinDriver.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/drivers/GremlinDriver.kt @@ -59,7 +59,7 @@ abstract class GremlinDriver : IDriver { * * @throws IllegalArgumentException if the graph database is already connected to. */ - open fun connect() { + open fun connect(): GremlinDriver = apply { require(!connected) { "Please close the graph before trying to make another connection." } graph = TinkerGraph.open(config) g = graph.traversal() diff --git a/plume/src/main/kotlin/io/github/plume/oss/drivers/JanusGraphDriver.kt b/plume/src/main/kotlin/io/github/plume/oss/drivers/JanusGraphDriver.kt index 80c934b7..89933ae2 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/drivers/JanusGraphDriver.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/drivers/JanusGraphDriver.kt @@ -15,7 +15,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource /** * The driver used to connect to a remote JanusGraph instance. */ -class JanusGraphDriver : GremlinDriver(), ISchemaSafeDriver { +class JanusGraphDriver internal constructor() : GremlinDriver(), ISchemaSafeDriver { private val logger = LogManager.getLogger(JanusGraphDriver::class.java) companion object { @@ -36,7 +36,7 @@ class JanusGraphDriver : GremlinDriver(), ISchemaSafeDriver { * @throws IllegalArgumentException if the graph database is already connected to or if the remote config path is * not set. */ - override fun connect() { + override fun connect(): JanusGraphDriver = apply { require(!connected) { "Please close the graph before trying to make another connection." } require(config.containsKey(REMOTE_CONFIG)) { "Remote config path not set! See the config field in JanusGraphDriver with key REMOTE_CONFIG." } // Test that the connection works and then close again diff --git a/plume/src/main/kotlin/io/github/plume/oss/drivers/Neo4jDriver.kt b/plume/src/main/kotlin/io/github/plume/oss/drivers/Neo4jDriver.kt index 077a9b1b..d494bbb8 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/drivers/Neo4jDriver.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/drivers/Neo4jDriver.kt @@ -23,7 +23,7 @@ import io.shiftleft.codepropertygraph.generated.nodes.Factories as NodeFactories /** * The driver used to connect to a remote Neo4j instance. */ -class Neo4jDriver : IDriver { +class Neo4jDriver internal constructor() : IDriver { private val logger = LogManager.getLogger(Neo4jDriver::class.java) private lateinit var driver: Driver @@ -104,7 +104,7 @@ class Neo4jDriver : IDriver { */ fun port(value: Int) = apply { port = value } - fun connect() { + fun connect(): Neo4jDriver = apply { require(!connected) { "Please close the graph before trying to make another connection." } driver = GraphDatabase.driver("bolt://$hostname:$port", AuthTokens.basic(username, password)) connected = true diff --git a/plume/src/main/kotlin/io/github/plume/oss/drivers/NeptuneDriver.kt b/plume/src/main/kotlin/io/github/plume/oss/drivers/NeptuneDriver.kt index f9d1d336..6b48fc8a 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/drivers/NeptuneDriver.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/drivers/NeptuneDriver.kt @@ -13,7 +13,7 @@ import org.apache.tinkerpop.gremlin.structure.Vertex /** * The driver used to connect to a remote Amazon Neptune instance. */ -class NeptuneDriver : GremlinOverriddenIdDriver() { +class NeptuneDriver internal constructor() : GremlinOverriddenIdDriver() { private val logger = LogManager.getLogger(NeptuneDriver::class.java) private val builder: Cluster.Builder = Cluster.build() @@ -53,7 +53,7 @@ class NeptuneDriver : GremlinOverriddenIdDriver() { * * @throws IllegalArgumentException if the graph database is already connected. */ - override fun connect() { + override fun connect(): NeptuneDriver = apply { require(!connected) { "Please close the graph before trying to make another connection." } cluster = builder.create() super.g = traversal().withRemote(DriverRemoteConnection.using(cluster)) diff --git a/plume/src/main/kotlin/io/github/plume/oss/drivers/OverflowDbDriver.kt b/plume/src/main/kotlin/io/github/plume/oss/drivers/OverflowDbDriver.kt index 33d7fcd7..ca7cbfe4 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/drivers/OverflowDbDriver.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/drivers/OverflowDbDriver.kt @@ -15,7 +15,7 @@ import io.shiftleft.codepropertygraph.generated.nodes.Factories as NodeFactories /** * Driver to create an OverflowDB database file from Plume's domain classes. */ -class OverflowDbDriver : IDriver { +class OverflowDbDriver internal constructor() : IDriver { private val logger = LogManager.getLogger(OverflowDbDriver::class.java) @@ -30,23 +30,55 @@ class OverflowDbDriver : IDriver { * Where the database will be serialize/deserialize and overflow to disk. */ var storageLocation: String = "" + private set /** * Specifies if OverflowDb should write to disk when memory is constrained. */ var overflow: Boolean = true + private set /** * Percentage of the heap from when overflowing should begin to occur. Default is 80%. */ var heapPercentageThreshold: Int = 80 + private set /** * If specified, OverflowDB will measure and report serialization/deserialization timing averages. */ var serializationStatsEnabled: Boolean = false + private set - fun connect() { + /** + * Set the storage location. + * + * @param value the storage location to overflow to e.g. /tmp/cpg.bin + */ + fun storageLocation(value: String): OverflowDbDriver = apply { storageLocation = value } + + /** + * Set whether the database overflows or not. + * + * @param value true to overflow, false to remain in memory. + */ + fun overflow(value: Boolean): OverflowDbDriver = apply { overflow = value } + + /** + * Set the percentage threshold before overflowing. + * + * @param value the percentage of the heap space. + */ + fun heapPercentageThreshold(value: Int): OverflowDbDriver = apply { heapPercentageThreshold = value } + + /** + * To set if serialization/deserialization timing averages should be reported. + * + * @param value true to report averages, false to not. + */ + fun serializationStatsEnabled(value: Boolean): OverflowDbDriver = apply { serializationStatsEnabled = value } + + fun connect(): OverflowDbDriver = apply { require(!connected) { "Please close the graph before trying to make another connection." } val odbConfig = Config.withDefaults() .apply { diff --git a/plume/src/main/kotlin/io/github/plume/oss/drivers/TigerGraphDriver.kt b/plume/src/main/kotlin/io/github/plume/oss/drivers/TigerGraphDriver.kt index f780fe27..97a22dcd 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/drivers/TigerGraphDriver.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/drivers/TigerGraphDriver.kt @@ -32,7 +32,7 @@ import io.shiftleft.codepropertygraph.generated.nodes.Factories as NodeFactories /** * The driver used to connect to a remote TigerGraph instance. */ -class TigerGraphDriver : IOverridenIdDriver, ISchemaSafeDriver { +class TigerGraphDriver internal constructor() : IOverridenIdDriver, ISchemaSafeDriver { private val logger = LogManager.getLogger(TigerGraphDriver::class.java) private val objectMapper = ObjectMapper() @@ -83,7 +83,7 @@ class TigerGraphDriver : IOverridenIdDriver, ISchemaSafeDriver { * The authorization key used for TigerGraph servers with token authorization turned on. This is placed under * the Authorization header when making requests. */ - var authKey: String? = null + var authKey: String = "" private set init { @@ -376,7 +376,7 @@ class TigerGraphDriver : IOverridenIdDriver, ISchemaSafeDriver { PlumeKeyProvider.clearKeyPools() } - private fun headers(contentType: String = "application/json"): Map = if (authKey == null) { + private fun headers(contentType: String = "application/json"): Map = if (authKey.isBlank()) { mapOf("Content-Type" to contentType) } else { mapOf("Content-Type" to contentType, "Authorization" to "Bearer $authKey") diff --git a/plume/src/main/kotlin/io/github/plume/oss/drivers/TinkerGraphDriver.kt b/plume/src/main/kotlin/io/github/plume/oss/drivers/TinkerGraphDriver.kt index 8944eb5d..c7e1a583 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/drivers/TinkerGraphDriver.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/drivers/TinkerGraphDriver.kt @@ -8,7 +8,9 @@ import java.io.File /** * The driver used to connect to an in-memory TinkerGraph instance. */ -class TinkerGraphDriver : GremlinDriver() { +class TinkerGraphDriver internal constructor() : GremlinDriver() { + + override fun connect(): TinkerGraphDriver = super.connect() as TinkerGraphDriver /** * Add or update a [BaseConfiguration] key-value pair. diff --git a/plume/src/main/kotlin/io/github/plume/oss/util/SootToPlumeUtil.kt b/plume/src/main/kotlin/io/github/plume/oss/util/SootToPlumeUtil.kt index ef8d9490..4ab9c84b 100644 --- a/plume/src/main/kotlin/io/github/plume/oss/util/SootToPlumeUtil.kt +++ b/plume/src/main/kotlin/io/github/plume/oss/util/SootToPlumeUtil.kt @@ -517,7 +517,6 @@ object SootToPlumeUtil { sym == "<=" -> Operators.lessEqualsThan sym == ">=" -> Operators.greaterEqualsThan else -> { - println("Unknown $sym") logger.warn("Unknown binary operator $sym") sym } diff --git a/plume/src/main/resources/log4j2.properties b/plume/src/main/resources/log4j2.properties deleted file mode 100644 index fd527b7f..00000000 --- a/plume/src/main/resources/log4j2.properties +++ /dev/null @@ -1,30 +0,0 @@ -status=error -name=PropertiesConfig -#Make sure to change log file path as per your need -property.filename=${sys:java.io.tmpdir}/plume/plume.log -filters=threshold -filter.threshold.type=ThresholdFilter -filter.threshold.level=debug -appenders=plume -appender.plume.type=RollingFile -appender.plume.name=RollingFile -appender.plume.fileName=${filename} -appender.plume.filePattern=${sys:java.io.tmpdir}/plume/debug-backup-%d{MM-dd-yy-HH-mm-ss}-%i.log.gz -appender.plume.layout.type=PatternLayout -appender.plume.layout.pattern=%d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n -appender.plume.policies.type=Policies -appender.plume.policies.time.type=TimeBasedTriggeringPolicy -appender.plume.policies.time.interval=1 -appender.plume.policies.time.modulate=true -appender.plume.policies.size.type=SizeBasedTriggeringPolicy -appender.plume.policies.size.size=10MB -appender.plume.strategy.type=DefaultRolloverStrategy -appender.plume.strategy.max=500 -loggers=plume,tigergraph -#Make sure to change the package structure as per your application -logger.plume.name=io.github.plume -logger.plume.level=debug -logger.plume.additivity=false -logger.plume.appenderRef.rolling.ref=RollingFile -logger.tigergraph.name=com.tigergraph -logger.tigergraph.level=fatal \ No newline at end of file diff --git a/plume/src/test/kotlin/io/github/plume/oss/drivers/Neo4jDriverIntTest.kt b/plume/src/test/kotlin/io/github/plume/oss/drivers/Neo4jDriverIntTest.kt index 4b8d481a..2eccdbc5 100644 --- a/plume/src/test/kotlin/io/github/plume/oss/drivers/Neo4jDriverIntTest.kt +++ b/plume/src/test/kotlin/io/github/plume/oss/drivers/Neo4jDriverIntTest.kt @@ -52,14 +52,13 @@ class Neo4jDriverIntTest { @BeforeAll fun setUpAll() = run { testStartTime = System.nanoTime() - driver = (DriverFactory(GraphDatabase.NEO4J) as Neo4jDriver).apply { - this.hostname("localhost") - .port(7687) - .username("neo4j") - .password("neo4j123") - .database("neo4j") - .connect() - } + driver = (DriverFactory(GraphDatabase.NEO4J) as Neo4jDriver) + .hostname("localhost") + .port(7687) + .username("neo4j") + .password("neo4j123") + .database("neo4j") + .connect() assertEquals("localhost", driver.hostname) assertEquals(7687, driver.port) assertEquals("neo4j", driver.username) diff --git a/plume/src/test/kotlin/io/github/plume/oss/drivers/NeptuneDriverIntTest.kt b/plume/src/test/kotlin/io/github/plume/oss/drivers/NeptuneDriverIntTest.kt index 5f3d8e84..3a860265 100644 --- a/plume/src/test/kotlin/io/github/plume/oss/drivers/NeptuneDriverIntTest.kt +++ b/plume/src/test/kotlin/io/github/plume/oss/drivers/NeptuneDriverIntTest.kt @@ -52,12 +52,11 @@ class NeptuneDriverIntTest { @BeforeAll fun setUpAll() { testStartTime = System.nanoTime() - driver = (DriverFactory(GraphDatabase.NEPTUNE) as NeptuneDriver).apply { - this.addHostnames(System.getenv("NEPTUNE_HOSTNAME") ?: "localhost") - .port(8182) - .keyCertChainFile("src/test/resources/conf/SFSRootCAG2.pem") - .connect() - } + driver = (DriverFactory(GraphDatabase.NEPTUNE) as NeptuneDriver) + .addHostnames(System.getenv("NEPTUNE_HOSTNAME") ?: "localhost") + .port(8182) + .keyCertChainFile("src/test/resources/conf/SFSRootCAG2.pem") + .connect() } @JvmStatic diff --git a/plume/src/test/kotlin/io/github/plume/oss/drivers/OverflowDbDriverIntTest.kt b/plume/src/test/kotlin/io/github/plume/oss/drivers/OverflowDbDriverIntTest.kt index 624cbee9..15e95c16 100644 --- a/plume/src/test/kotlin/io/github/plume/oss/drivers/OverflowDbDriverIntTest.kt +++ b/plume/src/test/kotlin/io/github/plume/oss/drivers/OverflowDbDriverIntTest.kt @@ -56,13 +56,12 @@ class OverflowDbDriverIntTest { @BeforeAll fun setUpAll() { testStartTime = System.nanoTime() - driver = (DriverFactory(GraphDatabase.OVERFLOWDB) as OverflowDbDriver).apply { - serializationStatsEnabled = false - overflow = true - heapPercentageThreshold = 90 - storageLocation = OverflowDbDriverIntTest.storageLocation - connect() - } + driver = (DriverFactory(GraphDatabase.OVERFLOWDB) as OverflowDbDriver) + .serializationStatsEnabled(false) + .overflow(true) + .heapPercentageThreshold(90) + .storageLocation(storageLocation) + .connect() assertFalse(driver.serializationStatsEnabled) assertTrue(driver.overflow) assertEquals(90, driver.heapPercentageThreshold) diff --git a/plume/src/test/kotlin/io/github/plume/oss/drivers/TinkerGraphDriverIntTest.kt b/plume/src/test/kotlin/io/github/plume/oss/drivers/TinkerGraphDriverIntTest.kt index 2f1dd020..a456f66b 100644 --- a/plume/src/test/kotlin/io/github/plume/oss/drivers/TinkerGraphDriverIntTest.kt +++ b/plume/src/test/kotlin/io/github/plume/oss/drivers/TinkerGraphDriverIntTest.kt @@ -36,11 +36,9 @@ import io.shiftleft.codepropertygraph.generated.EdgeTypes.* import io.shiftleft.codepropertygraph.generated.NodeKeyNames.NAME import io.shiftleft.codepropertygraph.generated.NodeTypes.FILE import io.shiftleft.codepropertygraph.generated.nodes.* -import io.shiftleft.proto.cpg.Cpg import org.apache.logging.log4j.LogManager import org.junit.jupiter.api.* import org.junit.jupiter.api.Assertions.* -import overflowdb.Config import overflowdb.Graph import scala.Option import java.io.File @@ -62,7 +60,7 @@ class TinkerGraphDriverIntTest { @BeforeAll fun setUpAll() = run { testStartTime = System.nanoTime() - driver = (DriverFactory(GraphDatabase.TINKER_GRAPH) as TinkerGraphDriver).apply { connect() } + driver = (DriverFactory(GraphDatabase.TINKER_GRAPH) as TinkerGraphDriver).connect() } @AfterAll diff --git a/testing/src/test/scala/io/github/plume/oss/PlumeCodeToCpgSuite.scala b/testing/src/test/scala/io/github/plume/oss/PlumeCodeToCpgSuite.scala index a0a9ac68..7babc9dc 100644 --- a/testing/src/test/scala/io/github/plume/oss/PlumeCodeToCpgSuite.scala +++ b/testing/src/test/scala/io/github/plume/oss/PlumeCodeToCpgSuite.scala @@ -14,7 +14,7 @@ class PlumeFrontend extends LanguageFrontend { val cpgFile = File.createTempFile("plume", ".bin") cpgFile.deleteOnExit() Using(DriverFactory.invoke(GraphDatabase.OVERFLOWDB).asInstanceOf[OverflowDbDriver]) { driver => - driver.setStorageLocation(cpgFile.getAbsolutePath) + driver.storageLocation(cpgFile.getAbsolutePath) val extractor = new Extractor(driver) extractor.load(sourceCodeFile).project().postProject() driver.close()