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

Properties are not escaped in function body for input builders. #6112

Closed
ebrattli opened this issue Aug 10, 2024 · 10 comments
Closed

Properties are not escaped in function body for input builders. #6112

ebrattli opened this issue Aug 10, 2024 · 10 comments

Comments

@ebrattli
Copy link
Contributor

ebrattli commented Aug 10, 2024

Version

4.0.0

Summary

When generating input builders for the following code, I get an invalid code that doesn't compile because the property in is not escaped when used in the function body of the builder methods.

Steps to reproduce the behavior

This graphql:

input _TimestampCondition {
  eq: Timestamp
  gt: Timestamp
  gte: Timestamp
  in: [Timestamp!]
  isNull: Boolean
  lt: Timestamp
  lte: Timestamp
}

Generates this code

//
// AUTO-GENERATED FILE. DO NOT MODIFY.
//
// This class was automatically generated by Apollo GraphQL version '4.0.0'.
//
package generated.type

import com.apollographql.apollo.api.Optional
import kotlin.Any
import kotlin.Boolean
import kotlin.collections.List

public data class _TimestampCondition(
  public val eq: Optional<Any?> = Optional.Absent,
  public val gt: Optional<Any?> = Optional.Absent,
  public val gte: Optional<Any?> = Optional.Absent,
  public val `in`: Optional<List<Any>?> = Optional.Absent,
  public val isNull: Optional<Boolean?> = Optional.Absent,
  public val lt: Optional<Any?> = Optional.Absent,
  public val lte: Optional<Any?> = Optional.Absent,
) {
  public class Builder {
    private var eq: Optional<Any?> = Optional.Absent

    private var gt: Optional<Any?> = Optional.Absent

    private var gte: Optional<Any?> = Optional.Absent

    private var `in`: Optional<List<Any>?> = Optional.Absent

    private var isNull: Optional<Boolean?> = Optional.Absent

    private var lt: Optional<Any?> = Optional.Absent

    private var lte: Optional<Any?> = Optional.Absent

    public fun eq(eq: Any?): Builder {
      this.eq = Optional.Present(eq)
      return this
    }

    public fun gt(gt: Any?): Builder {
      this.gt = Optional.Present(gt)
      return this
    }

    public fun gte(gte: Any?): Builder {
      this.gte = Optional.Present(gte)
      return this
    }

    public fun `in`(`in`: List<Any>?): Builder {
      this.in = Optional.Present(in)
      return this
    }

    public fun isNull(isNull: Boolean?): Builder {
      this.isNull = Optional.Present(isNull)
      return this
    }

    public fun lt(lt: Any?): Builder {
      this.lt = Optional.Present(lt)
      return this
    }

    public fun lte(lte: Any?): Builder {
      this.lte = Optional.Present(lte)
      return this
    }

    public fun build(): _TimestampCondition = _TimestampCondition(
      eq = eq,
      gt = gt,
      gte = gte,
      in = in,
      isNull = isNull,
      lt = lt,
      lte = lte,
    )
  }
}

In the setter function for in, we see that in is not escaped. this.in = Optional.Present(in). The issue seems to be here where %L is used instead of %N where %L does not escape the value.

Logs

No response

@BoD
Copy link
Contributor

BoD commented Aug 12, 2024

Good catch! Thanks for reporting. Would you be interested in opening a PR?

@ebrattli
Copy link
Contributor Author

I can do that, but I was unsure where to add the test for this. Could you point out where?

@BoD
Copy link
Contributor

BoD commented Aug 12, 2024

Thanks!

  • Add an input type containing an in field, and a query field that uses it, in libraries/apollo-compiler/src/test/graphql/schema.sdl
  • Add a query that uses this field/input type to libraries/apollo-compiler/src/test/graphql/com/example/input_object_type/TestOperation.graphql
  • Run the specific codegen test: testFilter='input_object_type' updateTestFixtures=true ./gradlew :apollo-compiler:test --tests "com.apollographql.apollo.compiler.CodegenTest"
  • Test should pass, and new .expected files should appear, which should be committed

@ebrattli
Copy link
Contributor Author

ebrattli commented Aug 12, 2024

I get this error when trying to run the tests. Is there any more setup I need to find the dependencies?

> testFilter='input_object_type' updateTestFixtures=true ./gradlew :apollo-compiler:test --tests "com.apollographql.apollo.compiler.CodegenTest"

Calculating task graph as no cached configuration is available for tasks: :apollo-compiler:test --tests com.apollographql.apollo.compiler.CodegenTest

> Configure project :build-logic
WARNING: Unsupported Kotlin plugin version.
The `embedded-kotlin` and `kotlin-dsl` plugins rely on features of Kotlin `1.9.23` that might work differently than in the requested version `2.0.10`.

