fix!: support null as option value (#221)

BREAKING-CHANGE: serialize options as JSON instead of TOML

build.gradle.kts

@@ -30,10 +30,17 @@ dependencies {
     implementation("com.github.revanced:jadb:master-SNAPSHOT") // updated fork
     implementation("com.android.tools.build:apksig:8.1.0-alpha09")
     implementation("org.bouncycastle:bcpkix-jdk15on:1.70")
-    implementation("cc.ekblad:4koma:1.1.0")
+    implementation("com.fasterxml.jackson.module:jackson-module-kotlin:2.14.+")
+    testImplementation("org.jetbrains.kotlin:kotlin-test:1.8.20-RC")
 }
 
 tasks {
+    test {
+        useJUnitPlatform()
+        testLogging {
+            events("PASSED", "SKIPPED", "FAILED")
+        }
+    }
     build {
         dependsOn(shadowJar)
     }

src/main/kotlin/app/revanced/cli/command/MainCommand.kt

@@ -7,16 +7,24 @@ import app.revanced.cli.patcher.logging.impl.PatcherLogger
 import app.revanced.cli.signing.Signing
 import app.revanced.cli.signing.SigningOptions
 import app.revanced.patcher.PatcherOptions
+import app.revanced.patcher.data.Context
 import app.revanced.patcher.extensions.PatchExtensions.compatiblePackages
 import app.revanced.patcher.extensions.PatchExtensions.description
 import app.revanced.patcher.extensions.PatchExtensions.patchName
+import app.revanced.patcher.patch.Patch
 import app.revanced.patcher.util.patch.PatchBundle
-import app.revanced.utils.OptionsLoader
+import app.revanced.utils.Options
+import app.revanced.utils.Options.setOptions
 import app.revanced.utils.adb.Adb
 import picocli.CommandLine.*
 import java.io.File
 import java.nio.file.Files
 
+/**
+ * Alias for return type of [PatchBundle.loadPatches].
+ */
+internal typealias PatchList = List<Class<out Patch<Context>>>
+
 private class CLIVersionProvider : IVersionProvider {
     override fun getVersion() = arrayOf(
         MainCommand::class.java.`package`.implementationVersion ?: "unknown"
@@ -55,8 +63,8 @@ internal object MainCommand : Runnable {
         @Option(names = ["-b", "--bundle"], description = ["One or more bundles of patches"], required = true)
         var patchBundles = arrayOf<String>()
 
-        @Option(names = ["--options"], description = ["Configuration file for all patch options"])
+        @Option(names = ["--options"], description = ["Path to patch options JSON file"])
-        var options: File = File("options.toml")
+        var optionsFile: File = File("options.json")
 
         @ArgGroup(exclusive = false)
         var listingArgs: ListingArgs? = null
@@ -134,7 +142,10 @@ internal object MainCommand : Runnable {
             PatchBundle.Jar(bundle).loadPatches()
         }
 
-        OptionsLoader.init(args.patchArgs!!.options, allPatches)
+        args.patchArgs!!.optionsFile.let {
+            if (it.exists()) allPatches.setOptions(it, logger)
+            else Options.serialize(allPatches, prettyPrint = true).let(it::writeText)
+        }
 
         val patcher = app.revanced.patcher.Patcher(
             PatcherOptions(

src/main/kotlin/app/revanced/cli/patcher/Patcher.kt

@@ -1,8 +1,7 @@
 package app.revanced.cli.patcher
 
+import app.revanced.cli.command.PatchList
 import app.revanced.patcher.PatcherResult
-import app.revanced.patcher.data.Context
-import app.revanced.patcher.patch.Patch
 import app.revanced.utils.patcher.addPatchesFiltered
 import app.revanced.utils.patcher.applyPatchesVerbose
 import app.revanced.utils.patcher.mergeFiles
@@ -10,7 +9,7 @@ import app.revanced.utils.patcher.mergeFiles
 internal object Patcher {
     internal fun start(
         patcher: app.revanced.patcher.Patcher,
-        allPatches: List<Class<out Patch<Context>>>
+        allPatches: PatchList
     ): PatcherResult {
         // merge files like necessary integrations
         patcher.mergeFiles()

src/main/kotlin/app/revanced/utils/Options.kt

@@ -0,0 +1,103 @@
+package app.revanced.utils
+
+import app.revanced.cli.command.PatchList
+import app.revanced.cli.logging.CliLogger
+import app.revanced.patcher.extensions.PatchExtensions.options
+import app.revanced.patcher.extensions.PatchExtensions.patchName
+import app.revanced.patcher.patch.NoSuchOptionException
+import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
+import java.io.File
+
+
+internal object Options {
+    private var mapper = jacksonObjectMapper()
+
+    /**
+     * Serializes the options for the patches in the list.
+     *
+     * @param patches The list of patches to serialize.
+     * @param prettyPrint Whether to pretty print the JSON.
+     * @return The JSON string containing the options.
+     * @see PatchList
+     */
+    fun serialize(patches: PatchList, prettyPrint: Boolean = false): String = patches
+        .filter { it.options?.any() == true }
+        .map { patch ->
+            PatchOption(
+                patch.patchName,
+                patch.options!!.map { option -> PatchOption.Option(option.key, option.value) }
+            )
+        }.let {
+            if (prettyPrint)
+                mapper.writerWithDefaultPrettyPrinter().writeValueAsString(it)
+            else
+                mapper.writeValueAsString(it)
+        }
+
+    /**
+     * Deserializes the options for the patches in the list.
+     *
+     * @param json The JSON string containing the options.
+     * @return The list of [PatchOption]s.
+     * @see PatchOption
+     * @see PatchList
+     */
+    @Suppress("MemberVisibilityCanBePrivate")
+    fun deserialize(json: String): Array<PatchOption> = mapper.readValue(json, Array<PatchOption>::class.java)
+
+    /**
+     * Sets the options for the patches in the list.
+     *
+     * @param json The JSON string containing the options.
+     * @param logger The logger to use for logging.
+     */
+    fun PatchList.setOptions(json: String, logger: CliLogger? = null) {
+        filter { it.options?.any() == true }.let { patches ->
+            if (patches.isEmpty()) return
+
+            val patchOptions = deserialize(json)
+
+            patches.forEach { patch ->
+                patchOptions.find { option -> option.patchName == patch.patchName }?.let {
+                    it.options.forEach { option ->
+                        try {
+                            patch.options?.set(option.key, option.value)
+                                ?: logger?.warn("${patch.patchName} has no options")
+                        } catch (e: NoSuchOptionException) {
+                            logger?.error(e.message ?: "Unknown error")
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * Sets the options for the patches in the list.
+     *
+     * @param file The file containing the JSON string containing the options.
+     * @param logger The logger to use for logging.
+     * @see setOptions
+     */
+    fun PatchList.setOptions(file: File, logger: CliLogger? = null) = setOptions(file.readText(), logger)
+
+    /**
+     * Data class for a patch and its [Option]s.
+     *
+     * @property patchName The name of the patch.
+     * @property options The [Option]s for the patch.
+     */
+    internal data class PatchOption(
+        val patchName: String,
+        val options: List<Option>
+    ) {
+
+        /**
+         * Data class for patch option.
+         *
+         * @property key The name of the option.
+         * @property value The value of the option.
+         */
+        internal data class Option(val key: String, val value: Any?)
+    }
+}

src/main/kotlin/app/revanced/utils/OptionsLoader.kt

@@ -1,77 +0,0 @@
-package app.revanced.utils
-
-import app.revanced.cli.command.MainCommand.logger
-import app.revanced.patcher.Patcher
-import app.revanced.patcher.data.Context
-import app.revanced.patcher.extensions.PatchExtensions.options
-import app.revanced.patcher.extensions.PatchExtensions.patchName
-import app.revanced.patcher.patch.Patch
-import cc.ekblad.toml.encodeToString
-import cc.ekblad.toml.model.TomlValue
-import cc.ekblad.toml.serialization.from
-import cc.ekblad.toml.tomlMapper
-import java.io.File
-
-private typealias PatchList = List<Class<out Patch<Context>>>
-private typealias OptionsMap = MutableMap<String, MutableMap<String, Any>>
-
-object OptionsLoader {
-    @JvmStatic
-    private val mapper = tomlMapper {}
-
-    @JvmStatic
-    fun init(file: File, patches: PatchList) {
-        if (!file.exists()) file.createNewFile()
-        val map = mapper.decodeWithDefaults(generateDefaults(patches), TomlValue.from(file.toPath()))
-        readAndSet(map, patches)
-        save(map, file)
-    }
-
-    private fun readAndSet(map: OptionsMap, patches: PatchList) {
-        for ((patchName, options) in map) {
-            val patch = patches.find { it.patchName == patchName } ?: continue
-            val patchOptions = patch.options ?: continue
-            for ((key, value) in options) {
-                if (value == "null") { // backwards compatibility, subject to removal
-                    options.remove(key)
-                    continue
-                }
-                try {
-                    patchOptions[key] = value
-                } catch (e: Exception) {
-                    logger.error("Error while setting option $key for patch $patchName: ${e.message}")
-                    e.printStackTrace()
-                }
-            }
-        }
-    }
-
-    private fun save(map: OptionsMap, file: File) {
-        val toml = mapper.encodeToString(map)
-        file.writeText(
-            """
-                # A list of options for each patch.
-                # This file does not contain all options by default.
-                # Run the CLI with the "--list --with-options" flags to see all available options.
-                # You can also run the CLI with the aforementioned flags and a patch name to see all available options for that patch.
-                # To set an option, add a line with the format "option = value" or set the value if the option already exists.
-                # To reset an option to its default value, delete the line.
-                # To reset all options to their default values, delete this file.
-                # 
-                # This file was generated by ReVanced Patcher version ${Patcher.version}.
-            """.trimIndent() + "\n\n$toml"
-        )
-    }
-
-    private fun generateDefaults(patches: PatchList) = buildMap {
-        for (patch in patches) {
-            val options = patch.options ?: continue
-            if (!options.iterator().hasNext()) continue
-            put(patch.patchName, buildMap {
-                for (option in options) {
-                    put(option.key, option.value ?: continue)
-                }
-            } as MutableMap)
-        }
-    } as MutableMap
-}

src/main/kotlin/app/revanced/utils/patcher/Patcher.kt

@@ -3,6 +3,7 @@ package app.revanced.utils.patcher
 import app.revanced.cli.command.MainCommand
 import app.revanced.cli.command.MainCommand.args
 import app.revanced.cli.command.MainCommand.logger
+import app.revanced.cli.command.PatchList
 import app.revanced.patcher.Patcher
 import app.revanced.patcher.data.Context
 import app.revanced.patcher.extensions.PatchExtensions.compatiblePackages
@@ -10,7 +11,7 @@ import app.revanced.patcher.extensions.PatchExtensions.include
 import app.revanced.patcher.extensions.PatchExtensions.patchName
 import app.revanced.patcher.patch.Patch
 
-fun Patcher.addPatchesFiltered(allPatches: List<Class<out Patch<Context>>>) {
+fun Patcher.addPatchesFiltered(allPatches: PatchList) {
     val packageName = this.context.packageMetadata.packageName
     val packageVersion = this.context.packageMetadata.packageVersion
 

src/test/kotlin/app/revanced/patcher/options/PatchOptionOptionsTest.kt

@@ -0,0 +1,52 @@
+package app.revanced.patcher.options
+
+import app.revanced.patcher.data.BytecodeContext
+import app.revanced.patcher.data.Context
+import app.revanced.patcher.patch.*
+import app.revanced.utils.Options
+import app.revanced.utils.Options.setOptions
+import org.junit.jupiter.api.Test
+
+class PatchOptionsTestPatch : BytecodePatch() {
+    override fun execute(context: BytecodeContext): PatchResult {
+        return PatchResultSuccess()
+    }
+
+    companion object : OptionsContainer() {
+        var key1 by option(
+            PatchOption.StringOption(
+                "key1", null, "title1", "description1"
+            )
+        )
+
+        var key2 by option(
+            PatchOption.BooleanOption(
+                "key2", true, "title2", "description2"
+            )
+        )
+    }
+}
+
+internal object PatchOptionOptionsTest {
+    private var patches = listOf(PatchOptionsTestPatch::class.java as Class<out Patch<Context>>)
+
+    @Test
+    fun serializeTest() {
+        assert(SERIALIZED_JSON == Options.serialize(patches))
+    }
+
+    @Test
+    fun loadOptionsTest() {
+        patches.setOptions(CHANGED_JSON)
+
+        assert(PatchOptionsTestPatch.key1 == "test")
+        assert(PatchOptionsTestPatch.key2 == false)
+    }
+
+    private const val SERIALIZED_JSON =
+        "[{\"patchName\":\"PatchOptionsTestPatch\",\"options\":[{\"key\":\"key1\",\"value\":null},{\"key\":\"key2\",\"value\":true}]}]"
+
+    private const val CHANGED_JSON =
+        "[{\"patchName\":\"PatchOptionsTestPatch\",\"options\":[{\"key\":\"key1\",\"value\":\"test\"},{\"key\":\"key2\",\"value\":false}]}]"
+
+}