eclipse / microprofile-graphql Goto Github PK

Add test cases for @deprecated and make sure that it can work the same as @ignore

Meaning that if placed on the field, both Input and Output is deprecated, however putting it on the getter of setter only the input or output is deprecated. (Same as @ignore)

Just pulling down the current repo and trying to run a mvn install is failing due to dependency issues.

Scotts-iMacPro:microprofile-graphql starksm$ mvn install
[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO] 
[INFO] MicroProfile GraphQL                                               [pom]
[INFO] MicroProfile GraphQL :: Specification                              [pom]
[INFO] MicroProfile GraphQL :: API                                        [jar]
[INFO] MicroProfile GraphQL :: TCK                                        [jar]
[INFO] 
[INFO] ----< org.eclipse.microprofile.graphql:microprofile-graphql-parent >----
[INFO] Building MicroProfile GraphQL 1.0.0-SNAPSHOT                       [1/4]
[INFO] --------------------------------[ pom ]---------------------------------
Downloading from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-plugin/0.13/apache-rat-plugin-0.13.pom
Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-plugin/0.13/apache-rat-plugin-0.13.pom (10 kB at 34 kB/s)
Downloading from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-project/0.13/apache-rat-project-0.13.pom
Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-project/0.13/apache-rat-project-0.13.pom (26 kB at 485 kB/s)
Downloading from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-plugin/0.13/apache-rat-plugin-0.13.jar
Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-plugin/0.13/apache-rat-plugin-0.13.jar (53 kB at 1.0 MB/s)
[INFO] 
[INFO] --- apache-rat-plugin:0.13:check (rat-check) @ microprofile-graphql-parent ---
Downloading from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-core/0.13/apache-rat-core-0.13.pom
Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-core/0.13/apache-rat-core-0.13.pom (4.2 kB at 115 kB/s)
Downloading from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-api/0.13/apache-rat-api-0.13.pom
Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-api/0.13/apache-rat-api-0.13.pom (1.9 kB at 49 kB/s)
Downloading from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-core/0.13/apache-rat-core-0.13.jar
Downloading from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-api/0.13/apache-rat-api-0.13.jar
Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-core/0.13/apache-rat-core-0.13.jar (128 kB at 2.6 MB/s)
Downloaded from central: https://repo.maven.apache.org/maven2/org/apache/rat/apache-rat-api/0.13/apache-rat-api-0.13.jar (11 kB at 116 kB/s)
[INFO] Enabled default license matchers.
[INFO] Will parse SCM ignores for exclusions...
[INFO] Parsing exclusions from /Users/starksm/Dev/JBoss/Microprofile/microprofile-graphql/.gitignore
[INFO] Finished adding exclusions from SCM ignore files.
[INFO] 66 implicit excludes (use -debug for more details).
[INFO] 5 explicit excludes (use -debug for more details).
[INFO] 4 resources included (use -debug for more details)
[INFO] Rat check: Summary over all files. Unapproved: 0, unknown: 0, generated: 0, approved: 3 licenses.
[INFO] 
[INFO] --- maven-checkstyle-plugin:3.0.0:check (verify-style) @ microprofile-graphql-parent ---
[INFO] Starting audit...
Audit done.
[INFO] 
[INFO] --- maven-install-plugin:2.4:install (default-install) @ microprofile-graphql-parent ---
[INFO] Installing /Users/starksm/Dev/JBoss/Microprofile/microprofile-graphql/pom.xml to /Users/starksm/.m2/repository/org/eclipse/microprofile/graphql/microprofile-graphql-parent/1.0.0-SNAPSHOT/microprofile-graphql-parent-1.0.0-SNAPSHOT.pom
[INFO] 
[INFO] -----< org.eclipse.microprofile.graphql:microprofile-graphql-spec >-----
[INFO] Building MicroProfile GraphQL :: Specification 1.0.0-SNAPSHOT      [2/4]
[INFO] --------------------------------[ pom ]---------------------------------
[WARNING] The POM for org.asciidoctor:asciidoctor-maven-plugin:jar:1.5.8 is missing, no dependency information available
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for MicroProfile GraphQL 1.0.0-SNAPSHOT:
[INFO] 
[INFO] MicroProfile GraphQL ............................... SUCCESS [  2.240 s]
[INFO] MicroProfile GraphQL :: Specification .............. FAILURE [  0.004 s]
[INFO] MicroProfile GraphQL :: API ........................ SKIPPED
[INFO] MicroProfile GraphQL :: TCK ........................ SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  2.388 s
[INFO] Finished at: 2019-04-30T11:47:50-07:00
[INFO] ------------------------------------------------------------------------
[ERROR] Plugin org.asciidoctor:asciidoctor-maven-plugin:1.5.8 or one of its dependencies could not be resolved: Failure to find org.asciidoctor:asciidoctor-maven-plugin:jar:1.5.8 in https://repo.maven.apache.org/maven2 was cached in the local repository, resolution will not be reattempted until the update interval of central has elapsed or updates are forced -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/PluginResolutionException