> Task :build-logic:compileKotlin FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':build-logic:compileKotlin'.
> Could not resolve all files for configuration ':build-logic:compileClasspath'.
   > Could not resolve all dependencies for configuration ':build-logic:compileClasspath'.
      > Could not find com.gradle:develocity-gradle-plugin:3.17.6.
        Searched in the following locations:
          - https://repo.maven.apache.org/maven2/com/gradle/develocity-gradle-plugin/3.17.6/develocity-gradle-plugin-3.17.6.pom
        Required by:
            project :build-logic
   > Could not resolve all dependencies for configuration ':build-logic:compileClasspath'.
      > Could not find org.jetbrains.kotlinx:kotlinx-benchmark-plugin:0.4.8.
        Searched in the following locations:
          - https://repo.maven.apache.org/maven2/org/jetbrains/kotlinx/kotlinx-benchmark-plugin/0.4.8/kotlinx-benchmark-plugin-0.4.8.pom
        Required by:
            project :build-logic
   > Could not resolve all dependencies for configuration ':build-logic:compileClasspath'.
      > Could not find com.android.tools.build:gradle:8.2.2.
        Searched in the following locations:
          - https://repo.maven.apache.org/maven2/com/android/tools/build/gradle/8.2.2/gradle-8.2.2.pom
        Required by:
            project :build-logic
   > Could not resolve all dependencies for configuration ':build-logic:compileClasspath'.
      > Could not find me.champeau.gradle:japicmp-gradle-plugin:0.2.8.
        Searched in the following locations:
          - https://repo.maven.apache.org/maven2/me/champeau/gradle/japicmp-gradle-plugin/0.2.8/japicmp-gradle-plugin-0.2.8.pom
        Required by:
            project :build-logic
   > Could not resolve all dependencies for configuration ':build-logic:compileClasspath'.
      > Could not find org.jetbrains.intellij.platform:intellij-platform-gradle-plugin:2.0.0.
        Searched in the following locations:
          - https://repo.maven.apache.org/maven2/org/jetbrains/intellij/platform/intellij-platform-gradle-plugin/2.0.0/intellij-platform-gradle-plugin-2.0.0.pom
        Required by:
            project :build-logic
   > Could not resolve all dependencies for configuration ':build-logic:compileClasspath'.
      > Could not find org.jetbrains.intellij.plugins:gradle-changelog-plugin:2.0.0.
        Searched in the following locations:
          - https://repo.maven.apache.org/maven2/org/jetbrains/intellij/plugins/gradle-changelog-plugin/2.0.0/gradle-changelog-plugin-2.0.0.pom
        Required by:
            project :build-logic
   > Could not resolve all dependencies for configuration ':build-logic:compileClasspath'.
      > Could not find com.android.lint:com.android.lint.gradle.plugin:8.2.2.
        Searched in the following locations:
          - https://repo.maven.apache.org/maven2/com/android/lint/com.android.lint.gradle.plugin/8.2.2/com.android.lint.gradle.plugin-8.2.2.pom
        Required by:
            project :build-logic

* Try:
> The project declares repositories, effectively ignoring the repositories you have declared in the settings.
   To determine how project repositories are declared, configure your build to fail on project repositories.
   For more information, please refer to https://docs.gradle.org/8.9/userguide/declaring_repositories.html#sub:fail_build_on_project_repositories in the Gradle documentation.
> If the artifact you are trying to retrieve can be found in the repository but without metadata in 'Maven POM' format, you need to adjust the 'metadataSources { ... }' of the repository declaration.
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Get more help at https://help.gradle.org.

BUILD FAILED in 640ms
1 actionable task: 1 executed
Configuration cache entry stored.

@martinbonnin
Copy link
Contributor

martinbonnin commented Aug 12, 2024

The project declares repositories, effectively ignoring the repositories you have declared in the settings.

Do you have Gradle init scripts maybe? Or maybe Android Studio changed your build files automatically? This warning shouldn't be printed.

@ebrattli
Copy link
Contributor Author

It seems that IntelliJ or Gradle made some changes to the build files. I'm trying to make a pr now, but I get permission denied.

@martinbonnin
Copy link
Contributor

we do know that this doesn't respect the GQL spec and we will fixing it in future version.

You'll need to push your branch to a fork of yours and open the PR from there.

@martinbonnin
Copy link
Contributor

Closed with #6116. Thanks again 🙏

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Kotlin usage and allow us to serve you better.

@martinbonnin martinbonnin added this to the 4.0.1 milestone Aug 13, 2024
@ebrattli
Copy link
Contributor Author

Thanks as well 🙏

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

No branches or pull requests

3 participants