Skip to content

Adds scoped context for propagating values specific to a field and its children #2634

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 3 commits into from
Dec 13, 2019
Merged

Adds scoped context for propagating values specific to a field and its children #2634

merged 3 commits into from
Dec 13, 2019

Conversation

lancelafontaine
Copy link
Contributor

Resolves #2602.

This PR adds a "scoped context" to the existing context. This is useful for propagating values which are specific to a field and its children, as opposed to for the entirety of the query.

This is achieved with very little overhead on the interpreter runtime by simply reinitializing the scoped context at various points where new resolvers would run within a possibly new context as the query is being executed.

@@ -155,6 +155,7 @@ def initialize(query:, values: , object:)
@path = []
@value = nil
@context = self # for SharedMethods
@scoped_context = {}
Copy link
Contributor Author

@lancelafontaine lancelafontaine Dec 5, 2019

Choose a reason for hiding this comment

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

My initial implementation relied on Context@scoped_context being an ImmutableHash instead of an Hash. I ended up opting out of that decision since it provides little benefit unless we can sure that all values, no matter how deeply nested, are immutable. That being said, with @scoped_context being a Hash, there's nothing stopping an arbitrary resolver from keeping a reference to an object which is used in a separate scoped context, and modifying it from there. I think that's okay though, since doing so would invalidate the goal of keeping this reference only for a field and its children.

@rmosolgo
Copy link
Owner

rmosolgo commented Dec 6, 2019

👍 Thanks for taking a crack at this! Nice to see how little overhead is required. Thanks for the comprehensive tests, too.

One general question I have about the design is, should we require uses to distinguish between scoped context values (using scoped_get) and unscoped ones (using ctx[key])? Alternatively, we could overload ctx[key] to check scoped and unscoped values. Similarly, should ctx.to_h and ctx.key include key-value pairs which are scoped? (I think they should, what do you think?)

I guess what I'm getting at is, what if .scoped_merge! added values to ctx which were just like other values except that they're only visible to descendant fields. Do you think it would be to magical? Or do you see a specific advantage to distinguishing between scoped values as "normal" values?

@lancelafontaine
Copy link
Contributor Author

Having scoped values be fetcheable from the existing methods Hash-like methods could work! I'm assuming that if a scoped value is set, it should always take precendence over the unscoped value. In that case, we'd have to be okay with always showing the scoped value if it is set, even if the unscoped value was set more recently. Eg.

context.scoped_merge!(value: 'old')
context[:value] = 'new'
assert_equal('old', context[:value])
assert_equal({ value: 'old' }, context.to_h)

# later, in sibling resolver ...
assert_equal('new', context[:value])
assert_equal({ value: 'new' }, context.to_h)

Otherwise, the implementation would need to keep track of the order in which scoped vs. unscoped values were inserted, or at least which one was most recently inserted, which also works.

@rmosolgo
Copy link
Owner

rmosolgo commented Dec 9, 2019

always showing the scoped value if it is set

👍

…ropagating values specific to a field and its children
@lancelafontaine
Copy link
Contributor Author

With the new API for getters, this is ready for a round of review 😄


# @!method [](key)
# Lookup `key` from the hash passed to {Schema#execute} as `context:`
def_delegators :@provided_values, :[]=
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔 is it inconsistent that the getter [] will magically return scoped values but the setter []= won't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The API could be a little confusing if the developer isn't aware of the existence of #scoped_merge!. Using a combination of #[]= and #scoped_merge can also lead to misleading behaviour with #[] because it always retrieves the value from @scoped_context if it exists regardless of the existing value in @provided_values, as described in #2634 (comment)

context.scoped_merge!(value: 'old')
context[:value] = 'new'
assert_equal('old', context[:value])

I can definitely see how this usage of the API can be misleading. I could also see this more as an edge case though. This only arises when values are set in ☝️ configuration, and using scoped_merge!(special_key: 1) in combination with [:special_key] = 2 is probably not what that developer means to do, because by doing [:special_key] = 2, they are voiding the benefit of having :special_key be scoped.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I think what you have makes sense 👍

