ermadmi78 / kobby

Kobby is a codegen plugin of Kotlin DSL Client by GraphQL schema. The generated DSL supports execution of complex GraphQL queries, mutation and subscriptions in Kotlin with syntax similar to native GraphQL syntax.
Apache License 2.0
83 stars 4 forks source link

Graphql enums with "name" field #21

Closed beepsoft closed 2 years ago

beepsoft commented 2 years ago

Hi,

first of all, kobby is a life saver, thanks for making it! I would really like to use it as it is very similar to https://github.com/mobxjs/mst-gql, which I use on the frontend.

I just tried each and every graphql kotlin and java client generation library and they are close to unusable with such huge schemas, which are generated by Hasura.

However, one problem I am facing with kobby right now is that in my Hasura generated schema I have an enum like this:

# update columns of table "user"
enum user_update_column {
  # column name
  email

  # column name
  id

  # column name
  name
}
/**
 *  update columns of table "user"
 */
public enum class user_update_column {
  /**
   *  column name
   */
  email,
  /**
   *  column name
   */
  id,
  /**
   *  column name
   */
  name
}

This is invalid and the kotlin compiler fails with

Conflicting declarations: public final val name: String, enum entry name

It seems that in kotlin one cannot have an enum field called name. Not even with backticks.

https://discuss.kotlinlang.org/t/is-name-not-allowed-in-enums/24191/3

Can this somehow be fixed?

ermadmi78 commented 2 years ago

Hi.

Thanks for finding the bug, I'll try to fix it as soon as possible.

ermadmi78 commented 2 years ago

To avoid conflict with the Kotlin enum name property, Kobby will rename the GraphQL enum name to Kotlin __name and add Jackson @JsonProperty annotation to ensure that the renamed enum maps correctly to JSON:

/**
 *  update columns of table "user"
 */
public enum class user_update_column {
  /**
   *  column name
   */
  email,
  /**
   *  column name
   */
  id,
  /**
   *  column name  
   * > see https://github.com/ermadmi78/kobby/issues/21
   */
  @JsonProperty("name")
  __name,
}
beepsoft commented 2 years ago

Great! I think ordinal should also be escaped this way as well as this is also a field of Java enum

https://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html

I just checked it, ordinal would fail as well:

https://play.kotlinlang.org/#eyJ2ZXJzaW9uIjoiMS42LjEwIiwicGxhdGZvcm0iOiJqYXZhIiwiYXJncyI6IiIsIm5vbmVNYXJrZXJzIjp0cnVlLCJ0aGVtZSI6ImlkZWEiLCJjb2RlIjoiZW51bSBjbGFzcyBDb2x1bW5OYW1lcyB7XG4gICAgaWQsXG4gICAgbmFtZSxcbiAgICBvcmRpbmFsLFxufVxuXG5mdW4gbWFpbigpIHtcbiAgICBwcmludGxuKENvbHVtbk5hbWVzLm5hbWUpXG59In0=

(I was also going to suggest that maybe it would make sense to make all enum fields uppercase with a @JsonProperty on them with the actual name, which would make the enums conform to the Kotlin naming conventions and surely avoid any name collisions. However, I just realized that there could be an enum field foo and Foo as well, and then we would have another kind of name collision when uppercasing, so it is not worth it.)

ermadmi78 commented 2 years ago

Yes, thank you, ordinal will be renamed too:

# update columns of table "user"
enum user_update_column {
    # column name
    email

    # column name
    id

    # column name
    name

    ordinal
}
/**
 *  update columns of table "user"
 */
public enum class user_update_column {
  /**
   *  column name
   */
  email,
  /**
   *  column name
   */
  id,
  /**
   *  column name  
   * > see https://github.com/ermadmi78/kobby/issues/21
   */
  @JsonProperty("name")
  __name,
  /**
   * see https://github.com/ermadmi78/kobby/issues/21
   */
  @JsonProperty("ordinal")
  __ordinal,
}
ermadmi78 commented 2 years ago

Fixed in release 1.4.1

@beepsoft Could you give me feedback - did this fix help you?

beepsoft commented 2 years ago

Sure, but it doesn't seem to be available in maven central yet. Or which repo should I use?

https://mvnrepository.com/artifact/io.github.ermadmi78/kobby-generator-kotlin

ermadmi78 commented 2 years ago

The plugin is already in the Maven Central, it's just that the UI hasn't been updated yet. Just update the version to 1.4.1 in the project.

beepsoft commented 2 years ago

OK, now generation runs with 1.4.1.

At first kobby generated this:

  /**
   *  column name  
   * > see https://github.com/ermadmi78/kobby/issues/21
   */
  __name,

I had to manually turn on Jackson support for DTO-s for this to be generated:

  /**
   *  column name  
   * > see https://github.com/ermadmi78/kobby/issues/21
   */
  @JsonProperty("name")
  __name,

I haven't actually run a query, which uses this enum but the generation now seems OK, IntelliJ reports no problems.

A side note: I could not compile my project yet, because I have 2950 files generated and the compilation now fails with:

[ERROR] Caused by: org.jetbrains.org.objectweb.asm.MethodTooLargeException: Method too large: ...graphql/client/entity/impl/mutation_rootProjectionImpl.___innerBuild$diwas_java_backend (Ljava/util/Set;Ljava/lang/StringBuilder;Ljava/lang/StringBuilder;Ljava/util/Map;)V

This ___innerBuild method is 18141 lines long.

ermadmi78 commented 2 years ago

It is a great testcase 😀

ermadmi78 commented 2 years ago

This ___innerBuild method is 18141 lines long.

I created an issue to solve this problem - #22