kotlinx.serialization icon indicating copy to clipboard operation
kotlinx.serialization copied to clipboard

Allow failing on duplicate keys

Open tjerkw opened this issue 3 years ago • 10 comments

WHY

The JSON format allows for duplicate keys in objects:

{'a': 2, 'a': 4}

but the spec says it "SHOULD" not be done.

Different parsers implement it differently. It would by nice to configure the Json parses to fail on duplicate keys

HOW

Allow a setting on Json to fail on duplicate keys:

val json = kotlinx.serialization.json.Json {
        ignoreDuplicateKeys = false (default should be true
    }
``

tjerkw avatar Jul 13 '22 13:07 tjerkw

Unless there are compelling use cases, we would prefer not to introduce such a feature: it's way too easy to misuse it and start ignoring backend errors or API misconfiguration.

A class-specific workaround would be to write your own serializer (or use JsonTransforming one) that deliberately skips duplicates

qwwdfsad avatar Sep 05 '22 12:09 qwwdfsad

A use case of such a feature is to check strict JSON validity. Since the spec does not recommend using duplicate keys, it seems rather natural to allow the user to forbid such a thing. Especially when trying to check the equality between two JSON, the following JSON appear to be the same using kotlinx serializers but they are not


{key: 'value1', key: 'value2'} 
{key: 'value2'}

I don't understand how it is so easy to misuse such a feature though. The error message should be clear and give the key that was duplicated. Since it would be an option that the user has to set if they want such a strict check, well, if they don't want that check they just have to not set the option, which would anyway be the default behaviour.

Are you still firmly against it or can I make a PR for this feature? I'm pretty sure it would be very easy to implement, making a little change in JsonTreeReader.

Lysoun avatar Oct 08 '22 18:10 Lysoun

I honestly was sure that we do fail on duplicate keys and that the request was about allowing duplicate keys, thus misreading the sentence.

The request indeed is reasonable and seems like it should be on by default, though it requires a proper damage assessment.

qwwdfsad avatar Oct 08 '22 19:10 qwwdfsad

@qwwdfsad Hi, I've been working in this issue but I have trouble running the tests with the JVM. I get this error:

> Task :kotlinx-serialization-core:compileKotlinJvm FAILED
e: Module java.base cannot be found in the module graph

Guess it must be linked to my version of the JDK. I tried using 1.8, 11 and 17 (configured in my project structure in IntelliJ and in my preferences, for kotlin compilation target) but it always fails. It seems to be a problem with the project gradle configuration but I'm very surprised that the contributors did not see that. Since I have a test that is not passing yet (I tested it using js), it makes it very hard to debug it. Do you know something about this problem?

Lysoun avatar Oct 09 '22 20:10 Lysoun

Are you encountering this error when running tests from IDE or from the console?

Checked clean build on my machine:

qwwdfsad@qwwdfsad kotlinx.serialization % java -version
java version "11.0.1" 2018-10-16 LTS
Java(TM) SE Runtime Environment 18.9 (build 11.0.1+13-LTS)
Java HotSpot(TM) 64-Bit Server VM 18.9 (build 11.0.1+13-LTS, mixed mode)
qwwdfsad@qwwdfsad kotlinx.serialization % ./gradlew :kotlinx-serialization-json:jvmTest
Starting a Gradle Daemon, 1 incompatible Daemon could not be reused, use --status for details
... executes normally in dev branch... 

I am not aware of this particular problem, but I've seen multiple times that Gradle sporadically fails on JPMS-related functionality. I suggest killing old daemons, cleaning .gradle build cache and try again

qwwdfsad avatar Oct 10 '22 08:10 qwwdfsad

I did it running the tests from my IDE. I'll try your suggestions, thank you.

Lysoun avatar Oct 10 '22 12:10 Lysoun

I managed to run the tests! What I had to do:

  • make sure I had a jdk 11 installed
  • make sure it was my default jdk I'm pretty sure there must be something fishy about the project's gradle configuration because when I ran the compilation task from a terminal, gradle logged this:
'compileJava' task (current target is 11) and 'compileKotlin' task (current target is 1.8) jvm target compatibility should be set to the same Java version.
Using Kotlin compiler version: 1.7.20-RC

The compileKotlin tasks must not be set correctly... Is there an issue for this or should I create one?

Lysoun avatar Oct 27 '22 09:10 Lysoun

The difference is caused by the fact that we should have Kotlin class files produced with majorVersion = 52 (for Android to read), thus, Java 8 target. But we also have to build a multi-release JAR to support the Java module system — and module-info.java is compiled with Java 11. I don't see any possibilities to unify this

sandwwraith avatar Oct 27 '22 12:10 sandwwraith

That means that the only way to run the JVM tests is to set java 11 as default version on one's computer, which can be pretty troublesome when it's not.

Lysoun avatar Oct 27 '22 12:10 Lysoun