Also wondering if set_scoped(key, value) should exist in addition to scoped_merge! as a convenience. If a common use case is setting a single key then it might match people's normal usage of setting a hash key?

Meaning most people would use []= instead of merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! I like set_scoped(key, value) as a convenience. I'll push a commit shortly.

Copy link
Contributor Author

@lancelafontaine lancelafontaine Dec 10, 2019

Choose a reason for hiding this comment

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

Added Context#scoped_set!(key, value) in commit afc292a.

@rmosolgo rmosolgo added this to the 1.9.17 milestone Dec 13, 2019
@rmosolgo
Copy link
Owner

🎉 Thanks for all your work exploring and implementing this! I'm going to follow up on some old issues asking for this feature.

@tjwallace
Copy link

Would it be possible to add/update docs with how to use this? Looks like a useful feature!

@omarpr
Copy link

omarpr commented Jul 20, 2022

As the previous comment also stated, would it be possible to get some documentation on how to get this to work? I need this feature right now for something we are working on and I have tried to use context.set_scoped(:course, course) and all I get is an undefined method error. I'm sure I'm accessing the wrong context but I don't know what else to do.

@omarpr
Copy link

omarpr commented Jul 20, 2022

As the previous comment also stated, would it be possible to get some documentation on how to get this to work? I need this feature right now for something we are working on and I have tried to use context.set_scoped(:course, course) and all I get is an undefined method error. I'm sure I'm accessing the wrong context but I don't know what else to do.

I just got it to work, but still, having documentation would be useful :)

Thanks!

@rmosolgo
Copy link
Owner

There's documentation at https://graphql-ruby.org/queries/executing_queries.html#scoped-context, please open a new issue (or submit a PR) if you think it can be improved!

ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext marker interface in your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

For now I decided to use a marker interface `ScopedContext`. Although I'm not fan of marker
interfaces, it was the easiest way to get this working. We could also make this an option that
needs to be passed to the `Executor`. Something like `$useScopedContext = false` (disabled by
default).

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext marker interface in your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

For now I decided to use a marker interface `ScopedContext`. Although I'm not fan of marker
interfaces, it was the easiest way to get this working. We could also make this an option that
needs to be passed to the `Executor`. Something like `$useScopedContext = false` (disabled by
default).

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext marker interface in your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

For now I decided to use a marker interface `ScopedContext`. Although I'm not fan of marker
interfaces, it was the easiest way to get this working. We could also make this an option that
needs to be passed to the `Executor`. Something like `$useScopedContext = false` (disabled by
default).

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext marker interface in your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

For now I decided to use a marker interface `ScopedContext`. Although I'm not fan of marker
interfaces, it was the easiest way to get this working. We could also make this an option that
needs to be passed to the `Executor`. Something like `$useScopedContext = false` (disabled by
default).

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext marker interface in your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

For now I decided to use a marker interface `ScopedContext`. Although I'm not fan of marker
interfaces, it was the easiest way to get this working. We could also make this an option that
needs to be passed to the `Executor`. Something like `$useScopedContext = false` (disabled by
default).

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext interface on your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext interface on your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext interface on your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 26, 2023
By implementing the ScopedContext interface on your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `d`
* `f` receives the cloned context from `d`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 30, 2023
By implementing the ScopedContext interface on your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `a`
* `f` receives the cloned context from `e`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
ruudk added a commit to ruudk/graphql-php that referenced this pull request May 30, 2023
By implementing the ScopedContext interface on your context object, you can ensure that
scope is only passed downwards and not shared with other fields.

Example:
```graphql
query {
  a {
    b {
      c
    }
    d {
      e {
        f
      }
    }
  }
  g {
    h {
      i
    }
  }
}
```

In the above situation, the following will happen:
* `a` receives the cloned context from the executor
* `b` receives the cloned context from `a`
* `c` receives the cloned context from `b`
* `d` receives the cloned context from `a`
* `f` receives the cloned context from `e`
* `g` receives the cloned context from the executor
* `h` receives the cloned context from `g`
* `i` receives the cloned context from `h`

References:
graphql/graphql-js#2692
rmosolgo/graphql-ruby#2634
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.

Middleware via hooks in the "round-trip" of a field
5 participants