Significant refactor to several annoying inhibitors to stability

[unclecheese]

Summary

Whilst working on the upgrade for silverstripe-gatsby to v4, I came across a lot of things that were slowing me down, and it just ended up being a rabbit hole, but these are all really solid, worthwhile improvements IMO, and to the extent that they break APIs, they're very unlikely to be relied upon by current users.

Issues addressed/resolved

• GraphQL 4: Controller constructor is too greedy #351
• Remove BuildState global state #341
• GraphQL 4: filter fields include custom getters #334
• GraphQL 4: Resolver registries are global, not scoped to schema #328
• Nested filter fields don't work #307
• GraphQL 4: Do type caching in build step? #270
• GraphQL v4: Child pages not able to be viewed in DRAFT  #349
• .. And potentially a few others that we'll have to test after merge

Dependent PRs

• NEW: Compatibility with GraphQL 4 changes silverstripe-asset-admin#1173 (asset-admin)
• NEW Compatibility with GraphQL 4 changes silverstripe-admin#1171 (admin)
• DOCS: Document GraphQL 4 BuildState changes silverstripe-framework#9835 (framework, docs)
• NEW: Compatibility with GraphQL 4 changes silverstripe-versioned#319 (versioned)
• NEW: Compatibility with GraphQL 4 changes silverstripe-elemental#870 (elemental)
• Compatibility with refactor silverstripe-graphql-devtools#35 (graphql-devtools)
• Fix GraphQL v4 compat silverstripe-frameworktest#84 (frameworktest)

TODO

• Aaron: Reconcile two admin.graphql.types commits
• Aaron: Check in with CMS Squad about merge
• Aaron: Remaining elemental fix (unless we can split that out of the PR)
• Fix upgrade guide (you mentioned something on community Slack)
• Ingo: Ensure other module builds are passing
• Ingo: Ensure Behat tests are passing They're not, but its likely unrelated. Let's see what CI says.
• Aaron: Answer Ingo’s question about getter side effects (Significant refactor to several annoying inhibitors to stability #352 (comment))
• Ingo: Write up something about the new deployment complexities of writing files into PUBLIC_PATH in main docs
• Aaron: Regression test a bit (elemental, asset admin)
• Ingo: Merge!
• Alpha1 changelog and release

Changelog

• Refactor context provider API so that they now plug into QueryHandlerInterface. They
  can set and get their own context based on their own intnernal logic. Before:
  $user = $context['currentUser']; after: $user = UserContextProvider::get($context)

• BuildState API is gone. Use new SchemaContextProvider in resolvers. Pass SchemaContext
  objects to the small number of classes that needed it in their internals.

• AbstractNestedInputPlugin is gone. This never made any sense to tie something
  so utilitarian to an arbitrary inheritance chain. It has been refactored as NestedInputBuilder
  and is far more single-purpose. No longer concerned with model and field mapping, but rather
  just building recursive input types and adding them to the schema.

• Refactor QuerySort, QueryFilter to use input builder

• Field mapping is no longer exhaustively encoded in the resolvers. New
  SchemaContextProvider allows resolvers to look at the schema as persisted to a
  PHP array in the code gen and invoke methods like $schema->mapField($type, $fieldName)

• No more resolveContext for DataObject\Resolver since field mapping is handled externally now

• Migrate type Controller::introspectTypes to standalone service (SchemaTranscriber)
  invoked through event dispatcher

• Remove IntrospectionProvider extension. Static only. Leaving it to the user to decide never made any sense, and it just slowed down the UI in the CMS. Now that we're doing code gen, it seems like the static solution is more consistent.

• Remove ModelConfigurationProvider: Didn't make any sense. Models are now
  responsible for fetching their own configurations.

• Remove defaultResolver: Didn't make any sense. Just set a default resolver
  property and allow it to be overriden.

• Resolver discovery is now lazy to avoid race conditions in config. If resolvers
  are null at code gen time, discovery is invoked and applied.

• Controller now only accepts a schema key and uses SchemaFactory to boot.

• Schema::quiet() is gone. Schema is quiet by default to better support autobuilding
  and test suites. Opt into verbose with setVerbose(true).

• CSRFMiddleware and HTTPMethodMiddleware now rely on new ContextProviders
  to better encapsultate their state concerns.

• Better error reporting:

  • QueryHandler now pretty prints the stack trace
  • Resolver level errors now throw ResolverFailure which express tons of useful
    context, including the query path, the field, the type, the args, and most importantly,
    the resolver chain. No more guessing which closure failed.
  • ResolverFailure relies on new resolverComposition field in encoded types.
  • Updates to code gen templates to support the above, and other tidyness like
    variable assignment to save code execution and comments to show closing bracket relationships.

• ComposedResolver is now stateful, mostly for diagnostic purposes. Before:
  ComposedResolver::create(): Closure after: ComposedResolver::create($list)->toClosure()

• All resolvers use ComposedResolver for consistency, even when no middle/afterwares

• ComposedResolver no longer needs separate arguments for primary, before, and after
  resolvers. Just a stack that leaves ordering a concern of the code composing the class.
  (this was a @todo annotation FWIW)

• SchemaModelInterface now must declare getPropertyForField (gql field -> object property mapping)

• SchemaModelInterface now must declare getSchemaContext and getModelConfiguration

• New hasNativeField in FieldAccessor helps with knowing what fields are
  filterable/sortable. Basically ORM insight.

• Fields using the model property (a bit of a polymorphic hack to support
  cases like CMS needing to know what the SiteTree model is named), will
  get just-in-time assignment before code generation time to prevent race conditions
  in the config.

• New suite of relational FakeProduct objects for testing nested queries, filters, mutations.

• New top-to-bottom integration test that creates, updates, and reads nested data structures with filters/sorting, field aliases, etc.

• NB: CodeGenerationStore is now responsible for type mapping and field mapping. This
  is a leaky abstraction as it's tightly coupled to SchemaContext now.

• New StorableSchema layer eliminates unpredictable state of Schema through process(). The readonly value object returned by createStorableSchema() is expected to
  be a complete set of primitive objects that are consumable by a storage service.

• SchemaFactory is now SchemaBuilder, which has four clearly defined functions, and eliminates the confusion around using Schema at query time:

  • getConfig(string $schemaKey): ?SchemaContext -- Get the cached configuration of a Schema. Useful for resolvers
  • boot(string $schemaKey): Schema -- Load all the configuration in to a Schema object. Useful for the configuration/build process.
  • getSchema(string $schemaKey): ?GraphQLSchema -- Retrieve a queryable graphql-php Schema object from the storage service. Useful at query time.
  • build(Schema $schema, bool $clear = false): GraphQLSchema -- put the Schema through a storage service and return its queryable schema

• SchemaStorageInterface is now the concern of SchemaBuilder rather than Schema, which results in a diminished API surface for Schema.

• Validation of Schema moved to StorableSchema, as this is the only time the schema truly needs to be valid (before storage)

• Type/field mapping done just-in-time for schema storage

• Prefer event dispatcher over extend() hooks in controller

• SchemaContext is now SchemaConfig

• Schema has new getState() for persisting state during build

• Improved handling of "empty" schemas that shouldn't be built, but also shouldn't error (e.g. default out of the box)

[chillu]

Amazing work as usual, thanks for chipping away at this! Definitely a tough one to peer review as a single commit, but I appreciate the effort to create a changelog - that helps a bit ;)

