Skip to content
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

Bug: target class name does not get updated after first schema #30

Closed
volkert-fastned opened this issue Sep 4, 2023 · 12 comments
Closed

Comments

@volkert-fastned
Copy link

volkert-fastned commented Sep 4, 2023

Hi,

While using the Gradle plugin based on this project, I encountered a weird bug in which the target class name gets "stuck" after processing the first of a number of JSON schemas in a directory, leading to each subsequent schema overwriting the previous one, resulting in only a single remaining generated class.

This happens with version 0.92 of both json-kotlin-schema-codegen and version 0.91 of json-kotlin-gradle. I also tried an earlier version of json-kotlin-gradle (I believe I randomly picked 0.87), and the issue existed there as well.

Code to reproduce:

/*
 * This Kotlin source file was generated by the Gradle 'init' task.
 */
package reproducebug

import net.pwall.json.schema.codegen.CodeGenerator
import java.io.File

class App {
    val greeting: String
        get() {
            return "Hello World!"
        }
}

fun main() {
    println(App().greeting)
    val codeGenerator = CodeGenerator()
    codeGenerator.baseDirectoryName = "./generated-sources"
    codeGenerator.basePackageName = "com.example"
    codeGenerator.generate(File(checkNotNull(App::class.java.classLoader.getResource("OCPP-2.0.1_part3_JSON_schemas")).path))
}

the JSON schemas that this code expects in src/main/resources/OCPP-2.0.1_part3_JSON_schemas can be downloaded from the website of the Open Charge Alliance. Free registration is required, and I'm not sure if I can just share the schemas here. You need to select "OCPP 2.0.1 (all files)". and will receive a download link. The ZIP file OCPP-2.0.1_-all_files_1.zip will in turn contain the ZIP file OCPP-2.0.1_part3_JSON_schemas, which contains these JSON schemas.

I'll try to get a clarification whether I'm allowed to immediately share the JSON schemas here as an attachment.

