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

Usage from Kotlin #65

Open
chrischandler25 opened this issue Jan 13, 2019 · 10 comments
Open

Usage from Kotlin #65

chrischandler25 opened this issue Jan 13, 2019 · 10 comments

Comments

@chrischandler25
Copy link

Hi.

Thanks for creating this great tool. I've been using it in a Kotlin application, which has been really simple and worked brilliantly for my requirement.

I have run into a little problem though. I am hoping to remove a field, from one of the GRPC types. I followed your documentation and examples, but think this may fall over in a difference between Kotlin and java. Kotlin by default creates a property getter when a read only var is declared. It looks to me as though you expect only field adding @SchemaModification annotations to be associated to methods, so it's reporting the following exception:

Jan 13, 2019 10:31:15 PM com.google.inject.internal.MessageProcessor visit
INFO: An exception was caught and reported. Message: java.lang.NoSuchMethodException: com.google.api.graphql.rejoiner.SchemaModification.getDescriptor()
java.lang.RuntimeException: java.lang.NoSuchMethodException: com.google.api.graphql.rejoiner.SchemaModification.getDescriptor()
at com.google.api.graphql.rejoiner.SchemaModule.configure(SchemaModule.java:228)

It is possible to annotate the variable definition with @JvmField which will prevent the getter creation and use a Java field, but then the @SchemaModification annotation seems to no longer be applied to the field.

I appreciate that this is a problem of using rejoiner from a language other than the intended, but wondered if you might be able to suggest any different approach to remove the field?

Thanks again
Chris

@siderakis
Copy link
Member

Can you please add a code snippet that results in the error.

@siderakis
Copy link
Member

BTW someone created an Kotlin example at https://github.com/dbaggett/medallion

@chrischandler25
Copy link
Author

chrischandler25 commented Jan 14, 2019

The Kotlin example is useful to see, although unfortunately doesn't utilise the remove field functionality.

My first attempt was:

class AuthServiceSchemaModule : SchemaModule() {
    @SchemaModification
    val removeRegisterUserRemoteAddr = Type
        .find(RegisterUserRequest.getDescriptor())
        .removeField("remoteAddr")!!

This results in the error:
java.lang.NoSuchMethodException: com.google.api.graphql.rejoiner.SchemaModification.getDescriptor()
My Java is basic, but I think I can see why this is happening.

Line 228 in SchemaModule.java I think expects the only @SchemaModification annotations on a method to be for adding fields. As Kotlin creates a getter behind the scenes this is conflicting with the intentions of SchemaModule.

The decompiled Java code of this approach is:

public final class AuthServiceSchemaModule extends SchemaModule {
   @NotNull
   private final TypeModification removeRegisterUserRemoteAddr;

   /** @deprecated */
   // $FF: synthetic method
   @SchemaModification
   public static void removeRegisterUserRemoteAddr$annotations() {
   }

   @NotNull
   public final TypeModification getRemoveRegisterUserRemoteAddr() {
      return this.removeRegisterUserRemoteAddr;
   }

Another thing I tried was adding the @JvmField annotation, which tells the compiler to use a java field. The problem here is that it appears to have created a field, but rather than assign the value inline it has done it from a constructor:

public final class AuthServiceSchemaModule extends SchemaModule {
   @JvmField
   @NotNull
   public final TypeModification removeRegisterUserRemoteAddr;

   /** @deprecated */
   // $FF: synthetic method
   @SchemaModification
   public static void removeRegisterUserRemoteAddr$annotations() {
   }

   public AuthServiceSchemaModule() {
      TypeModification var10001 = Type.find((GenericDescriptor)RegisterUserRequest.getDescriptor()).removeField("remoteAddr");
      if (var10001 == null) {
         Intrinsics.throwNpe();
      }

      this.removeRegisterUserRemoteAddr = var10001;
   }

This approach results in the same error. Any ideas would be greatly appreciated.

Regards
Chris

@chrischandler25
Copy link
Author

chrischandler25 commented Jan 14, 2019

I've got a little closer to a solution with this. It turns out that by prefixing the annotation with @field the compiler will ensure that the annotation is attached to the java backing field.

The generated code now looks more sensible, as follows:

public final class AuthServiceSchemaModule extends SchemaModule {
   @SchemaModification
   @JvmField
   @NotNull
   public final TypeModification removeRegisterUserRemoteAddr;

   public AuthServiceSchemaModule() {
      TypeModification var10001 = Type.find((GenericDescriptor)RegisterUserRequest.getDescriptor()).removeField("remoteAddr");
      if (var10001 == null) {
         Intrinsics.throwNpe();
      }

      this.removeRegisterUserRemoteAddr = var10001;
   }
}

This still doesn't seem to have any effect however. I wonder if I'm misunderstanding the concept of removing fields.

I have a field for the remote users IP address in my GRPC service, which I want to populate from the servlet request rather than have supplied by the GraphQL called. My aim is to remove this field from the GraphQL schema.

I am on the wrong track here?

Chris

@siderakis
Copy link
Member

siderakis commented Jan 14, 2019 via email

@chrischandler25
Copy link
Author

I can't find any way to have the value assigned inline, but it was suggested to me that the bytecode is likely the same. They seemed to think that in Java when values are assigned inline a default constructor is created by the compiler for the assignment.

@siderakis
Copy link
Member

Do you know why it's casting to GenericDescriptor? I wonder if you could look at the fields of TypeModification if you set a breakpoint.

As a temporary workaround could you include a Java SchemaModule that includes removeRegisterUserRemoteAddr?

@siderakis
Copy link
Member

Ohh, can you try Type.find("RegisterUserRequest")....

@chrischandler25
Copy link
Author

I'm not sure about the cast, but don't think it would make any difference.

I had tried using a string for the type name in addition to the getDescriptor approach with no noticeable difference. They both result in a RemoveFieldByName object with the values:

fieldNameToRemove = "remoteAddr"
typeName = "RegisterUserRequest"

I've also tried using the following Java code:

public class AuthServiceSchemaExtModule extends SchemaModule {
    @SchemaModification
    TypeModification removeRegisterUserRemoteAddr = Type
            .find("RegisterUserRequest")
            .removeField("remoteAddr");

    @SchemaModification
    TypeModification removeAuthenticateUserRemoteAddr = Type
            .find(AuthenticateUserRequest.getDescriptor())
            .removeField("remoteAddr");
}

But even that appears to have no effect.

I tried to follow your schema provider test -https://github.com/google/rejoiner/blob/7f18f8c6056cdb2f2cf54237b17856f08c794df4/rejoiner/src/test/java/com/google/api/graphql/rejoiner/SchemaProviderModuleTest.java
I'm a little confused though as even the comment suggests it may not work?

// TODO: this should be empty, currently type modifications only apply to types
// annotated with ExtraTypes.

@chrischandler25
Copy link
Author

I've been looking at this a little further as I'd love to be able to get it working.

Stepping through the modifyTypes method of the ProtoRegistry.java class, I notice that for my two defined type RegisterUserRequest and RegisterUserResponse, 2 additional mapping types have been created, Input_RegisterUserRequest, and Input_RegisterUserResponse.

The field removals are actually applied to my defined types (without the Input_ prefix), however the line

if (mapping.get(key) instanceof GraphQLObjectType) {

skips the modification code for each of the Input_ types. It seems that the Input types are the ones exposed in the schema (for input values), so it makes some sense that the expected fields aren't being removed. Should an Input_ type be produced for the return types as it seems to be (although this isn't causing any issue)? I'd probably have to put in a fair amount of effort to understand why the Input types are generated, but was hoping you may be able to point me in the right direction?

Regards
Chris

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