@GraphQLVariables isn't a class level annotation

[jeremygiberson]

Unlike GraphQLProperty, GraphQLVariable(s) isn't class level -- I'm not sure how I'm suppose to use them to achieve my query/mutation.

I'm trying to build the following mutation

mutation Create2FA($inputDescriptor: Input2FA!) {
   Create2FA(inputDescriptor: $inputDescriptor) {
       TwoFAID
       Description
       ...
   }
}

Or A more general usage example for Queries:

query FindProducts($filter: InputFilter!) {
     FindProductsWithFilter(filter: $filter) {
        SKU
        Description
     }
}

Expected Uses

I was anticipating a model definition like (but illegal to put variables @ class level):

@GraphQLProperty(name = "Create2FA")
@GraphQLVariables({
    @GraphQLVariable(name = "TwoFA", scalar = "CreateTwoFA!")
})
public class MutationCreate2FA extends TwoFAModel {
}

Available Uses

These are my (legal) options but all provide the wrong result.

Test Case

package privoro.aid.authenticator.graphql;

import io.aexp.nodes.graphql.GraphQLRequestEntity;
import io.aexp.nodes.graphql.Variable;
import org.junit.Test;
import org.privoro.aid.authenticator.graphql.InputCreate2FA;
import org.privoro.aid.authenticator.graphql.MutationCreate2FA;

import java.net.MalformedURLException;

import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.Assert.assertThat;

public class TestGraphQLRequestEntity {
    private String EXAMPLE_URL = "https://graphql.example.com";
    @Test
    public void mutationWithVariables() throws MalformedURLException {
        InputCreate2FA input = new InputCreate2FA();
        input.setUserID("user-id");
        input.setDeviceID("device-id");
        input.setRelyingPartyID("relying-party-id");
        input.setDescription("description");

        GraphQLRequestEntity requestEntity = GraphQLRequestEntity.Builder()
            .url(EXAMPLE_URL)
            .variables(new Variable<InputCreate2FA>("TwoFA", input))
            .request(MutationCreate2FA.class)
            .build();
        System.out.println("Request Entity:" + requestEntity.toString());
        assertThat(requestEntity.toString(), containsString("'query ($TwoFA: CreateTwoFA!) { Create2FA (TwoFA: $TwoFA) {"));
    }
}

Input variable model

package org.privoro.aid.authenticator.graphql;

public class InputCreate2FA {
    public String UserID;
    public String RelyingPartyID;
    public String Description;
    public String DeviceID;
}

Result Model

package org.privoro.aid.authenticator.graphql;

public class TwoFA {
    public String UserID;
    public String RelyingPartyID;
    public String Description;
    public String Status;
    public String Reason;
    public String CreatedTimestamp;
    public String ExpiresAt;
    public String TwoFAID;
}

1 Property and Values set inside the class on the result model property

MutationCreate2FA

public class MutationCreate2FA {
@GraphQLProperty(name = "Create2FA")
@GraphQLVariables({
    @GraphQLVariable(name = "TwoFA", scalar = "CreateTwoFA!")
})
    public TwoFAModel twoFa;
}

Output:

query ($TwoFA:CreateTwoFA!){ twoFA : Create2FA(TwoFA:$TwoFA) { Status RelyingPartyID Description CreatedTimestamp UserID TwoFAID ExpiresAt Reason } } 

2 Property on the class but variables on a result model property

MutationCreate2FA

@GraphQLProperty(name = "Create2FA")
public class MutationCreate2FA {
@GraphQLVariables({
    @GraphQLVariable(name = "TwoFA", scalar = "CreateTwoFA!")
})
    public TwoFAModel twoFa;
}

Output:

query ($TwoFA:CreateTwoFA!){ Create2FA { twoFA (TwoFA:$TwoFA) { Status RelyingPartyID Description CreatedTimestamp UserID TwoFAID ExpiresAt Reason } } } 

3 Variables only specified when building request

MutationCreate2FA

@GraphQLProperty(name = "Create2FA", arguments = {
    @GraphQLArgument(name = "TwoFA", value = "$TwoFA")
})
public class MutationCreate2FA extends TwoFA {
}