Anyway, once you run this code on these JSON schemas, the output will be as follows (I'm just showing the first couple of lines, because that's enough to see what's going wrong):

Hello World!
15:54:18.819 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/NotifyEVChargingNeedsRequest.json
15:54:18.821 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.182 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/GetLocalListVersionResponse.json
15:54:19.182 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.190 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/ClearChargingProfileResponse.json
15:54:19.191 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.197 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/DeleteCertificateRequest.json
15:54:19.197 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.203 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/UnlockConnectorResponse.json
15:54:19.203 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.208 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/GetVariablesResponse.json
15:54:19.208 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.213 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/UnlockConnectorRequest.json
15:54:19.213 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.294 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/GetLogResponse.json
15:54:19.295 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.303 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/AuthorizeRequest.json
15:54:19.304 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.311 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/HeartbeatResponse.json
15:54:19.312 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
15:54:19.317 INFO  ...all.json.schema.codegen.CodeGenerator: Generating for $HOME/poc/reproducebug/app/build/resources/main/OCPP-2.0.1_part3_JSON_schemas/ClearCacheResponse.json
15:54:19.317 INFO  ...all.json.schema.codegen.CodeGenerator: -- target class com.example.NotifyEVChargingNeedsRequest
(...)

(I replaced my actual home folder with $HOME in this output.)

As you can see, the source file name gets updated properly each log line Generating for ..., but it remains stuck on the first one in each log line -- target class ....

And indeed, once the application finishes, only one file NotifyEVChargingNeedsRequest.kt ends up in the generated sources.

I briefly looked into the code, and my guess it that some kind of instance variable in CodeGenerator.kt does not seem to get updated properly. Possibly classNameMapping?

Anyway, let me know if I can offer assistance in solving this issue.

Thanks for sharing this very useful project with the world!

@volkert-fastned
Copy link
Author

I just tried renaming the directory src/main/resources/OCPP-2.0.1_part3_JSON_schemas to src/main/resources/schemas, to check whether the complex schema directory name with dots, hyphens and underscores could be a factor somehow, but that didn't make any difference. The target class name remains stuck after the first processed schema.

@volkert-fastned
Copy link
Author

Okay, I did some debugging of the code, and the culprit seems to be in Parser.kt, specifically this function:

    fun parse(file: File): JSONSchema {
        if (!file.isFile)
            throw JSONSchemaException("Invalid file - $file")
        val uri = file.toURI()
        schemaCache[uri]?.let { return it }
        val json = jsonReader.readJSON(file)
        return parse(json, uri)
    }

Why is it caching the URI, which is different between each schema file, and also used to determine the output class name? Am I missing something obvious here 🤔

@volkert-fastned
Copy link
Author

volkert-fastned commented Sep 4, 2023

According to #13, this is apparently intended behavior in Parser.parse(...).

A more likely suspect might be private fun addTarget(subDirectories: List<String>, schema: JSONSchema, source: String, json: JSONValue) in CodeGenerator.kt, because this is the funcation that determines the className from the uri and appears to do some complex checks and parsing while doing so.

It's also interesting that classNameMapping remains empty (size 0) in each invocation of this function.

@volkert-fastned
Copy link
Author

The only place in the code where any entries are added to classNameMapping is in the function configure(), but that function isn't invoked at all in this case, since no config.json file is supplied. But that should be optional, right?

@volkert-fastned
Copy link
Author

volkert-fastned commented Sep 4, 2023

Okay, another clue. I supplied the following config.json file:

{
  "classNames": {
    "urn:OCPP:Cp:2:2020:3:AuthorizeRequest": "AuthorizeRequest",
    "urn:OCPP:Cp:2:2020:3:NotifyEVChargingNeedsRequest": "SomeDifferentClassName"
  },
  "derivePackageFromStructure": true
}

And now it writes out all the parsed schemas as SomeDifferentClassName.kt, including AuthorizeRequest.json, even though I had explicitly specified the URI and the desired class name for that schema in the config file as well.

So it appears that the schema URI gets cached upon the first parsed JSON schema, and does not get updated for each subsequently parsed schema, even though the file name of each generated class is based on the schema URI.

So whether or not there are any mappings in classNameMapping does not seem to matter here.

Also, changing derivePackageFromStructure to false has not effect on this either.

@volkert-fastned
Copy link
Author

addTarget

It's not the complex logic in private fun addTarget(subDirectories: List<String>, schema: JSONSchema, source: String, json: JSONValue), since inside each iteration in the run block of that function, the uri is already always the same cached one.

It really seems like the caching mechanism it its current form doesn't seem to be working well with directories containing multiple JSON schemas, each with different URIs (URNs in the $id field).

In particular, the following step seems to never take place:

When parsing of an individual schema is complete, it replaces the dummy entry with the actual schema reference.
(As quoted from #13 (comment))

@pwall567
Copy link
Owner

pwall567 commented Sep 5, 2023

Thank you for providing such extensive information to assist with diagnosing this issue.

Your analysis is mostly correct, but the underlying problem is that the caching mechanism uses as its key the extended URI, that is, the original URI plus the pointer as a fragment (the portion after the '#'), but the function that it uses to create that extended URI (in the Java URI class) does not operate as expected. That's not to say that the Java class is wrong, just that it does not work in the way that I thought it would.

The problem arises when the URI is a URN (or any other form of "opaque" URI), and that is a case that I had not tested extensively.

I can see a straightforward fix for this and I will deploy a new version as quickly as possible, but that may take a day or more.

Thank you again for bringing this to my attention, and for persevering in your attempts to use the code generator.

Regards,
Peter Wall

@volkert-fastned
Copy link
Author

volkert-fastned commented Sep 5, 2023

Thank you for your quick reply and for willing to release a fix so quickly! I appreciate that. 🙂

By the way, to make things more challenging, I've been trying to use your Gradle plugin in a Kotlin Multiplatform project, and it seems like I actually managed to get that working with some tweaks in the build script file.

For one thing, the code generator defaults to using Java-specific types such as Java JSR-310 date/time classes and BigDecimal. Obviously, that causes the Kotlin/Native and Kotlin/JS builds in the project to fail. But with custom classMappings, I've been able to work around that, at least so far.

Also, in JSONSchemaCodegenPlugin.kt, there is the line project.tasks.findByName("compileKotlin")?.dependsOn(generateTask), which doesn't work in a Multiplatform project, since there are different kinds of variants of compileKotlin, namely a common one and one for each specific platform.

Instead, I've found the following workaround in build.gradle.kts to work well:

project.tasks.withType<KotlinCompileCommon>().configureEach {
    dependsOn(project.tasks.findByName("generate"))
}

Anyway, if I end up getting it fully working, I'll be happy to share all of the needed workarounds with you, so that you can perhaps make the Gradle plugin more Kotlin-Multiplatform-friendly out of the box.

Thanks again!

@pwall567
Copy link
Owner

pwall567 commented Sep 6, 2023

OK, version 0.93 should fix the issue with the use of URN as $id. Let me know if you have any further problems.

And I'm very interested to about your workarounds to get the Multiplatform project working – I'm always keen to make the code generator usable by a wider audience. I'd be happy to incorporate your suggestions into the project if you're willing to share them. (i didn't incorporate your suggestion regarding the dependsOn this time, but I'll check it out and incorporate something based on that in the next version of the plugin.)

Regards,
Peter

@pwall567
Copy link
Owner

pwall567 commented Sep 6, 2023

Oops!

I deployed version 0.93 of json-kotlin-schema-codegen, but then when I updated json-kotlin-gradle I forgot to update all the dependencies. That means I have had to do an additional release of json-kotlin-gradle, version 0.93.1.

Sorry about that!

@volkert-fastned
Copy link
Author

Yes! I can confirm that with version 0.93.1 of json-kotlin-gradle, this has indeed been fixed.

Thank you for releasing a fix so quickly.

One small thing I noticed, though: even though version 0.93.1 of json-kotlin-gradle has indeed been published to the Maven Central Repository, it has not yet been tagged on the GitHub project page of that project. No big deal, but perhaps you forgot to do so?

Anyway, I'm resolving this ticket, since it has been resolved.

@pwall567
Copy link
Owner

Thanks for pointing out that I had forgotten to push the changes to GitHub ☹️

That has now been done 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants