Don't fail with enums in directive parameters.

[meistermeier]

---

Description

Support mapping of enum values in directive parameters.
This does currently fail and I think that this little bit extra for the null-handling can improve the situation.
I am pretty new to the whole GraphQL world and do not know if this makes sense at all.
Since I haven't found a similar test scenario for Kotlin, I skipped this part for this PR and just focused on Scala and Java.

Related to #673

---

Changes were made to:

• Codegen library - Java
• Codegen library - Kotlin
• Codegen library - Scala

[jxnu-liguobin]

Enumeration declaration of OUT should be provided, which is very important for Scala. It will have one more import statement. Enumeration not imported is not available. in scala.

Then, class will be:

import package.model._
import OUTENUM._
import scala.collection.JavaConverters._

/**
 * type with directive using enum value
 */
@javax.annotation.Generated(
    value = Array("com.kobylynskyi.graphql.codegen.GraphQLCodegen"),
    date = "2020-12-31T23:59:59-0500"
)
case class User(
    name: String,
    @com.example.Relationship(type = "FRIEND_WITH", direction = OUT)
    friends: scala.Seq[User]
) {

}

[kobylynskyi]

I think that such null-checks should be done before if (graphQLType instanceof NonNullType) and f (graphQLType instanceof TypeName), so can you please move this verification to the top of the method?
Thanks.

[jxnu-liguobin]

👍

[meistermeier]

Thanks for your swift feedback. I wonder if it makes sense to support Scala in the scope of this PR. As far as I interpret the code around the ...DataModelMapper the annotations are just Strings and would need to get elevated into a class on their own, or?
I don't mind doing this but this will probably blur the focus of the PR.

[kobylynskyi]

@meistermeier, thanks for your contributions. I think that it would be good to make end-to-end changes in a single PR in order to keep track what was made and what's still pending (Java, Scala, Kotlin)
Please let me know your thoughts.

[jxnu-liguobin]

There is no problem with this PR change. I just want us to try our best to make the content of the test file tend to be correct for future reference.

[meistermeier]

Was investigating the needed changes (for Scala) and found out that it is not only that the annotation needs to be sth. like an AnnotationDefinition but also it needs to get hydrated from the directive information. But I cannot see any suitable place in the code where I could do:
Create type definition -> create fields -> create annotations -> ⛔ from annotation to directive meta-information to retrieve the type of the parameters ⛔
So I would suggest (and I can do this) to remove the Scala path and stick with a plain Java approach for now.

Unrelated but since I was on the Scala imports: I removed duplicates from the imports for Enums in the template. (8d63027)

[jxnu-liguobin]

Unrelated but since I was on the Scala imports: I removed duplicates from the imports for Enums in the template. (8d63027)

Thanks, the implementation looks good to me, just like the existing one. However, we may need to use HashSet to transfer to the template later to avoid the template being too complex. But this PR does not need to considere it.

btw, please add one testcase for it , It's good for the future to reconstruct it..

[kobylynskyi]

@jxnu-liguobin could you please review? Thanks

[jxnu-liguobin]

I just think PR lacks some test case. Although logically there is no problem. If you are in a hurry, you can merge.

[meistermeier]

I can add the test case for the import fix later this week. Was a little bit distracted by daily work ;)
Do you think I should remove the Scala part from the initial change because I cannot bring in the import?

[jxnu-liguobin]

Sorry I'm not sure what you mean. 😅

Do you think I should remove the Scala part from the initial change because I cannot bring in the import?

Was investigating the needed changes (for Scala) and found out that it is not only that the annotation needs to be sth. like an AnnotationDefinition but also it needs to get hydrated from the directive information. But I cannot see any suitable place in the code where I could do:
Create type definition -> create fields -> create annotations -> ⛔ from annotation to directive meta-information to retrieve the type of the parameters ⛔
So I would suggest (and I can do this) to remove the Scala path and stick with a plain Java approach for now.

[meistermeier]

There is right now the somehow wrong User.scala test in place where I check for the right annotation.
In there is the enum that does not declare the import for the enum due to the lack of infrastructure as I mentioned #674 (comment)

[jxnu-liguobin]

There is right now the somehow wrong User.scala test in place where I check for the right annotation.
In there is the enum that does not declare the import for the enum due to the lack of infrastructure as I mentioned #674 (comment)

Because enumeration Out must also be declared in src/test/resources/schemas/test.graphqls, then document parser can get it for scala and has a import.

[meistermeier]

Yes, I understand this but the problem is with

type User {
    name: String,
    friends: [User] @relationship(type: "FRIEND_WITH", direction: OUT)
}

enum Direction {
    IN,
    OUT
}

the import will not get triggered in the Scala world. Nowhere to be precise but Java does not complain about this.

Even adding a

directive @relationship (
    type: String,
    direction: Direction
) on FIELD_DEFINITION

