blue-veery-gmbh / spring-rest-2-ts

spring rest 2 ts is typescript generator which produces data model and services in typescript based on Spring MVC annotations. It supports generation for Angular and React
MIT License
64 stars 17 forks source link

Custom Nullability of Types #24

Closed BloodWorkXGaming closed 2 years ago

BloodWorkXGaming commented 2 years ago

Hello again,

As I am working on a kotlin Project I am trying to implement the nullability check in a way it uses the nullable type hints for that. While doing that I ran into a couple of Problems:

  1. setAsNullableType seems to be never called (in ModelClassesToTsInterfacesConverter.java) as tested with breakpoints
  2. The nullable annotation doesn't seem to be used at all (maybe because of Kotlin interop, but doubt that)
  3. In the resulting ts there is a | null on some types, however I don't really understand where and how (maybe due to kotlin auto boxing?)

I tried to extend the ModelClassesToTsInterfacesConverter to check for kotlin nullable, however it never seems to be called either even tho I set it with tsGenerator.setModelClassesConverter(KConverter(jacksonObjectMapper))

    class KConverter(
        objectMapper: com.blueveery.springrest2ts.converters.ObjectMapper?
    ) : ModelClassesToTsInterfacesConverter(objectMapper) {
        override fun setAsNullableType(
            property: Property,
            tsField: TSField,
            nullableTypesStrategy: NullableTypesStrategy
        ) {
            println("---")
            println(property)
            val kp = property.field.kotlinProperty ?: return
            println(kp)
            tsField.isNullable = kp.returnType.isMarkedNullable
        }
    }

Thank you very much for any help :)

tomasz-wozniak75 commented 2 years ago

Hey Here is a problem because I don't know Kotlin but my colleges are using generator with Kotlin and in fact they had to create own nullable strategy.

There are following conditions under which nullable strategy is called:

  1. com/blueveery/springrest2ts/converters/ModelClassesToTsInterfacesConverter.java:74 Interface has properties to iterate in the loop
  2. com/blueveery/springrest2ts/converters/ModelClassesToTsInterfacesConverter.java:76 property is mapped to single TS field (Jackson unwrap could map one java field to many ts fields)
  3. com/blueveery/springrest2ts/converters/ModelClassesAbstractConverter.java:44 getter type is defined for TS field if these conditions are met nullable strategy is called.

I think that converter is not required for such problem, it is enough to set your nullable strategy in the generator: KotlinNullableTypesStrategy nullableTypesStrategy = new KotlinNullableTypesStrategy(); tsGenerator.setNullableTypesStrategy(nullableTypesStrategy);

Let me know if my hints solved your problem if not I will contact my colleges to learn if some special setup is required for Kotlin

BloodWorkXGaming commented 2 years ago

Thanks for the help! The problem actually wasn't anything with my generator code.. I set the model class conditions twice instead of using the Or filter (and therefore not generating anything)? May I suggest a warning here when it is set multiple times?

Sadly the NullableTypesStrategy doesn't have enough information inside of it to get the information whether it is a nullable type or not. That is only present on the property but not on the Type anymore.

Therefore my current generator Code is:

 class KConverter(
        objectMapper: com.blueveery.springrest2ts.converters.ObjectMapper?
    ) : ModelClassesToTsInterfacesConverter(objectMapper) {
        override fun setAsNullableType(
            property: Property,
            tsField: TSField,
            nullableTypesStrategy: NullableTypesStrategy
        ) {
            if (property.field?.kotlinProperty?.returnType?.isMarkedNullable == true) {
                println("setting $property to null (from field)")
                tsField.isNullable = true

                return
            }

            if (property.getterType != null) {
                nullableTypesStrategy.setAsNullableType(property.getterType, property.declaredAnnotations, tsField)
                if (property.getter?.kotlinFunction?.returnType?.isMarkedNullable == true) {
                    println("setting $property to null (from getter)")
                    tsField.isNullable = true
                }

                return
            }

            if (property.setterType != null && property.getterType == property.setterType) {
                nullableTypesStrategy.setAsNullableType(property.setterType, property.declaredAnnotations, tsField)

                if (property.setter?.kotlinFunction?.returnType?.isMarkedNullable == true) {
                    println("setting $property to null (from setter)")

                    tsField.isNullable = true
                }
            }
        }
    }

on a further note: in line 44 in ModelClassesAbstractConverter there are two if clauses:

        if (property.getGetterType() != null) {
            nullableTypesStrategy.setAsNullableType(property.getGetterType(), property.getDeclaredAnnotations(), tsField);
            return;
        }

        if (property.getSetterType() != null && Objects.equals(property.getGetterType(), property.getSetterType())) {
            nullableTypesStrategy.setAsNullableType(property.getSetterType(), property.getDeclaredAnnotations(), tsField);
        }

isn't the second one impossible to be ever entered? As the value for the getter is always null after the first if (due to the return) and the setter may also not be null, however that results to null != notnull in the equals?

BloodWorkXGaming commented 2 years ago

In the meantime I noticed another wierd interaction:

I can seem to get custom Annotations working with the HasAnnotationJavaTypeFilter. It just always says false doesn't matter whether it has the annotation or not. Wit the debugger inside the accept even says that annotation and my custom class are different?

If I however use a different annotation, doesn't matter what, like e.g. @JsonInclude it suddenly works. Any idea why that could happen?

tomasz-wozniak75 commented 2 years ago

Hey I will try to add to TSField reference to property from which it was created and You can get all required information.

The second condition is wrong should be

        if (property.getSetterType() != null && !Objects.equals(property.getGetterType(), property.getSetterType())) {
            nullableTypesStrategy.setAsNullableType(property.getSetterType(), property.getDeclaredAnnotations(), tsField);
        }

the first case is when getter and settter types are the same, second if they are different and setter type is defined we can say if can set null value, in other words we can define nullability of setter and getter types. if condition will be fixed. I will raise separate issue for that. Thanks :) https://github.com/blue-veery-gmbh/spring-rest-2-ts/issues/25

I will test in the evening HasAnnotationJavaTypeFilter

tomasz-wozniak75 commented 2 years ago

Hey Annotation in the HasAnnotationJavaTypeFilter has to have runtime retention, I added proper validation https://github.com/blue-veery-gmbh/spring-rest-2-ts/commit/800ccd58b3b4ff5248e26a042fe61e7e5ba708e1

BloodWorkXGaming commented 2 years ago

Hey, thanks for checking, however that's the wierd part. I had a runtime retention It looked like this

@Target(TYPE)
@Retention(RUNTIME)
public @interface ControllerResponse {
    String name() default "";
}

The same thing happens with the baseclass filter.. it wierdly doesn't find any of my own classes. I highly doubt it has anything to do with your code, probably a bug in the java/kotlin/gradle interaction or something.

A workaround would be a BaseType and/or Annotation already present in the library for that purpose (therefore another annotation doesn't have to be 'recycled')

tomasz-wozniak75 commented 2 years ago

Hey But maybe it is not a problem with annotation but with package scanning for classes, is filter getting required class for check if it has annotation?

tomasz-wozniak75 commented 2 years ago

In version-1.4.0 TSField has reference to Property from which it was created, ticket #25 has also been fixed so I will close this ticket