Validate concurrency key is required when scope is account or env (#81)

- If concurrency scope is account or env, validate that key is provided - Also added check that at most 2 concurrency options are provided - Removed exception catch in InngestFunction.kt since it is swallowing other InngestInvalidConfigurationExceptions - Add key for RestoreFromGlacier example
inngest · Sep 10, 2024 · 937f7af · 937f7af
1 parent 70ac4e3
commit 937f7af
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 10 deletions.
diff --git a/inngest-test-server/src/main/kotlin/com/inngest/testserver/RestoreFromGlacier.kt b/inngest-test-server/src/main/kotlin/com/inngest/testserver/RestoreFromGlacier.kt
@@ -9,7 +9,7 @@ class RestoreFromGlacier : InngestFunction() {
             .id("RestoreFromGlacier")
             .name("Restore from Glacier")
             .trigger(InngestFunctionTriggers.Event("delivery/restore.requested"))
-            .concurrency(10, null, ConcurrencyScope.ENVIRONMENT)
+            .concurrency(10, "event.data.user_id", ConcurrencyScope.ENVIRONMENT)
 
     override fun execute(
         ctx: FunctionContext,

diff --git a/inngest/src/main/kotlin/com/inngest/InngestFunction.kt b/inngest/src/main/kotlin/com/inngest/InngestFunction.kt
@@ -20,13 +20,7 @@ abstract class InngestFunction {
     }
 
     fun id(): String {
-        try {
-            return buildConfig().id!!
-        } catch (e: Exception) {
-            throw InngestInvalidConfigurationException(
-                "Function id must be configured via builder: ${this.javaClass.name}",
-            )
-        }
+        return buildConfig().id!!
     }
 
     internal fun toInngestFunction(): InternalInngestFunction {

diff --git a/inngest/src/main/kotlin/com/inngest/InngestFunctionConfigBuilder.kt b/inngest/src/main/kotlin/com/inngest/InngestFunctionConfigBuilder.kt
@@ -113,12 +113,20 @@ class InngestFunctionConfigBuilder {
         key: String? = null,
         scope: ConcurrencyScope? = null,
     ): InngestFunctionConfigBuilder {
+        when (scope) {
+            ConcurrencyScope.ENVIRONMENT -> if (key == null) throw InngestInvalidConfigurationException("Concurrency key required with environment scope")
+            ConcurrencyScope.ACCOUNT -> if (key == null) throw InngestInvalidConfigurationException("Concurrency key required with account scope")
+            ConcurrencyScope.FUNCTION -> {}
+            null -> {}
+        }
+
         val c = Concurrency(limit, key, scope)
-        // TODO - Limit concurrency length to 2
         if (this.concurrency == null) {
             this.concurrency = mutableListOf(c)
+        } else if (this.concurrency!!.size == 2) {
+            throw InngestInvalidConfigurationException("Maximum of 2 concurrency options allowed")
         } else {
-            this.concurrency?.add(c)
+            this.concurrency!!.add(c)
         }
         return this
     }

diff --git a/inngest/src/test/kotlin/com/inngest/InngestFunctionConfigBuilderTest.kt b/inngest/src/test/kotlin/com/inngest/InngestFunctionConfigBuilderTest.kt
@@ -21,4 +21,68 @@ class InngestFunctionConfigBuilderTest {
                 .build("app-id", "https://mysite.com/api/inngest")
         }
     }
+
+    @Test
+    fun testConcurrencyLimitOnly() {
+        val config =
+            InngestFunctionConfigBuilder()
+                .id("test-id")
+                .concurrency(5)
+                .build("app-id", "https://mysite.com/api/inngest")
+        assertEquals<List<Concurrency>?>(listOf(Concurrency(5)), config.concurrency)
+    }
+
+    @Test
+    fun testConcurrencyLimitAndKeyWithoutScope() {
+        val config =
+            InngestFunctionConfigBuilder()
+                .id("test-id")
+                .concurrency(5, "event.data.user_id")
+                .build("app-id", "https://mysite.com/api/inngest")
+        assertEquals<List<Concurrency>?>(listOf(Concurrency(5, "event.data.user_id")), config.concurrency)
+    }
+
+    @Test
+    fun testConcurrencyScope() {
+        val config =
+            InngestFunctionConfigBuilder()
+                .id("test-id")
+                .concurrency(5, null, ConcurrencyScope.FUNCTION)
+                .build("app-id", "https://mysite.com/api/inngest")
+        assertEquals<List<Concurrency>?>(listOf(Concurrency(5, null, ConcurrencyScope.FUNCTION)), config.concurrency)
+
+        assertFailsWith<InngestInvalidConfigurationException> {
+            InngestFunctionConfigBuilder()
+                .id("test-id")
+                .concurrency(5, null, ConcurrencyScope.ENVIRONMENT)
+                .build("app-id", "https://mysite.com/api/inngest")
+        }
+
+        assertFailsWith<InngestInvalidConfigurationException> {
+            InngestFunctionConfigBuilder()
+                .id("test-id")
+                .concurrency(5, null, ConcurrencyScope.ACCOUNT)
+                .build("app-id", "https://mysite.com/api/inngest")
+        }
+    }
+
+    @Test
+    fun testConcurrencyMaxOptionsLength() {
+        val config =
+            InngestFunctionConfigBuilder()
+                .id("test-id")
+                .concurrency(5, null, ConcurrencyScope.FUNCTION)
+                .concurrency(7, "event.data.user_id", ConcurrencyScope.ENVIRONMENT)
+                .build("app-id", "https://mysite.com/api/inngest")
+        assertEquals<List<Concurrency>?>(listOf(Concurrency(5, null, ConcurrencyScope.FUNCTION), Concurrency(7, "event.data.user_id", ConcurrencyScope.ENVIRONMENT)), config.concurrency)
+
+        assertFailsWith<InngestInvalidConfigurationException> {
+            InngestFunctionConfigBuilder()
+                .id("test-id")
+                .concurrency(5, null, ConcurrencyScope.FUNCTION)
+                .concurrency(7, "event.data.user_id", ConcurrencyScope.ENVIRONMENT)
+                .concurrency(9, "event.data.account_id", ConcurrencyScope.ACCOUNT)
+                .build("app-id", "https://mysite.com/api/inngest")
+        }
+    }
 }