won't help because the meta-information are not telling me anything about types in the annotation.
This would need a lot of refactoring -if even possible- to get those information.

Thus my initial question was if I should remove the assumptions in the Scala test code completely.

[jxnu-liguobin]

The problem here may be that ftl does not contains enumeration in the fields, so it is a bug.

Because enumeration in annotation is not in fields.

<#if fields?has_content>
    <#if enumImportItSelfInScala?has_content>
        <#list fields as field>
            <#list enumImportItSelfInScala as enum>
                <#if MapperUtil.isScalaCollection(field.type)>
                    <#if enum == MapperUtil.getGenericParameter(field.type)>
import ${enum}._
                    </#if>
                <#else >
                    <#if enum == field.type>
import ${enum}._
                    </#if>
                </#if>
            </#list>
        </#list>
    </#if>
</#if>

And it's not easy to handle. If processing is forced to be added to the template, the parameter of the annotation should use Direction.OUT, which makes the template more complicated. In view of this, the PR no longer needs to be related to the problem, I will create a new issue to track it.

[kobylynskyi]

@jxnu-liguobin @meistermeier is this ready to be merged to develop and eventually be released as part of the next release (5.3.0)?

[jxnu-liguobin]

@jxnu-liguobin @meistermeier is this ready to be merged to develop and eventually be released as part of the next release (5.3.0)?

Yes

[tejas-sastry]

@kobylynskyi @jxnu-liguobin @meistermeier enums are not working for Kotlin. Is it supported or planned to be supported?

[jxnu-liguobin]

@tejas-sastry Please show the failed code and schema.

[tejas-sastry]

@jxnu-liguobin Sorry. should have given this before asking if it was supported.

schema:

directive @NotNull(message : String, groups: ValidationGroup) on INPUT_FIELD_DEFINITION

input TrustAccountInput
{
  accountId: String! @NotNull(message: "test", groups: CAMS)
}

Plugin config:

<plugin>
                <groupId>io.github.kobylynskyi</groupId>
                <artifactId>graphql-codegen-maven-plugin</artifactId>
                <version>${graphqlcodegen-maven-plugin.version}</version>
                <executions>
                    <execution>
                        <goals>
                            <goal>generate</goal>
                        </goals>
                    </execution>
                </executions>
                <configuration>
                    <graphqlSchemas>
                        <rootDir>${project.basedir}/src/main/resources/graphql/</rootDir>
                    </graphqlSchemas>
                    <generatedLanguage>KOTLIN</generatedLanguage>
                    <generateEqualsAndHashCode>true</generateEqualsAndHashCode>
                    <outputDir>${project.basedir}/target/generated</outputDir>
                    <addGeneratedAnnotation>false</addGeneratedAnnotation>
                    <generateClient>true</generateClient>
                    <packageName>com.intuit.identity.manage.model</packageName>
                    <modelPackageName>com.intuit.identity.manage.model.graphql</modelPackageName>
                    <apiPackageName>com.intuit.identity.manage.model.querybuilders</apiPackageName>
                    <initializeNullableTypes>true</initializeNullableTypes>
                    <directiveAnnotationsMapping>
                        <ValidAccountId>
                            <f>
                                @com.intuit.identity.manage.account.model.constraints.ValidAccountId
                            </f>
                        </ValidAccountId>
                        <NotBlank>
                            <f>
                                @javax.validation.constraints.NotBlank
                            </f>
                        </NotBlank>
                        <NotNull>
                            <f>
                                @javax.validation.constraints.NotNull(message = {{message}}, groups = {{groups}})
                            </f>
                        </NotNull>
                   
                        <Valid>
                            <f>
                                @javax.validation.Valid
                            </f>
                        </Valid>
                        <Validated>
                            <f>
                                @org.springframework.validation.annotation.Validated
                            </f>
                        </Validated>
                    </directiveAnnotationsMapping>
                    <customTypesMapping>
                        <Date>java.time.LocalDate</Date>
                        <DateTime>java.time.LocalDateTime</DateTime>
                        <CAMS>com.intuit.identity.manage.enum.CamsGroup::class</CAMS>
                    </customTypesMapping>
                    <useObjectMapperForRequestSerialization>
                        Date,DateTime
                    </useObjectMapperForRequestSerialization>
                </configuration>
            </plugin>

Generated class:

import com.kobylynskyi.graphql.codegen.model.graphql.GraphQLRequestSerializer
import java.util.StringJoiner

/**
 * Input for providing trust account in different mutation variations
 */
data class TrustAccountInput(
    @field:javax.validation.constraints.NotNull(message = "test", groups = CAMS)
    var accountId: String
) {...}

Error: Unresolved reference: CAMS

My expectation is that the customTypesMapping replaces theCAMS.

[jxnu-liguobin]

@tejas-sastry Thank you for your report. This is indeed a problem. In kotlin, customTypesMapping only handles primitive types. I have opened a new issue for it andthis is a legacy issue left over from the first support for kotlin