GraphQL error: Non-nullable field was not present in result from Dgraph

Issues GraphQL
1

I am receiving this error because of a edge that did not pass auth rules.

For this use case, I want to make sure that Notes get added with authors. But I want to restrict Authors and Notes based upon a different rule set. So a user may be able to see the Note but not the author of the note. In my UI I can add in the missing author as Restricted Author. However the query fails because the Note is not returned with any authors. Basically I need a way to make the edge required for input but optional for query.

type Note @auth(
  query: { rule: "query { queryNote(filter: {isPublic: true}){ id } }" }
  # ...
) {
  id:ID!
  note: String
  isPublic: boolean
  by: Author!
  forLocation: Location
}
type Author @auth(
  query: { rule: "query { queryAuthor(filter: {isPublic: true}){ id } }" }
  # ...
) {
  id: ID!
  name: String!
  isPublic: boolean
  authoredNotes: [Note] @hasInverse(field: by)
}
type Location {
  id: ID!
  name: String!
  city: String
  state: String
  hasNotes: [Note] @hasInverse(field: forLocation)
}

This requires an author to be present when adding a note, but does not allow me to see notes where I do not have access to view the Author on the Note.by edge.

Not sure how to best handle this.

This produces a strange result when viewing the Note from a parent edge. If I query Locations and hasNotes and there are some notes that I do not have access to view the Note.by, that entire Note node will be an empty object instead of giving me the data that I do have access to see such as Note.note for example. This may be graphql normal way of handling missing required children though.

I know that when I made my own API I could make some fields required on input but not required on query. That is what I am needing here.

2

Hi @amaster507
We will need to think if this can be supported. Marking it as an enhancement.

Thanks

1 Like
3

For the time being, I had to just remove the required(!) flag from the field. Hopefully this can be supported before I accidentally have a lot of orphans.

4

I actually think this is best handled by a custom mutation that has the input restrictions.

For example, you could set the actual model to not require the author. That’s really what the types are about - what you expect a query result to have - which in this case is that the author might not be present in the result.

Then set a custom mutation where the input does require the author. That custom mutation just calls the add mutation as it’s implementation (maybe after some other data validation).

ATM, you’d need to deploy that custom mutation somewhere - another server, lambda function etc. But soon we’ll have the JS hooks and you should be able to define it as part of the schema easily.

2 Likes
5

I tend to disagree with this. I believe that there should be a distinguishing factor between a type that generates required inputs and a type that generates required fields.

If this is only supported by a custom mutation then all generated mutations for types that reference this type may allow invalid input.

type Post {
  id: ID
  title: String
  author: Person!
}

This would correctly prevent from adding a new post if I did not supply the author, but it would also throw errors if I am authorized to query the post, but am restricted from querying the Person.

Since a developer cannot control the generated inputs outside of @search, I propose a change that all required fields are only used for generated input types and the generated schema types do not have any required fields.

This would not be a breaking change in that any where a field in the input schema is marked as required should still be there in the responses unless there are unique cases as above, which the developer can code around without having to handle erroneous errors.

I propose the generated input for the schema above should make the following changes:

type Post {
  id: ID
  title: String
  author(filter: PersonFilter): Person # notice not required
}
input AddPostInput {
  title: String
  author: PersonRef! # notice still required
}

I am thinking this could be done somewhat simply when generating the schema for any type or interface object remove all exclamation marks ! after the inputs have been generated.

@pawan what do you think?

6

I agree with this.

But, disagree with this as one would lose the ability to have non-null fields at all in a query response.
and also because below quote is more true:

So, I think, we should propose this:

Let the ! on fields represent that a field in the query response is non-null. Instead, provide the capability to the user to toggle the fields inside mutation inputs as non-null. This can also allow us to explicitly specify nullability for each kind of mutation input (add, update-set, update-remove). In addition, we can also allow users to be able to choose which fields they want in the addInput vs which ones they want in the updateInput, etc. This can be achieved with a new directive like this:

directive @input(
  add: InputParams,
  update: UpdateInputParams
) on FIELD_DEFINITION

input InputParams {
  # defaults to false
  skip: Boolean,
  # defaults to
  # for add inputs: whether `!` has been defined on the field in schema
  # for patch inputs: false
  nonNull: Boolean
}

input UpdateInputParams {
  set: InputParams
  remove: InputParams
}

So, now I would be able to define a schema like this:

type Note @auth(
  query: { rule: "query { queryNote(filter: {isPublic: true}){ id } }" }
  # ...
) {
  id:ID!
  text: String!
  isPublic: boolean
  by: Author @input(add: {nonNull: true}, update: {set: {skip: true}, remove: {skip: true}})
  version: String @input(update: {set: {nonNull: true}, remove: {skip: true}})
}

which would generate following inputs:

input AddNoteInput {
  text: String!
  isPublic: Boolean
  by: Author!
  version: String
}
input SetNotePatch {
  text: String
  isPublic: Boolean
  version: String!
}
input RemoveNotePatch {
  text: String
  isPublic: Boolean
}

This allows me to say things like:

  1. A Note’s text can’t be null in the query response.
  2. A Note’s author can be null in the query response.
  3. A Note’s author must be provided while adding the Note, but after that, you can’t update or remove the author from the note.
  4. A Note’s version may not be provided while adding the note, but after that, every time you update a note, you must update its version. In addition, you can’t remove a note’s version.

Later, we may extend @input to also support default values as well like this:

type Note {
  ...
  version: String @input(add:{default: "v0"}, update: {set: {nonNull: true}, remove: {skip: true}})
}

which allows me to say that if I don’t provide a version while adding a Note, it should default to “v0”.

2 Likes
7

@abhimanyusinghgaur indeed, this satisfies my use-case as explained here Mark fields / types / interfaces and skip GrapQL CRUD generation - #7 by iyinoluwaayoola

What’s the way forward… will this be implemented soon?

1 Like
8

Pinging here… This feature is a requirement for my use case. An ETA will be appreciated to plan accordingly.

1 Like
9

@diggy @aman-bansal Any update on this? This is beyond an enhancement for me, I require it for production.

@amaster507 did you find a workaround?

1 Like
10

Nope. Still present bug/feature depending on who is talking about it.

I just took the ! off from almost every edge. Especially any edge that might return null because of child auth rules.

I think there is another topic if not this one discussing the possibility of changing the schema requiring input but not throwing an error of there is not an output. This is another thing that should be a quick fix and bring a lot of value, but no one seems focused on developing the GraphQL API endpoint right now from what I have been seeing.

11

Hmm… :grimacing: I started full-on with Dgraph’s GraphQL but have slowed down tremendously due to numerous feature requests/ bug fixes required for true production readiness.

Thanks for the suggestion, for this, I need a way to control the fields for the auto-generated input types… i.e., not all type-fields are mutable on an update, and therefore should be removed from the update input.

12

That was another feature request somewhere. You probably have already seen that though.