Yeah, that is totally understandable. For the use case we have right now, it's more of a convenience thing. We are basically serializing and deserializing between a directive and a Kotlin data class that is easier to work with programatically to attach metadata to fields and types. This directive is not used by anyone to define the schema, its metadata added during a processing step of the schema. The other option would be to keep that state outside of the schema objects.

Thinking out loud here, ExecutionInput is also immutable and offers ExecutionInput.localContext : Object. After being constructed there is no immutability guarantee made by the library against localContext, i.e users can use a mutable map and live dangerously. But I'd say it's OK since it's really a field reserved for the library users, not the library itself.

I'm trying to see if it would be any different to have such a field on all Graph* elements? (val context: Object)
The library would only guarantee (or not maybe with some options) that context on Graph* elements are being transferred over when using transform()?
@andimarek What kind of usage you wouldn't want to have if we had such a mechanism?

yes you right that we could say: this is not our business, you are on your own when you transform or change or copy a GraphQLSchema. The problem is that this is weaker than the guarantee we give so far, but also transforming/modifying a GraphQLSchema is a real use case vs transforming an ExecutionInput is not really.

And people will misuse this, believe me. We will end up with issues asking why the MyUser object stored as metadata in the Schema is not correctly copied or why suddenly something changed because they stored a mutable object in the schema and references it from multiple times. It is simply not a good design decision for such a widely used library as GraphQL Java imho.

I see two ways forward:

1. We introduce a limited metadata field like Map<String,String> or something similar, which is guaranteed immutable.
2. We make it easier to store Metadata outside of the GraphQLSchema in some form like `Map<GraphQLSchemElement, Object>. I would look for your input what this exactly means, based on your needs.

One hacky way to actually store arbitrary data today is to misuse custom scalars:

        GraphQLScalarType hack = GraphQLScalarType.newScalar().name("HACK").coercing(new Coercing<Object, Object>() {
            @Override
            public @Nullable Object serialize(@NotNull Object dataFetcherResult) throws CoercingSerializeException {
                return null;
            }

            @Override
            public @NotNull Object parseValue(@NotNull Object input) throws CoercingParseValueException {
                return input;
            }

            @Override
            public @Nullable Object parseLiteral(@NotNull Object input) throws CoercingParseLiteralException {
                return null;
            }
        }).build();

        Map<String, Object> metadata = new LinkedHashMap<>();
        GraphQLObjectType query = newObject().name("Query")
                .field(newFieldDefinition()
                        .name("hello")
                        .type(Scalars.GraphQLString)
                        .argument(newArgument().name("notUsed").type(hack).defaultValueProgrammatic(metadata)))
                .build();
        GraphQLSchema schema = newSchema()
                .query(query).build();

Here we define a custom scalar to hold the metadata map and add it with defaultValueProgrammatic to the schema.
Not really recommend :-), but I wanted to show it.

What kind of usage you wouldn't want to have if we had such a mechanism?

I think the concern has been about object equality.

We use it a lot in areas like schema transformers where the visitors and backing zipper code can use object equality direct.

if (thisSchemaElement.equals(thatSchemaElement)) {
  // then do something important based on strong object equality
}

In fact we declare this in the GraphqlSchemaElement interface

 /**
     * No GraphQLSchemaElement implements `equals/hashCode` because we need object identity
     * to treat a GraphQLSchema as an abstract graph.
     *
     * @return a hash code value for this object.
     */

So the concern was a "floating" open context object would break that.

Now as you say if we had

public interface GraphQLSchemaElement {
   GraphqlContext getContext();

then we could document it as "all bets are off - stick whatever you like in here but its not immutability and its not part of the equality contract of the object.

ps if we did this I think we should use graphql.GraphQLContext as a glorified map rather than Object. It's a smidge more type safe - but only a smidge

smidge
noun INFORMAL
noun: smidgen; plural noun: smidgens; noun: smidgin; plural noun: smidgins; noun: smidgeon; plural noun: smidgeons
a small amount of something.

mid 19th century: perhaps from Scots smitch in the same sense.

The above is mostly for my German colleague ;)

One thing I can see is that by putting a context on schema elements (and maybe the schema itself) then you solve some lifecycle problems. The metadata for a schema and its elements is tied to the lifetime of that schema - where as if you externalise it in some other parallel object you need manage that yourself as a library consumer

And people will misuse this, believe me. We will end up with issues asking why the MyUser object stored as metadata in the Schema is not correctly copied or why suddenly something changed because they stored a mutable object in the schema and references it from multiple times. It is simply not a good design decision for such a widely used library as GraphQL Java imho.

Are you saying users of the SchemaTransformer could expect the "context" to be deep copied when running a transform vs being at best referenced?
Maybe we could we have a few options when running a transform?

1. context on Graph* elements is copied by reference
2. [default] context on Graph* elements is cleared (null)
3. as part of the transform, let me pick what I want to do with the context on a visited Graph* element. Similarly to other fields.

If the user application is modifying context elements on a GraphQLSchema that is transformed, it's on them. The same way someone could modify the same value at DataFetchingEnvironment.graphqlContext()["key"] across threads.

For (1) is a "smidge" better since it solves the lifecycle issue and it doesn't hijack the directives, but as @xuorig said, it forces serialization/deserialization. Improvement nonetheless.

For (2) is interesting since it does the opposite, no need to serialize but where would the object actually live?
And if it doesn't live on the GraphQLSchema, it could as well just be implemented in user code?

Now as you say if we had

public interface GraphQLSchemaElement {
  GraphqlContext getContext();

then we could document it as "all bets are off - stick whatever you like in here but its not immutability and its not part of the equality contract of the object.

Yes! Exactly what I had in mind.

(...) where as if you externalise it in some other parallel object you need manage that yourself as a library consumer

Yes we did not create such a construct and most of our code pass around GraphQLSchema instances around, which in retrospect is probably something we should have done.