Currently respects JsonbProperty - but only if no value specified in @query
Needs to be documented

Is Argument mandatory ? It should not allow no value.
Or we should have a way to name arguments without the annotation

See: https://github.com/eclipse/microprofile-graphql/blob/master/spec/src/main/asciidoc/entities.asciidoc#scalars

This should include spec text indicating how the built-in scalars should be mapped to Java types (i.e. "Int" to "int" and "Integer", "String" to "String", etc.). This should also include TCK tests that verify that scalars are correctly mapped to the appropriate Java types.

More background information here: https://graphql.org/learn/schema/#scalar-types

Custom scalar types should not be addressed with this issue. That should be addressed in a separate issue.

As @phillip-kruger mentioned in issue #80, graphql-java/SPQR defines a Long scalar type. This should be documented in the spec, and tested in the TCK.

Currently the spec is incorrect (saying that Java long/java.lang.Long types are mapped to GraphQL Int scalars) - it says:

Int - which maps to a Java int/Integer or long/Long.

This text should be changed when resolving this issue.

In issue #46 / pull request #52 we added tests where @DefaultValue was on the mutation or query method arguments, but we also need tests and spec docs to indicate that @DefaultValue can be applied to fields/setters on input type entity objects.

From our meeting notes

JSon-B Needs - open issues here: https://github.com/eclipse-ee4j/jsonb-api/issues
Getting Config (readable)
Introspection - issue 189
Read (deserialize) from a tree structure
Better naming strategy - adding context, etc.

After discussion with @kaqqao, it seems that field ordering is no longer implemented by graphql-java v12. Does it make sense to keep these 2 annotations in the spec in these conditions? graphql-java is considered as the GraphQL RI in the Java ecosystem. If it doesn't implement field ordering will it ever exist?

Per conversation at August 1st, 2019 hangout, we determined that we should document that each field on an entity is technically a query. This allows for more complex and useful queries and field selection. This should be documented in the spec.

