Skip to content

Added alias or field name as variable to Cypher.kt #172

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

Merged
merged 7 commits into from
Mar 25, 2021

Conversation

DirkMahler
Copy link
Contributor

@DirkMahler DirkMahler commented Feb 25, 2021

The alias or field name has been added as variable to the Cypher class. This comes for two purposes:

In a controller the following code can be used:

    private Map<String, Object> getData(List<Cypher> translated) {
        Map<String, Object> data = new HashMap<>();
        for (Cypher cypher : translated) {
            String variable = cypher.getVariable();
            GraphQLType type = cypher.getType();
            Result result = session.run(cypher.getQuery(), cypher.getParams());
            if (result.hasNext()) {
                Object queryResult = isListType(type) ? toIterableResult(result, variable) : toSingleResult(result, variable);
                data.put(variable, queryResult);
            } else {
                if ((type instanceof GraphQLNonNull)) {
                    if (isListType(type)) {
                        data.put(variable, emptyList());
                    } else {
                        throw new IllegalStateException("No result available for non-null alias/field " + variable + ".");
                    }
                } else {
                    data.put(variable, null);
                }
            }
        }
        return data;
    }

    private Iterable<?> toIterableResult(Result result, String key) {
        return () -> {
            Iterator<Record> iterator = result.stream().iterator();
            return new Iterator<>() {
                @Override
                public boolean hasNext() {
                    return iterator.hasNext();
                }

                @Override
                public Object next() {
                    return iterator.next().asMap().get(key);
                }
            };
        };
    }

    private Object toSingleResult(Result result, String key) {
        return result.single().asMap().get(key);
    }

    private boolean isListType(GraphQLType type) {
        if (type instanceof GraphQLList) {
            return true;
        } else if (type instanceof GraphQLNonNull) {
            return isListType(((GraphQLNonNull) type).getWrappedType());
        } else {
            return false;
        }
    }

PS: I did not update the examples as I felt a bit unsafe about Kotlin (still stuck in the "pure" Java universe). Any guidance appreciated for providing this as well.

@Andy2003 Andy2003 self-requested a review March 3, 2021 16:10
Copy link
Collaborator

@Andy2003 Andy2003 left a comment

Choose a reason for hiding this comment

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

Apply changes from DirkMahler#1

@Andy2003 Andy2003 self-requested a review March 3, 2021 17:29
@jexp
Copy link
Contributor

jexp commented Mar 25, 2021

What I don't understand is the unrelated change of the "Cypher" type as result/parameter to String
that makes it inherently less communicating and less safe.

In places where we know we expect Cypher, we should also make use of that.

What does that "variable" actually mean?
I didn't understand it from the code changes.

Is that one of the column names that the contained cypher statement returns? Do we always know these?

@DirkMahler
Copy link
Contributor Author

The generated Cypher query per row returns a map with exactly one column. This one is the query field or alias that was part of the GraphQL query. To fill the data field of the GraphQL response the name of this field or alias needs to be extracted, until now this was only possible by looking at the first row of the result. Even worse: if the result was empty it was not possible to infer that name, e.g. for returning an empty list if the schema requires a Non-Null value. This now has been made available via variable, maybe the name can be subject to discussion.

@Andy2003
Copy link
Collaborator

@jexp

What I don't understand is the unrelated change of the "Cypher" type as result/parameter to String
that makes it inherently less communicating and less safe.

The use of the Cypher type in the generation classes was replaced by the Cypher-DSL. AS such, we no longer need this type internally. It is now only the API for this library.

Because the variable field is mandatory in the Cypher class, we would have needed to initialize this when using it internally.
At the end I just removed one wrapping of the directives' statement-argument, and passed it directly to the generator without wrapping it in with the cypher class

@jexp jexp merged commit 0f89393 into neo4j-graphql:master Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants