-
Notifications
You must be signed in to change notification settings - Fork 3
Add unsafe metadata support to clerk-android #355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| package com.clerk.api.user | ||
|
|
||
| import kotlinx.serialization.encodeToString | ||
| import kotlinx.serialization.json.Json | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertNotNull | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
| import org.robolectric.RobolectricTestRunner | ||
|
|
||
| @RunWith(RobolectricTestRunner::class) | ||
| class UserTest { | ||
|
|
||
| @Test | ||
| fun `UpdateParams should include unsafeMetadata when provided`() { | ||
| // Given | ||
| val unsafeMetadataJson = """{"birthday":"11-30-1969"}""" | ||
| val params = | ||
| User.UpdateParams(firstName = "John", lastName = "Doe", unsafeMetadata = unsafeMetadataJson) | ||
|
|
||
| // When | ||
| val paramsMap = params.toMap() | ||
|
|
||
| // Then | ||
| assertNotNull("Map should not be null", paramsMap) | ||
| assertEquals("Should include firstName", "John", paramsMap["first_name"]) | ||
| assertEquals("Should include lastName", "Doe", paramsMap["last_name"]) | ||
| assertEquals("Should include unsafe_metadata", unsafeMetadataJson, paramsMap["unsafe_metadata"]) | ||
| } | ||
|
|
||
| @Test | ||
| fun `UpdateParams should exclude unsafeMetadata when null`() { | ||
| // Given | ||
| val params = User.UpdateParams(firstName = "Jane", lastName = "Smith", unsafeMetadata = null) | ||
|
|
||
| // When | ||
| val paramsMap = params.toMap() | ||
|
|
||
| // Then | ||
| assertNotNull("Map should not be null", paramsMap) | ||
| assertEquals("Should include firstName", "Jane", paramsMap["first_name"]) | ||
| assertEquals("Should include lastName", "Smith", paramsMap["last_name"]) | ||
| assertTrue( | ||
| "Should not include unsafe_metadata when null", | ||
| !paramsMap.containsKey("unsafe_metadata"), | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun `UpdateParams should include all metadata types when provided`() { | ||
| // Given | ||
| val publicMetadataJson = """{"role":"admin"}""" | ||
| val privateMetadataJson = """{"internal_id":"12345"}""" | ||
| val unsafeMetadataJson = """{"birthday":"11-30-1969"}""" | ||
|
|
||
| val params = | ||
| User.UpdateParams( | ||
| firstName = "Alice", | ||
| publicMetadata = publicMetadataJson, | ||
| privateMetadata = privateMetadataJson, | ||
| unsafeMetadata = unsafeMetadataJson, | ||
| ) | ||
|
|
||
| // When | ||
| val paramsMap = params.toMap() | ||
|
|
||
| // Then | ||
| assertNotNull("Map should not be null", paramsMap) | ||
| assertEquals("Should include firstName", "Alice", paramsMap["first_name"]) | ||
| assertEquals("Should include public_metadata", publicMetadataJson, paramsMap["public_metadata"]) | ||
| assertEquals( | ||
| "Should include private_metadata", | ||
| privateMetadataJson, | ||
| paramsMap["private_metadata"], | ||
| ) | ||
| assertEquals("Should include unsafe_metadata", unsafeMetadataJson, paramsMap["unsafe_metadata"]) | ||
| } | ||
|
|
||
| @Test | ||
| fun `UpdateParams should only include unsafeMetadata when only it is provided`() { | ||
| // Given | ||
| val unsafeMetadataJson = """{"preferences":{"theme":"dark"}}""" | ||
|
|
||
| val params = User.UpdateParams(unsafeMetadata = unsafeMetadataJson) | ||
|
|
||
| // When | ||
| val paramsMap = params.toMap() | ||
|
|
||
| // Then | ||
| assertNotNull("Map should not be null", paramsMap) | ||
| assertEquals("Map should contain only one entry", 1, paramsMap.size) | ||
| assertEquals("Should include unsafe_metadata", unsafeMetadataJson, paramsMap["unsafe_metadata"]) | ||
| } | ||
|
|
||
| @Test | ||
| fun `UpdateParams serialization should include unsafe_metadata field`() { | ||
| // Given | ||
| val unsafeMetadataJson = """{"custom":"data"}""" | ||
| val params = User.UpdateParams(username = "testuser", unsafeMetadata = unsafeMetadataJson) | ||
|
|
||
| // When | ||
| val json = Json.encodeToString(params) | ||
|
|
||
| // Then | ||
| assertTrue("Serialized JSON should contain unsafe_metadata", json.contains("unsafe_metadata")) | ||
| assertTrue( | ||
| "Serialized JSON should contain the metadata value", | ||
| json.contains(unsafeMetadataJson), | ||
| ) | ||
| } | ||
|
Comment on lines
96
to
111
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify JSON escaping in serialization test. The assertion on lines 107-110 checks if the serialized JSON contains the unescaped
The test Please run this test to confirm it passes, or consider updating the assertion to verify proper JSON escaping: If the test passes, the assertion is working as intended. If it fails, consider this alternative assertion: // Verify proper JSON escaping
assertTrue(
"Serialized JSON should contain escaped metadata value",
json.contains("\\\"custom\\\":\\\"data\\\"") || json.contains("{\\\"custom\\\":\\\"data\\\"}")
)🏁 Script executed: #!/bin/bash
# Run the specific serialization test to verify it passes
./gradlew :api:testDebugUnitTest --tests "com.clerk.api.user.UserTest.UpdateParams serialization should include unsafe_metadata field"Length of output: 189 Length of output: 26 🏁 Script executed: #!/bin/bash
# Search for unsafeMetadata field definition and usage
rg -n "unsafeMetadata" --type=kt source/api/src -C 3Length of output: 26 🏁 Script executed: #!/bin/bash
# Check if there are custom serializers for UpdateParams
rg -n "@Serializable|KSerializer" --type=kt source/api/src -C 2 | grep -A 5 -B 5 "UpdateParams"Length of output: 26 Length of output: 53 🏁 Script executed: #!/bin/bash
# Search for UpdateParams class without type restriction
rg -n "UpdateParams" source/api/src -C 5 | head -100Length of output: 8221 🏁 Script executed: #!/bin/bash
# Search for unsafeMetadata without type restriction
rg -n "unsafeMetadata" source/api/src -C 3 | head -100Length of output: 8692 Length of output: 26371 Length of output: 6826 🏁 Script executed: #!/bin/bash
# Check for custom serializers or Json configuration
rg -n "kotlinx.serialization|Json\\.encodeToString|KSerializer" source/api/src -C 2 | head -60Length of output: 4541 💡 Result: Short answer: kotlinx.serialization follows the JSON spec — string values are emitted in double quotes and any interior double-quote characters are escaped with a backslash (" ) when you serialize (e.g., Json.encodeToString). If you need an unquoted/raw JSON literal (not ordinary JSON string escaping), you can use JsonUnquotedLiteral (experimental) but that can produce invalid JSON if misused. [1][2][3] Sources:
Fix the JSON escaping assertion in the serialization test. The assertion at lines 107-110 checks for an unescaped JSON substring, but Update the assertion to verify the escaped version: assertTrue(
"Serialized JSON should contain escaped metadata value",
json.contains("\\\"custom\\\":\\\"data\\\"") || json.contains("{\\\"custom\\\":\\\"data\\\"}")
)🤖 Prompt for AI Agents |
||
|
|
||
| @Test | ||
| fun `UpdateParams with empty unsafeMetadata should be handled correctly`() { | ||
| // Given | ||
| val emptyMetadata = "{}" | ||
| val params = User.UpdateParams(unsafeMetadata = emptyMetadata) | ||
|
|
||
| // When | ||
| val paramsMap = params.toMap() | ||
|
|
||
| // Then | ||
| assertNotNull("Map should not be null", paramsMap) | ||
| assertEquals( | ||
| "Should include unsafe_metadata with empty object", | ||
| emptyMetadata, | ||
| paramsMap["unsafe_metadata"], | ||
| ) | ||
| } | ||
|
|
||
| @Test | ||
| fun `UpdateParams with complex nested unsafeMetadata should be preserved`() { | ||
| // Given | ||
| val complexMetadata = | ||
| """{"user":{"preferences":{"theme":"dark","language":"en"},"settings":{"notifications":true}}}""" | ||
| val params = User.UpdateParams(firstName = "Bob", unsafeMetadata = complexMetadata) | ||
|
|
||
| // When | ||
| val paramsMap = params.toMap() | ||
|
|
||
| // Then | ||
| assertNotNull("Map should not be null", paramsMap) | ||
| assertEquals( | ||
| "Should preserve complex nested metadata", | ||
| complexMetadata, | ||
| paramsMap["unsafe_metadata"], | ||
| ) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Implementation looks good and follows existing patterns.
The addition of
unsafeMetadatatoUpdateParamscorrectly follows the same pattern aspublicMetadataandprivateMetadata. The typeString?is appropriate for the update parameters, matching the existing metadata fields.However, the AI summary incorrectly states that
unsafeMetadatawas added to theUserdata class. The field already exists at line 165, so this appears to be a documentation inconsistency rather than a code change.As per coding guidelines, please add tests to cover the new
unsafeMetadataparameter. The tests should verify:unsafeMetadatasucceedsWould you like me to help generate test cases for this functionality?
Additionally, verify that the
@AutoMapannotation properly handles the new field:🏁 Script executed:
Length of output: 41447
Length of output: 3857
🏁 Script executed:
Length of output: 1622
🏁 Script executed:
Length of output: 2795
Add tests for the new
unsafeMetadatafield in User.update().The implementation correctly follows the existing pattern for
publicMetadataandprivateMetadatafields, with proper@AutoMapand@SerialNameannotations. However, per coding guidelines, tests must be added to cover this new functionality since none exist in the codebase forUser.update()operations.Add tests to verify:
unsafeMetadatasucceedstoMap()