Output (close, but the builder doesn't inject the variables):

query { Create2FA (TwoFA:$TwoFA) { Status RelyingPartyID Description CreatedTimestamp UserID TwoFAID ExpiresAt Reason } } 

Preferred usage

If I step back from the docs a little and imagine how I might specify query's and mutations with variables I come up with:

@GraphQLOperation(name = "Create2FAMutation", type="mutation", variables = {
   @GraphQLVariable(name = "inputCreate2FA", scalar = "Create2FA!")
}, properties = {
   @GraphQLProperty(name = "Create2FA", arguments = {
      @GraphQLArgument(name = "TwoFA", variable = "inputCreate2FA")
   })
})
class MutationCreate2FA extends TwoFA {

}

Output

mutation Create2FAMutation ($inputCreate2FA: Create2FA!) { Create2FA (TwoFA:$inputCreate2FA) { Status RelyingPartyID Description CreatedTimestamp UserID TwoFAID ExpiresAt Reason } } 

I think this preferred usage can be provided and be backwards compatible w/ current usage.

I've introduced an annotation @GraphQLOperation which is optional (if you don't need variables) in your query/mutation. This would remove the need for @GraphQLVariables (altogether) and @GraphQLVariable would only be used in top level operation annotations (not on models or on model properties).

I've also added an optional variable parameter to the @GraphQLArgument which associates the argument to a specified variable. The addition allows properties to be defined as arguments that can be bound to variables later which reduces unwanted coupling from the model to the operation.

[jeremygiberson]

FYI I'm working on a PR for this suggestion

[chemdrew]

I like the idea of a operation class to distinguish that parameter separately from the fields, it could clean the code up a lot. But I think a good first step might be adding support for variables in the @GraphQLProperty annotation as well as making @GraphQLArgument, @GraphQLArguments, @GraphQLVariable, and @GraphQLVariableswork above classes as well and not just fields. It will also then be very simple to mimic most of that behavior in a new @GraphQLOperation annotation down the road. This should actually be relatively simple and small change too since Argument and Variable classes are essentially the same and arguments are already supported in the property annotation, just different names to be explicit and clear.
Let me know your thoughts on this! Might be worth a separate PR for those changes first and then a revisit on #17 afterwards. I'll also try to keep discussion on the topic here and code specifics in the PR.

[jeremygiberson]

Hey Chemdrew,

My thoughts,
Concerning @GraphQLArgument, @GraphQLArguments I agree that these should available at class level. There is an edge scenario this will affect -- right now a class w/out property annotations is bypassed and it's properties become the top level property. For example TestModelSimple in the tests, when passed by itself through the request builder becomes { simpleString } as opposed to TestModelSimple { simpleString }. So the question arises, what to do with a model w/ arguments but no property annotation. Can/should we require Arguments to be specified within @GraphQLProperty? Or just treat the model as a top level property inferring name from class?

Regarding @GraphQLVariable and @GraphQLVariables and adding variables to @GraphQLProperty -- I think the best direction for these annotations is to deprecate @GraphQLVariables altogether and restrict @GraphQLVariable usage to only in @GraphQLOperation. A few justifications for this:

1. Variables are defined at the operation level and proliferate down into the query in place of argument variables. To mirror writing graphQL statements I think its good align annotations with a similar usage.
2. Variables are non-optional, as once defined must be utilized or GraphQL chokes when a variable is defined and not used.
3. Models represent the Property and available arguments (optional or required) as defined by the schema. These are fixed. Where as variables are consumer defined and arbitrary.
4. Multiple models (representing the same property w/ different views or different properties all together) may have similar argument needs by slightly varying names (UserID, userId, UserId, etc). When included as part of a larger query (because of 2) if models define variables in a similar way you end up with a GraphQL operation like query myQuery($userId: String, $UserID: String, $UserId: String) { ....

Example.
Variables defined by models

@GraphQLVariable(name = "UserID")
class EmailPreferences {
}

@GraphQLVariable(name = "userId")
class TextPreferences {
}

class PreferencesOperation {
   public EmailPreferences emailPreferences;
   public TextPreferences textPreferences;
}

// despite arguments having different names (case counts) we are able to share the (singular) userId variable with both of them
builder.setVariables(Variable(name="UserID", userIdValue));
builder.setVariables(Variable(name="userId", userIdValue));

Output

query PreferencesOperation($UserID: String, $userId: String) { EmailPreferences(UserID: $UserID) { ... } TextPreferences(userId: $userId) { ... } }

Vs Arguments defined by models

@GraphQLArgument(name = "UserID")
class EmailPreferences {
}

@GraphQLArgument(name = "userId")
class TextPreferences {
}

@GraphQLOperation(variables={ @GraphQLVariable(name = 'userId', scalar = 'String') })
class PreferencesOperation {
   public EmailPreferences emailPreferences;
   public TextPreferences textPreferences;
}

// despite arguments having different names (case counts) we are able to share the (singular) userId variable with both of them
builder.setArguments('emailPreferences', Argument(name="UserID", variable='userId'));
builder.setArguments('textPreferences', Argument(name="userId", variable='userId'));

Output

query PreferencesOperation($UserID: String) { EmailPreferences(UserID: $UserID) { ... } TextPreferences(userId: $UserID) { ... } }

So I think variables outside of the Operation annotation just facilitate some icky consequences. What do you think?

next steps

Making argument(s) class level is a small change that shouldn't have any impact on the operation code. So I can bring that into the PR easily.

However,

It will also then be very simple to mimic most of that behavior in a new @GraphQLOperation annotation down the road.

Do I understand this to be that you don't like how operation is implemented within this PR and are looking to do it differently pending changes to class level updates to the specified annotations?

[chemdrew]

So far my approach has been to treat the model as a top level property inferring name from class, and just using the property annotation to apply overrides there - which is what had me excited by the idea of having the operation class separate because then a lot of logic around which fields are present and conditionals on how to handle them can be simplified.

So for a first simple implementation I would picture it looking like

@GraphQLArguments({
  @GraphQLArgument(name="first"),
  @GraphQLArgument(name="second")
})

with the request being

query {
  TestModelSimple(first: null, second: null) {
    simpleString
  }
}

In regards to variables, I agree there are some big consequences to using variables that require a significant understanding of the GraphQL specification but I believe allowing them on all fields provides a huge benefit that outweighs this. A specific example of this is for Date fields. For every date field we use a custom resolver that wraps moment.js to allow the consumers to specify timezone/format/locale of that date. Many of our models have multiple date fields in each and this is the perfect use-case for variables since different consumers may want a different timezone/format/locale but they will also want that consistently maintained throughout the response.

I like the way you have operations implemented currently, but I think adding the option of a variable field in @GraphQLProperty, and making @GraphQLArgument(s) and @GraphQLVariable(s) work above classes would all be beneficial in separate PRs just to break things up into smaller features to make things easier

[jeremygiberson]

Roger on arguments translation to TestSimpleModel.

A specific example of this is for Date fields. For every date field we use a custom resolver that wraps moment.js to allow the consumers to specify timezone/format/locale of that date. Many of our models have multiple date fields in each and this is the perfect use-case for variables since different consumers may want a different timezone/format/locale but they will also want that consistently maintained throughout the response.

If I understand your use case correctly, I believe operation-vars/property-arguments cover this scenario. .

The use case requirements as I understand:
1 consumer can specify the timezone format local
2 consistency maintained through response
3 different consumers may want a different timezone/format/locale

This is a sample query I think you're describing that is resolved on the server side:

query GetUser(userId: ID!) {
    SignUpDate(format: String) : String
    EnrollmentDate(format: String) : String
    CancellationDate(format: String) : String
}

Right off the bat this is solved because it is the consumer that is writing the model -- so they have all control over the definitions.

As represented by models using arguments:

class User {
    @GraphQLArgument(name="format", value="yyyy-mm-dd")
    public String SignUpDate;
    @GraphQLArgument(name="format", value="yyyy-mm-dd")
    public String EnrollmentDate;
    @GraphQLArgument(name="format", value="yyyy-mm-dd")
    public String CancellationDate;
}

@GraphQLOperation(name="getSubscribers")
class GetHistoricalSubscribers {
    public User[] Subscribers;
}

Above meets requirements 1 and 2 because the consumer defines the model and can specify their default values to match their needs. Requirement 3 is met by the very nature that each consumer is defining their own models: thus 1 & 2 again.

We can also accommodate exceptions to the desired default format, given the same model definition:

@GraphQLOperation(name="getSubscribers", variables={@GraphQLVariable(name = "format", scalar="String")})
class GetHistoricalSubscribers {
    public User[] Subscribers;
}

// when building the request we can then bind a dateFormat variable to the arguments (though variable isn't needed, it could just be set arguments)
builder.setVariables(new Variable("dateFormat", "mm/dd/yy"));
builder.setArguments("GetHistoricalSubscribers.Subscribers.SignUpDate", new Argument("format", null, "dateFormat"));
builder.setArguments("GetHistoricalSubscribers.Subscribers.EnrollmentDate", new Argument("format", null, "dateFormat"));

Again this would be for exceptional occasions -- because why wouldn't the consumer define the model w/ the desired format in the first place?

[chemdrew]

So is your goal for declaring variables at an operation level over class level to keep from having variations like userId/UserID/UserId all in the same query like shown here myQuery($userId: String, $UserID: String, $UserId: String)?
My primary reasons for defining the variables at any level are to keep closely synced with the GraphQL specification. Defining variables at the top level also means shifting the weight from setting them in the annotation to defining them through the dot notation path, which slightly concerns me. Since the dot notation seems a lot more likely to lead to a bad path or a typo. I'm pretty torn on this part actually, the weight is pretty heavy on both pros/cons here

[jeremygiberson]

So is your goal for declaring variables at an operation level over class level to keep from having variations like userId/UserID/UserId all in the same query like shown here myQuery($userId: String, $UserID: String, $UserId: String)?

It's one concern. Another concern is that arguments can have default values while variables do not. A model with a bunch of properties defined w/ sane defaults is easier to use than a model with a bunch of variables.

With the following model definition:

class User {
    @GraphQLArgument(name="format", value="yyyy-mm-dd")
    public String SignUpDate;
    @GraphQLVariable(name="format", scalar="String")
    public String EnrollmentDate;
}

Assuming I use request builder to make a query from the model and don't set any arguments or variables.

GraphQLRequestEntity requestEntity = GraphQLRequestEntity.Builder()
            .headers(headers)
            .url(this.endpointUrl);
            .request(User.class)
            .build()

What format will SignUpDate be? Look at the model: defaults to "yyyy-mm-dd". What format will EnrollmentDate be? Dunno, what ever the default value is in the spec. Go look at the spec. Because the model was defined with a variable more work is required in the request builder to specify good defaults.

My primary reasons for defining the variables at any level are to keep closely synced with the GraphQL specification.

To me -- that's what arguments do and more accurately. The model defines what arguments are available, as well as sane defaults. GraphQL schemas define arguments on fields (not variables). Consumers define variables with operations to be used as argument values.

There's also a weird constraint imposed on variables if you use them at the property level. For graphQL, variables are arbitrarily named placeholders that the consumer gets to define when they craft their operation. Ie query User($myUserIdVariable:ID!) { User(UserID: $myUserIdVariable){. As is, because request builder translates property variable definitions to argument names -- defining a variable at the property level, the consumer is constrained to using the name that matches the argument for the property. This is a very subtle constraint but its just another thing that can trip a consumer up when writing their models.

Defining variables at the top level also means shifting the weight from setting them in the annotation to defining them through the dot notation path, which slightly concerns me. Since the dot notation seems a lot more likely to lead to a bad path or a typo.

Yeah, this is true and using dot path to set arguments and bind variables does feel clunky.

I'm pretty torn on this part actually, the weight is pretty heavy on both pros/cons here

So, I think I've got a compromise. This is already supported in the PR unintentionally -- but:

1. Drop @GraphQLVariable(s) from property annotations
2. @GraphQLArgument has the attribute variable of type @GraphQLVariable
3. @GraphQLOperation has optional variables attribute and picks up variables defined in @GraphQLArgument

class MyUserQuery {
   public User user;
}
class User {
    // if you want to define variables @ property level
    @GraphQLArgument(name="format", variable=@GraphQLVariable(name = "dateFormat", Scalar = "String"))
    public String SignUpDate;
    // if you want to define them at the operation
    @GraphQLArgument(name="format", value="yyyy-mm-dd")
    public String EnrollmentDate;
}

No variable set at request time

Because we've defined the variable with the argument we can decide if we should render the statement with variable substitution based on if the builder has variables defined.

builder.request(MyUserQuery.class).build();

Output

query MyUserQuery { User { SignUpDate(format: "yyyy-mm-dd") EnrollmentDate(format: "yyyy-mm-dd") } } 

Variable set at request time

When variables have been set in the builder, we render the substitution in the query.

builder.request(MyUserQuery.class)
     .setVariables(new Variable("dateFormat", "mm/dd/yy")
     .build();

Output

query MyUserQuery($dateFormat: String) { User { SignUpDate(format: $dateFormat) EnrollmentDate(format: "yyyy-mm-dd") } } variables { "$dateFormat": "mm/dd/yy" }

Variables bound to argument value at request time

Properties with arguments defined not including a variable definition can be bound at request time.

builder.request(MyUserQuery.class)
    .setVariables(new Variable("dateFormat", "mm/dd/yy")
    .setArguments("MyUserQuery", new Argument("EnrollmentDate", null, "dateFormat"))
    .build();

Output

query MyUserQuery($dateFormat: String) { User { SignUpDate(format: $dateFormat) EnrollmentDate(format: $dateFormat) } } variables { "$dateFormat": "mm/dd/yy" }

[chemdrew]

If we remove the ability to set variables at any level then we break away from the GraphQL specification, I see the concerns with it being more advanced and difficult to use than arguments but they are two distinct features not in my control and the discussion may be better moved there.
If the goal is to have default values defined in the models, Arguments may be the best case, but I can also get behind an operation annotation setting defaults on variables since that should be restricted to the top level of the query - where variables are passed in.

I'll be opening a couple PRs tonight:

1. Add field variables in @GraphQLProperty
2. @GraphQLArgument(s)/@GraphQLVariable(s) available at a class level

Hopefully that adds any of the missing features you mentioned and then we can look at the PR you raised and use the Operation annotation for setting default values at the top level