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

Take default values into account when computing field keys #5384

Merged
merged 4 commits into from
Nov 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
10 changes: 5 additions & 5 deletions libraries/apollo-api/api/apollo-api.api
Original file line number Diff line number Diff line change
Expand Up @@ -270,16 +270,17 @@ public abstract interface class com/apollographql/apollo3/api/BuilderScope {
}

public final class com/apollographql/apollo3/api/CompiledArgument {
public synthetic fun <init> (Ljava/lang/String;Ljava/lang/Object;ZZLkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Ljava/lang/String;Lcom/apollographql/apollo3/api/Optional;ZZLkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun getName ()Ljava/lang/String;
public final fun getValue ()Ljava/lang/Object;
public final fun getValue ()Lcom/apollographql/apollo3/api/Optional;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking change.

public final fun isKey ()Z
}

public final class com/apollographql/apollo3/api/CompiledArgument$Builder {
public fun <init> (Ljava/lang/String;Ljava/lang/Object;)V
public fun <init> (Ljava/lang/String;)V
public final fun build ()Lcom/apollographql/apollo3/api/CompiledArgument;
public final fun isKey (Z)Lcom/apollographql/apollo3/api/CompiledArgument$Builder;
public final fun value (Ljava/lang/Object;)Lcom/apollographql/apollo3/api/CompiledArgument$Builder;
}

public final class com/apollographql/apollo3/api/CompiledCondition {
Expand All @@ -305,7 +306,7 @@ public final class com/apollographql/apollo3/api/CompiledField : com/apollograph
public final fun getType ()Lcom/apollographql/apollo3/api/CompiledType;
public final fun nameWithArguments (Lcom/apollographql/apollo3/api/Executable$Variables;)Ljava/lang/String;
public final fun newBuilder ()Lcom/apollographql/apollo3/api/CompiledField$Builder;
public final fun resolveArgument (Ljava/lang/String;Lcom/apollographql/apollo3/api/Executable$Variables;)Ljava/lang/Object;
public final fun resolveArgument (Ljava/lang/String;Lcom/apollographql/apollo3/api/Executable$Variables;)Lcom/apollographql/apollo3/api/Optional;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. An argument with a variable value is possibly absent if that variable is absent as well and unless we change the signature, there's no way to account for that. We could go through deprecation cycles but given this is a low level API, I'm hoping this is fine.

}

public final class com/apollographql/apollo3/api/CompiledField$Builder {
Expand Down Expand Up @@ -351,7 +352,6 @@ public final class com/apollographql/apollo3/api/CompiledGraphQL {
public static final fun -notNull (Lcom/apollographql/apollo3/api/CompiledType;)Lcom/apollographql/apollo3/api/CompiledNotNullType;
public static final fun isComposite (Lcom/apollographql/apollo3/api/CompiledNamedType;)Z
public static final fun keyFields (Lcom/apollographql/apollo3/api/CompiledNamedType;)Ljava/util/List;
public static final fun resolveVariables (Ljava/lang/Object;Lcom/apollographql/apollo3/api/Executable$Variables;)Ljava/lang/Object;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another breaking change. This one was an accidental public I'd say

}

public final class com/apollographql/apollo3/api/CompiledListType : com/apollographql/apollo3/api/CompiledType {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package com.apollographql.apollo3.api
import com.apollographql.apollo3.annotations.ApolloDeprecatedSince
import com.apollographql.apollo3.annotations.ApolloDeprecatedSince.Version.v4_0_0
import com.apollographql.apollo3.annotations.ApolloExperimental
import com.apollographql.apollo3.api.json.ApolloJsonElement
import com.apollographql.apollo3.api.json.BufferedSinkJsonWriter
import com.apollographql.apollo3.api.json.writeAny
import okio.Buffer
Expand All @@ -28,13 +29,48 @@ class CompiledField internal constructor(
get() = alias ?: name

/**
* Resolves field argument value by [name]. If the argument contains variables, replace them with their actual value
* Resolves field argument value by [name].
*
* @return [Optional.Absent] if no runtime value is present for this argument else returns the argument
* value with variables substituted for their values.
*/
fun resolveArgument(
name: String,
variables: Executable.Variables,
): Any? {
return resolveVariables(arguments.firstOrNull { it.name == name }?.value, variables)
): Optional<ApolloJsonElement> {
val argument = arguments.firstOrNull { it.name == name }
if (argument == null) {
// no such argument
return Optional.Absent
}
if (argument.value is Optional.Absent) {
// this argument has no value
return Optional.Absent
}

val result = resolveVariables(argument.value.getOrThrow(), variables)
if (result is Optional.Absent) {
// this argument has a variable value that is absent
return Optional.Absent
}
return Optional.present(result)
}

/**
* @return a map where the key is the name of the argument and the value the JSON value of that argument
*
* Absent arguments are not returned
*/
@ApolloExperimental
fun resolveArguments(variables: Executable.Variables, filter: (CompiledArgument) -> Boolean= {true}): Map<String, ApolloJsonElement> {
val arguments = arguments.filter(filter).filter { it.value is Optional.Present<*> }
if (arguments.isEmpty()) {
return emptyMap()
}
val map = arguments.associate { it.name to it.value.getOrThrow() }

@Suppress("UNCHECKED_CAST")
return resolveVariables(map, variables) as Map<String, ApolloJsonElement>
}

/**
Expand All @@ -43,21 +79,15 @@ class CompiledField internal constructor(
* This is mostly used internally to compute records
*/
fun nameWithArguments(variables: Executable.Variables): String {
val filterOutPaginationArguments = arguments.any { it.isPagination }
val arguments = if (filterOutPaginationArguments) {
this.arguments.filterNot { it.isPagination }
} else {
this.arguments
}
val arguments = resolveArguments(variables) { !it.isPagination }
if (arguments.isEmpty()) {
return name
}
val map = arguments.associateBy { it.name }.mapValues { it.value.value }
val resolvedArguments = resolveVariables(map, variables)

return try {
val buffer = Buffer()
val jsonWriter = BufferedSinkJsonWriter(buffer)
jsonWriter.writeAny(resolvedArguments)
jsonWriter.writeAny(arguments)
jsonWriter.close()
"${name}(${buffer.readUtf8()})"
} catch (e: Exception) {
Expand Down Expand Up @@ -281,7 +311,7 @@ class InputObjectType(

class EnumType(
name: String,
val values: List<String>
val values: List<String>,
) : CompiledNamedType(name)

/**
Expand All @@ -304,40 +334,52 @@ fun CompiledType.list() = CompiledListType(this)
class CompiledVariable(val name: String)

/**
* The Kotlin representation of a GraphQL argument
* The Kotlin representation of a GraphQL value
*
* value can be
* - String, Int, Double, Boolean
* - null
* - Map<String, Any?>
* - List<Any?>
* - [CompiledVariable]
* [CompiledValue] can be any of [ApolloJsonElement] or [CompiledVariable]
*
* Note: for now, enums are mapped to Strings
* Enum values are mapped to strings
* Int and Float values are mapped to [com.apollographql.apollo3.api.json.JsonNumber]
*/
typealias CompiledValue = Any?

class CompiledArgument private constructor(
val name: String,
val value: Any?,
/**
* The compile-time value of that argument.
*
* Can be the defaultValue if no argument is defined in the operation.
* Can contain variables.
* Can be [Optional.Absent] if:
* - no value is passed and no default value is present
* - or if a variable value is passed but no variable with that name is present
*/
val value: Optional<CompiledValue>,
val isKey: Boolean = false,
@ApolloExperimental
val isPagination: Boolean = false,
) {
class Builder(
private val name: String,
private val value: Any?,
) {
private var value: Optional<CompiledValue> = Optional.absent()
private var isKey: Boolean = false
private var isPagination: Boolean = false

fun isKey(isKey: Boolean) = apply {
this.isKey = isKey
}

fun value(value: CompiledValue) = apply {
this.value = Optional.present(value)
}

@ApolloExperimental
fun isPagination(isPagination: Boolean) = apply {
this.isPagination = isPagination
}


fun build(): CompiledArgument = CompiledArgument(
name = name,
value = value,
Expand All @@ -349,27 +391,53 @@ class CompiledArgument private constructor(

/**
* Resolve all variables that may be contained inside `value`
*
* If a variable is absent, the key is removed from the containing Map or List.
*
* @param value an [ApolloJsonElement] or [CompiledVariable] instance
*
* @return [ApolloJsonElement] or [Optional.Absent] if a variable is absent
*/
@Suppress("UNCHECKED_CAST")
fun resolveVariables(value: Any?, variables: Executable.Variables): Any? {
private fun resolveVariables(value: Any?, variables: Executable.Variables): Any? {
return when (value) {
null -> null
is CompiledVariable -> {
variables.valueMap[value.name]
if (variables.valueMap.containsKey(value.name)) {
variables.valueMap[value.name]
} else {
Optional.Absent
}
}

is Map<*, *> -> {
value as Map<String, Any?>
value.mapValues {
resolveVariables(it.value, variables)
}.toList()
}.filter { it.value !is Optional.Absent }
.toList()
.sortedBy { it.first }
.toMap()
}

is List<*> -> {
value.map {
resolveVariables(it, variables)
}.filter {
/**
* Not sure if this is correct
*
* ```
* {
* # what if c is not present?
* a(b: [$c])
* }
* ```
*/
it !is Optional.Absent
}
}

else -> value
}
}
Expand Down Expand Up @@ -410,6 +478,7 @@ fun CompiledNamedType.isComposite(): Boolean {
is InterfaceType,
is ObjectType,
-> true

else
-> false
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.apollographql.apollo3.api

import com.apollographql.apollo3.api.Executable.Variables
import com.apollographql.apollo3.api.json.ApolloJsonElement
import com.apollographql.apollo3.api.json.JsonWriter
import okio.IOException

Expand All @@ -21,7 +22,7 @@ interface Executable<D: Executable.Data> {
* Serializes the variables of this operation to a json
*/
@Throws(IOException::class)
fun serializeVariables(writer: JsonWriter, customScalarAdapters: CustomScalarAdapters, withBooleanDefaultValues: Boolean)
fun serializeVariables(writer: JsonWriter, customScalarAdapters: CustomScalarAdapters, withDefaultValues: Boolean)

/**
* A list of [CompiledSelection]. Used when reading from the cache and/or normalizing a model.
Expand All @@ -41,5 +42,10 @@ interface Executable<D: Executable.Data> {
* serialized to their json representation (String/Map most of the time).
* Input objects are serialized to Map<String, Any?>
*/
class Variables(val valueMap: Map<String, Any?>)
class Variables(val valueMap: VariablesJson)
}

/**
* The variables as a Json representation. If some
*/
typealias VariablesJson = Map<String, ApolloJsonElement>
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ fun <D : Executable.Data> Executable<D>.falseVariables(customScalarAdapters: Cus
@ApolloInternal
fun <D : Executable.Data> Executable<D>.variables(
customScalarAdapters: CustomScalarAdapters,
withBooleanDefaultValues: Boolean,
withDefaultValues: Boolean,
): Executable.Variables {
val valueMap = MapJsonWriter().apply {
beginObject()
serializeVariables(this, customScalarAdapters, withBooleanDefaultValues)
serializeVariables(this, customScalarAdapters, withDefaultValues)
endObject()
}.root() as Map<String, Any?>
}.root() as VariablesJson
return Executable.Variables(valueMap)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import okio.IOException
*/
interface Fragment<D : Fragment.Data> : Executable<D> {
@Throws(IOException::class)
override fun serializeVariables(writer: JsonWriter, customScalarAdapters: CustomScalarAdapters, withBooleanDefaultValues: Boolean)
override fun serializeVariables(writer: JsonWriter, customScalarAdapters: CustomScalarAdapters, withDefaultValues: Boolean)

override fun adapter(): CompositeAdapter<D>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ interface Operation<D : Operation.Data> : Executable<D> {
override fun adapter(): CompositeAdapter<D>

@Throws(IOException::class)
override fun serializeVariables(writer: JsonWriter, customScalarAdapters: CustomScalarAdapters, withBooleanDefaultValues: Boolean)
override fun serializeVariables(writer: JsonWriter, customScalarAdapters: CustomScalarAdapters, withDefaultValues: Boolean)

override fun rootField(): CompiledField

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.apollographql.apollo3.api

import com.apollographql.apollo3.annotations.ApolloExperimental
import com.apollographql.apollo3.api.Optional.Absent
import com.apollographql.apollo3.api.Optional.Present
import com.apollographql.apollo3.exception.MissingValueException
Expand Down Expand Up @@ -46,3 +47,11 @@ sealed class Optional<out V> {
* Returns the value if this [Optional] is [Present] or null else.
*/
fun <V> Optional<V>.getOrElse(fallback: V): V = (this as? Present)?.value ?: fallback

@ApolloExperimental
fun <V, R> Optional<V>.map(mapper: (V) -> R): Optional<R> {
return when(this) {
is Optional.Absent -> Optional.Absent
is Optional.Present -> Optional.present(mapper(value))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,7 @@ import com.apollographql.apollo3.exception.JsonDataException
import kotlin.jvm.JvmOverloads

/**
* A [JsonReader] that can consumes Kotlin values as Json
*
* values should be any of:
* - String
* - Int
* - Double
* - Long
* - JsonNumber
* - null
* - Map<String, Any?> where values are any of these values recursively
* - List<Any?> where values are any of these values recursively
*
* Anything else is undefined
* A [JsonReader] that can consumes [ApolloJsonElement] values as Json
*
* To read from a [okio.BufferedSource], see also [BufferedSourceJsonReader]
*
Expand Down Expand Up @@ -423,3 +411,22 @@ constructor(
}
}
}

/**
* A typealias for a type-unsafe Kotlin representation of JSON. This typealias is
* mainly for internal documentation purposes and low-level manipulations and should
* generally be avoided in application code.
*
* [ApolloJsonElement] can be any of:
* - String
* - Int
* - Double
* - Long
* - JsonNumber
* - null
* - Map<String, ApolloJsonElement> where values are any of these values recursively
* - List<ApolloJsonElement> where values are any of these values recursively
*
* Anything else is undefined
*/
typealias ApolloJsonElement = Any?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting that this doesn't show in apiDump. I guess it's source-breaking only.

In all cases, I wish I've done that earlier to be able to point in KDoc to what Any? can contain. Naming is very open to suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApolloJsonElement sounds good to me 👍. I remember doing a similar thing (typealias JsonMap = Map<String, Any?>) in DeferredJsonMerger which made the code less messy.

Loading
Loading