SchemaModelInterface now must declare getPropertyForField (gql field -> object property mapping)
SchemaModelInterface now must declare getSchemaContext and getModelConfiguration

I don't understand this layer enough to determine if that's a good change. Might need to write a bit of an architecture overview doc to get my head around it.

BuildState API is gone. Use new SchemaContextProvider in resolvers. Pass SchemaContext
objects to the small number of classes that needed it in their internals.

Yep that's a great change, a lot more explicit and testable.

[chillu]

Remove IntrospectionProvider extension. Static only. Leaving it to the user to decide never made any sense, and it just slowed down the UI in the CMS. Now that we're doing code gen, it seems like the static solution is more consistent.

How will that work when the schema build is performed outside of the production environment, e.g. during CI when creating a deployment package? That environment won't have access to the production assets/ folder.

[unclecheese]

How will that work when the schema build is performed outside of the production environment, e.g. during CI when creating a deployment package? That environment won't have access to the production assets/ folder.

OK, now:

• Remove GeneratedAssetHandler dependency from SchemaTranscriber (injector hell for one little file)
• admin module adds event for graphqlSchemaBuild.admin (only applies to admin schema)
• SchemaTranscriber has new rootDir property that defaults to PUBLIC_PATH
• Graphql special assets.yml config (behind moduleexists) sets rootDir of SchemaTranscriber from PUBLIC_PATH to ASSETS_PATH
• Document edge case for running SS without assets installed -- block those .graphql files to outside traffic

Why not add the transcriber event in only assets scenarios?

It seems likely that this will come up again in other context as we expand graphql to other corners of the CMS. Using fragments on unions and interfaces, given our setup, seems like a pretty standard case, so we might as well future proof.

[chillu]

I've sent a (very straightforward) devtools PR for this: silverstripe/silverstripe-graphql-devtools#33

[chillu]

Also fixed frameworktest with silverstripe/silverstripe-frameworktest#84

[unclecheese]

Test failure is due to the silverstripe/versioned dependency.

[chillu]

Refactoring the StorableSchema as a value object is great, much cleaner. I'm still a bit confused why we need a Schema that's separate from a SchemaBuilder during the build phase. Feels like this could be inlined into SchemaBuilder to avoid confusion between the two "schema" types which perform very different objectives. But I don't think it's worth extra effort at this point, you describe the responsibilities pretty clearly in PHPDoc - and we've already identified the need for an architecture overview doc.

How will that work when the schema build is performed outside of the production environment, e.g. during CI when creating a deployment package? That environment won't have access to the production assets/ folder.

I don't think your explanation covers it. The "run cms without assets module" edge case is worth covering, but I'm more concerned about building the schema during CI. If this <schema>.types.graphql file is created anywhere other than PUBLIC_PATH, it simply won't be accessible by the browser. You removed the dynamic fallback which is fine in general (since creation is part of a build that's a necessary precondition anyway). But you also removed the dynamic passthrough of a file's contents when they don't live in PUBLIC_PATH.

That's important to resolve, because it means that CI-based builds would break admin/assets. The schema cache files exist on the webserver(s) because they're part of the production deployment. The types file doesn't, because it's either stuck in CI on PUBLIC_PATH (which will get filtered out of the deployment artefact), or because it's in a path where the webserver can't directly serve it without going through file_get_contents().

[chillu]

Just noted the overlap with silverstripe/silverstripe-framework#9688 - by merging this PR we're making a stronger (not yet semver binding) commitment to supporting silverstripe/event-dispatcher