As discussed in the meeting (https://docs.google.com/document/d/1gb3jirFGrJwDZSbrtnFPVTNjPNe3Y0dUYfm-HkU1c3U/edit#heading=h.4fgdpqwrqxep)

Move description field form annotations (eg. @query @mutation etc) to it's own Annotation.
This way a InputField can also be described without an @query annotation needed

The javadoc for the Type annotation is wrong: there is in fact no "name" parameter in the definition.

Hence, Type(name = "Starship", description = "A starship in StarWars") should be replaced by Type(value="Starship", description = "A starship in StarWars")

Per discussion on May 3, 2019 (minutes), we decided to use the SuperHeroDatabase sample as one of the TCK test apps.

As discussed, we want to support JSON-B Annotations on objects (eg. @JsonbDateFormat("dd/MM/yyyy") and others) that should be used we serializing to json on the response or to object on the request.

We need test cases for all support JSON-B annotations

Add a test with an Union type.

I noticed a redundant use of Mutation annotation for the delete example.

The doc file in question is https://github.com/eclipse/microprofile-graphql/blob/0b4d2600914da69ee071a4f9041da408d4e712ce/spec/src/main/asciidoc/mutations.asciidoc

What should we name the query in cases like @query getX() ? Should it be “getX” or “x”?
Possibly use a MP Config property to determine the behavior - default behavior should be “getX”.

There are a lot of dynamic tests in the TCK and it isn't immediately clear what is being tested. For example, the TCK might expect that a particular format is expected for a date field - but the input and output doesn't explain why the format should be that way. It would be nice if the tests were better documented so that a new user could see a failure in the TCK and be able to quickly understand what is expected (i.e. that @JsonbDateFormat is used for field dateofLastCheckin, etc.).

There are a few ways that could help clarify the TCK for new users (or experienced users that need to refresh):

• create UML-like diagrams (class diagrams, etc.)
• add comments in the test.properties file that more fully explains what is being tested and why the expectations are the way they are
• possibly we could add READMEs to the dynamic tests to make it more clear

[Phillip] what happens when @id annotation is placed on a complex typed field?
SPQR attempts to stringify the object, possibly to JSON.
Might be better to fail - deployment error (even for ints, floats, etc.)

Correct me if I'm wrong but it seems that there is no annotation to provide a description to a field type. @inputfield enables that on Input Type but there is no equivalent on regular Type.

Java enum types need to be converted to GraphQL enum types in the schema - and then fields using those types should be made into custom scalars.

This seems to work out of the box for me, but should be documented and tested appropriately in the TCK.

Create a runner project that makes it easy to use GraphiQL to create test cases.

To test that everything still work when a Object contain that Object, add a new field to Team:

private Team rivalTeam.

As per the high level diagram, we need a application to test against, that can run in Arquillian.

We need a test case that verifies deleting an entity via mutation. The HeroFinder class's removeHero method is currently used for other test cases to clean up after creating a new hero. We could use this for the test case - delete an existing hero - just make sure to bring him/her back for subsequent tests. :)

Add a test on a type that inherit from an interface.

In issue #17, we decided to postpone the option of specifying input/output fields order until MP GraphQL v.Next. So, for the 1.0 release, we need to remove these annotations, and any possible references to it in the TCK, spec, etc.

See: https://github.com/eclipse/microprofile-graphql/blob/master/spec/src/main/asciidoc/entities.asciidoc#fieldsorder--inputfieldsorder

Argument/InputField is the same thing ? Bojan to do POC to remove InputField and feedback next week

Add section in the spec on Queries. Include:

• named
• unnamed
• without fragments
• with fragments
• without variables
• with variables
• directive

Then create test to create this against the current Query options in HeroFinder.

In testing some JSON-B annotations, I noticed that all fields that used primitive (long, int, boolean, float, etc.) the schema made them all non-nullable (using the exclamation point in the schema - meaning that values for these fields must be provided by the client). If the Java field was annotated with @DefaultValue(someValue) then the primitive Java property was listed as nullable (no exclamation point).

For all other (non-primitive) Java fields (String, List, MyCustomWidget, etc.) they were always nullable. Looking at the SPQR code, there is a @GraphQLNonNull annotation that is used on Java fields, setters, etc. that would mark the field as non-nullable. SPQR also honors @javax.annotation.Nonnull and some Bean Validation annotations (@NotNull, @NotEmpty and @NotBlank) and marks the field as non-nullable in the schema.

This issue exists to determine what we want to support in MP GraphQL and also to ensure that the behavior is documented correctly in the spec and tested correctly in the TCK. It may also be used to add an API.

Here are some questions that we should answer first:

1. Should we have our own annotation for marking a field as non-nullable? Something like @org.eclipse.microprofile.graphql.NonNull?
2. Should we also use JSON-B's @JsonbNillable (class level) and @JsonbProperty(isNillable=true|false) (setter/field level) to determine a field's nullable status?
3. Bean Validation is not part of the MicroProfile spec. Should we honor those annotations? Perhaps we could document them as "if the user or implementation uses Bean Validation, then these annotations would have the following effect..."
4. Is the behavior for primitives what we want? (where a primitive is declared as non-nullable unless the Java property has a @DefaultValue annotation.)

Another alternative to 1 would be to add a boolean "isNullable" property to the @InputField annotation and then use that. I personally would prefer to use a separate annotation, but this is still an option.

Add Spec, test setup, tests for pagination

In last week's meeting we discussed what currently happens and what should happen when a user puts an @Query annotation on a void method.

The current behavior seems to be that it would generate a schema with Boolean return type and then return Boolean.TRUE (assuming no errors).

The desired behavior is that this is a deployment error and should prevent the application from starting.

To resolve this issue, we should document the behavior and if possible add a TCK test for it.

Create the first dynamic test, to be an example for other tests to follow

Like @JsonbTransient, the @Ignore annotation should have different meaning whether it is placed on the getter method, setter method or the field itself.

If the @Ignore annotation is placed on the Java field, then that field's GraphQL field should not exist on either the input type or the output type. If the annotation is placed on the getter method, then it should not exist on the output type, but should exist on the input type. If the annotation is placed on the setter method, then it should not exist on the input type, but should exist on the output type.

This issue should only cover the @Ignore annotation. Ideally, this project will also support JSON-B semantics, and then the same rules should apply to the JsonbTransient annotation - but that should be part of a separate issue.

This error when you build the TCK:

Enum:
microprofile-graphql/tck/src/main/java/org/eclipse/microprofile/graphql/tck/annotations/enum/package-info.java:[21,58] as of release 5, 'enum' is a keyword, and may not be used as an identifier
and
microprofile-graphql/tck/src/main/java/org/eclipse/microprofile/graphql/tck/apps/annotations/enum/package-info.java:[21,58] as of release 5, 'enum' is a keyword, and may not be used as an identifier

Interface
microprofile-graphql/tck/src/main/java/org/eclipse/microprofile/graphql/tck/apps/annotations/interface/package-info.java:[21,58] expected
and
microprofile-graphql/tck/src/main/java/org/eclipse/microprofile/graphql/tck/annotations/interface/package-info.java:[21,58] expected

I assume that means we need to name this differently ?

@WhiteCat22 ?

@InputField("someOtherName") should change the field name in the input type in the schema. The test needs to ensure that the schema is correct, but also that the field can be mapped appropriately to the application object.

While discussing this project with some others inside IBM, an interesting use case came up. Suppose there is a larger expense to querying some fields compared to others. Using the super hero application as an example, suppose that querying a currentLocation field is expensive (requires using satellites, Cerebro, etc.) while querying a super hero's name or super-powers is relatively cheap.

If we have a method like this:

@Query
public Collection<SuperHero> allHeroesInTeam(@Argument("team") String teamName) throws UnknownTeamException {
    for (SuperHero hero: heroDB.getHeroesFromTeam(teamName)) {
        hero.setCurrentLocation(cerebro.findHero(hero));
    }
    ...
}

and the user executes a query like:

query allXMen {
    allHeroesInTeam(team: "X-Men") {
        name
        superPowers
    }
}

then we end up spending a lot of time and effort calculating the current location of each super hero when the client doesn't actually care about that information.

There should be a way for the allHeroesInTeam method to obtain the list of fields requested by the client so that the method could be more efficient. Something like (just an idea to help illustrate the request - not necessarily my proposed solution):

@Inject
GraphQLRequest request; // this should be a request scoped proxy

@Query
public Collection<SuperHero> allHeroesInTeam(@Argument("team") String teamName) throws UnknownTeamException {
    Collection<SuperHero> heroes = heroDB.getHeroesFromTeam(teamName);
    if (request.requestedField(SuperHero.class, "currentLocation")) { // only perform this check if the client wants the currentLocation
        for (SuperHero hero: heroes) {
            hero.setCurrentLocation(cerebro.findHero(hero));
        }
    }
    ...
}

See: https://github.com/eclipse/microprofile-graphql/blob/master/spec/src/main/asciidoc/components.asciidoc

As discussed and shown here

https://github.com/jefrajames/mpgraphql-basic-client/pull/1

Move the code to the TCK and create the initial structure

The current spec text mentioned that users can create their own scalar types, however not how.

My suggestion is to create a new Annotation @CustomScalarMarker and an interface CustomScalar that will allow a user to create a Scalar, example:

@CustomScalarMarker
public class LocalDateScalarProvider implements CustomScalar<LocalDate, String> {

    @Override
    public String getName() {
        return "Date";
    }

    @Override
    public String getDescription() {
        return "Date Scalar";
    }

    @Override
    public String serialize(LocalDate localDate) {
        return localDate.format(DateTimeFormatter.ISO_DATE);
    }

    @Override
    public LocalDate deserialize(String fromScalar) {
        return LocalDate.parse(fromScalar);
    }

}

This is just an example on how it could be done. Any other thoughts ?

Explicitly exclude el-api from cdi-api dependency

See eclipse/microprofile#56

Per conversation in gitter and previously in the MP GraphQL meetings, we discussed the issues surrounding the use of generics as a return type or parameter in a query/mutation.

The consensus is that collections (i.e. List<Book>, Set<Widget>, etc.) should work just fine. Naming of types/input types for other generics are awkward, and thus allowed, but not recommended - for example:

@Type("Box")
@InputType("BoxInput")
class Container<T> {...}

@Query
public Container<Book> findBooks() {...}

@Mutation
public Container<Book> saveBooks(Container<Book> input) {...}

The schema might generate something like:

type Box_Book { ... }
input Box_BookInput { ... }

with no way for a user to override it unless they created a concrete class like:

@Type("BookBox")
@InputType("BookBoxInput")
class MyBookBox extends Container<Book> { ... }

This should be the recommended approach for dealing with non-collection generic types.

Further, we should also document that inferred generics are not allowed to be the return type for query/mutation methods - i.e.

@Query public <T> T something() {...}

See: https://github.com/eclipse/microprofile-graphql/blob/master/spec/src/main/asciidoc/entities.asciidoc#scalars

As discussed, add the hight level diagram to the README of the TCK

We need test cases for @DefaultValue. It does not seem to be working at the moment.

The schema still makes the field mandatory so you can not pass an empty value so that the default is being used

From our meeting notes :

BeanValidation (@nonnull, @nonempty , @blank → required tag (“!”) in schema) - just on the Schema (!) - what about group parameter ? (ignore for now ?)

The @Deprecated API was a stretch goal for 1.0, but we ran out of time. We need to remove it from the 1.0 release, and then re-add it to the 1.1 release (see issue #47).

See: https://github.com/eclipse/microprofile-graphql/blob/master/spec/src/main/asciidoc/errorhandling.asciidoc

In PR #69, @phillip-kruger brought up the issue that the date format used in the @JsonbDateFormat annotation should be used as the description for the given field in the generated GraphQL schema (see conversation here).

If there is already a description provided by the user via the (yet to be created, see issue #44) @Description annotation, then that is used for the description of the field (both for input and output types). If a description is not provided, but the Java field, method (setter for input type, getter for output type), class or package contains a @JsonbDateFormat, then the type is a custom Date/Time/DateTime scalar (already part of PR #69) and the description will default to the date format string from the annotation.