Add param to GraphQLRequest to identify GET requests (#2329)

* Use request extensions to identify GET requests

* Propagate `isHttpGetRequest` via `GraphQLRequest` param instead

* Add `@transient` annotation to `isHttpGetRequest`

adapters/quick/src/main/scala/caliban/QuickRequestHandler.scala

@@ -168,10 +168,8 @@ final private class QuickRequestHandler[R](
 
   private def executeRequest(method: Method, req: GraphQLRequest)(implicit
     trace: Trace
-  ): ZIO[R, Response, GraphQLResponse[Any]] = {
-    val calibanMethod = if (method == Method.GET) HttpRequestMethod.GET else HttpRequestMethod.POST
-    HttpRequestMethod.setWith(calibanMethod)(interpreter.executeRequest(req))
-  }
+  ): ZIO[R, Response, GraphQLResponse[Any]] =
+    interpreter.executeRequest(if (method == Method.GET) req.asHttpGetRequest else req)
 
   private def responseHeaders(headers: Headers, cacheDirective: Option[String]): Headers =
     cacheDirective match {
@@ -204,7 +202,7 @@ final private class QuickRequestHandler[R](
             encodeSingleResponse(resp, keepDataOnErrors = !isBadRequest, hasCacheDirective = cacheDirective.isDefined)
         )
       case resp                                                 =>
-        val isBadRequest = resp.errors.contains(HttpRequestMethod.MutationOverGetError)
+        val isBadRequest = resp.errors.contains(HttpUtils.MutationOverGetError)
         Response(
           status = if (isBadRequest) Status.BadRequest else Status.Ok,
           headers = responseHeaders(ContentTypeJson, cacheDirective),

build.sbt

@@ -734,7 +734,9 @@ lazy val enableMimaSettingsJVM =
     mimaPreviousArtifacts  := previousStableVersion.value.map(organization.value %% moduleName.value % _).toSet,
     mimaBinaryIssueFilters := Seq(
       ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.quick.*"),
-      ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.QuickAdapter.*")
+      ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.QuickAdapter.*"),
+      ProblemFilters.exclude[DirectMissingMethodProblem]("caliban.GraphQLRequest.*"),
+      ProblemFilters.exclude[Problem]("caliban.HttpRequestMethod*")
     )
   )
 

core/src/main/scala/caliban/GraphQL.scala

@@ -118,7 +118,7 @@ trait GraphQL[-R] { self =>
                 (for {
                   doc          <- wrap(parseZIO)(parsingWrappers, request.query.getOrElse(""))
                   coercedVars  <- coerceVariables(doc, request.variables.getOrElse(Map.empty))
-                  executionReq <- wrap(validation(request.operationName, coercedVars))(validationWrappers, doc)
+                  executionReq <- wrap(validation(request, coercedVars))(validationWrappers, doc)
                   result       <- wrap(execution(schemaToExecute(doc), fieldWrappers))(executionWrappers, executionReq)
                 } yield result).catchAll(Executor.fail)
               )(overallWrappers, request)
@@ -138,20 +138,20 @@ trait GraphQL[-R] { self =>
           }
 
         private def validation(
-          operationName: Option[String],
+          req: GraphQLRequest,
           coercedVars: Map[String, InputValue]
         )(doc: Document)(implicit trace: Trace): IO[ValidationError, ExecutionRequest] =
           Configurator.ref.getWith { config =>
             Validator.prepare(
               doc,
               typeToValidate(doc),
-              operationName,
+              req.operationName,
               coercedVars,
               config.skipValidation,
               config.validations
             ) match {
-              case Right(value) => checkHttpMethod(config)(value)
-              case Left(error)  => ZIO.fail(error)
+              case Right(value) => checkHttpMethod(config)(req, value)
+              case Left(error)  => Exit.fail(error)
             }
           }
 
@@ -183,16 +183,15 @@ trait GraphQL[-R] { self =>
         private def schemaToExecute(doc: Document) =
           if (doc.isIntrospection) introspectionRootSchema else schema
 
-        private def checkHttpMethod(cfg: ExecutionConfiguration)(req: ExecutionRequest)(implicit
-          trace: Trace
-        ): IO[ValidationError, ExecutionRequest] =
-          if ((req.operationType eq OperationType.Mutation) && !cfg.allowMutationsOverGetRequests)
-            HttpRequestMethod.getWith {
-              case HttpRequestMethod.GET => ZIO.fail(HttpRequestMethod.MutationOverGetError)
-              case _                     => Exit.succeed(req)
-            }
+        private def checkHttpMethod(
+          cfg: ExecutionConfiguration
+        )(gqlReq: GraphQLRequest, req: ExecutionRequest): IO[ValidationError, ExecutionRequest] =
+          if (
+            req.operationType == OperationType.Mutation &&
+            !cfg.allowMutationsOverGetRequests &&
+            gqlReq.isHttpGetRequest
+          ) Exit.fail(HttpUtils.MutationOverGetError)
           else Exit.succeed(req)
-
       }
     }
 

core/src/main/scala/caliban/GraphQLRequest.scala

@@ -13,18 +13,21 @@ case class GraphQLRequest(
   query: Option[String] = None,
   operationName: Option[String] = None,
   variables: Option[Map[String, InputValue]] = None,
-  extensions: Option[Map[String, InputValue]] = None
-) {
+  extensions: Option[Map[String, InputValue]] = None,
+  @transient isHttpGetRequest: Boolean = false
+) { self =>
 
   def withExtension(key: String, value: InputValue): GraphQLRequest =
     copy(extensions = Some(extensions.foldLeft(Map(key -> value))(_ ++ _)))
 
   def withFederatedTracing: GraphQLRequest =
     withExtension(`apollo-federation-include-trace`, StringValue(ftv1))
 
+  private[caliban] def asHttpGetRequest: GraphQLRequest =
+    new GraphQLRequest(query, operationName, variables, extensions, isHttpGetRequest = true)
+
   private[caliban] def isEmpty: Boolean =
     operationName.isEmpty && query.isEmpty && extensions.isEmpty
-
 }
 
 object GraphQLRequest {

core/src/main/scala/caliban/HttpUtils.scala

@@ -1,5 +1,6 @@
 package caliban
 
+import caliban.CalibanError.ValidationError
 import caliban.ResponseValue.{ ObjectValue, StreamValue }
 import caliban.Value.NullValue
 import caliban.wrappers.Caching
@@ -8,6 +9,9 @@ import zio.{ Cause, Chunk, Trace }
 
 private[caliban] object HttpUtils {
 
+  val MutationOverGetError: ValidationError =
+    ValidationError("Mutations are not allowed for GET requests", "")
+
   object DeferMultipart {
     private val Newline        = "\r\n"
     private val ContentType    = "Content-Type: application/json; charset=utf-8"

core/src/test/scala/caliban/interop/jsoniter/GraphQLRequestJsoniterSpec.scala

@@ -47,6 +47,14 @@ object GraphQLRequestJsoniterSpec extends ZIOSpecDefault {
           writeToString(res) ==
             """{"query":"{}","operationName":"op","variables":{"hello":"world","answer":42,"isAwesome":true,"name":null}}"""
         )
+      },
+      test("isHttpGetRequest is ignored when serializing to JSON") {
+        val res = writeToString(GraphQLRequest(isHttpGetRequest = true))
+        assertTrue(res == """{}""")
+      },
+      test("isHttpGetRequest is ignored when deserializing from JSON") {
+        val res = readFromString[GraphQLRequest]("""{"isHttpGetRequest":true}""").isHttpGetRequest
+        assertTrue(!res)
       }
     )
 }

interop/tapir/src/main/scala/caliban/interop/tapir/HttpInterpreter.scala

@@ -73,16 +73,10 @@ object HttpInterpreter {
     def executeRequest[BS](
       graphQLRequest: GraphQLRequest,
       serverRequest: ServerRequest
-    )(implicit streamConstructor: StreamConstructor[BS]): ZIO[R, TapirResponse, CalibanResponse[BS]] =
-      setRequestMethod(serverRequest)(interpreter.executeRequest(graphQLRequest))
-        .map(buildHttpResponse[E, BS](serverRequest))
-
-    private def setRequestMethod(req: ServerRequest)(exec: URIO[R, GraphQLResponse[E]]): URIO[R, GraphQLResponse[E]] =
-      req.method match {
-        case Method.POST => HttpRequestMethod.setWith(HttpRequestMethod.POST)(exec)
-        case Method.GET  => HttpRequestMethod.setWith(HttpRequestMethod.GET)(exec)
-        case _           => exec
-      }
+    )(implicit streamConstructor: StreamConstructor[BS]): ZIO[R, TapirResponse, CalibanResponse[BS]] = {
+      val req = if (serverRequest.method == Method.GET) graphQLRequest.asHttpGetRequest else graphQLRequest
+      interpreter.executeRequest(req).map(buildHttpResponse[E, BS](serverRequest))
+    }
   }
 
   private case class Prepended[R, E](

interop/tapir/src/main/scala/caliban/interop/tapir/TapirAdapter.scala

@@ -138,7 +138,7 @@ object TapirAdapter {
           encodeTextEventStreamResponse(resp)
         )
       case resp                                                 =>
-        val isBadRequest = response.errors.contains(HttpRequestMethod.MutationOverGetError: Any)
+        val isBadRequest = response.errors.contains(HttpUtils.MutationOverGetError: Any)
         (
           MediaType.ApplicationJson,
           if (isBadRequest) StatusCode.BadRequest else StatusCode.Ok,

interop/tapir/src/test/scala/caliban/interop/tapir/TapirAdapterSpec.scala

@@ -266,6 +266,12 @@ object TapirAdapterSpec {
             ZIO
               .serviceWithZIO[SttpBackend[Task, Capabilities]](run(GraphQLRequest(None)).send(_))
               .map(r => assertTrue(r.code.code == 400))
+          },
+          test("returns 400 for mutations over GET methods") {
+            runHttpRequest(
+              method = Method.GET.method,
+              query = """mutation{ deleteCharacter(name: "Amos Burton") }"""
+            ).map(r => assertTrue(r.code.code == 400))
           }
         )
       ),