Rework GraphQL API surface and migrate to Hot Chocolate from GraphQL.NET

[chyyran]

Common Framework Level Improvements

These are common framework-level improvements that assist in the Hot Chocolate infrastructure, but also benefit elsewhere.

• Lazy IQueryable in IGameLibrary
• Projection from a IServiceRepository instance within the Compose hook of an IComposable to a System.IServiceProvider
• IInstallables now must specify the name of the source IGameInstaller that produced it.
• Renamed IControllerElementMappings to IControllerElementMappingProfile
  • IControllerElementMappingsStore is now IControllerElementMappingProfileStore
• IServiceContainer is now an internal interface
• Added a EmulatorCompatibility API to IEmulatorOrchestrator to better define compatibility
• Allow higher-kinded IAsyncEnumerable in IAsyncJobQueue
  • Allow embedded cancellation in IAsyncJobQueue
• Cancellation is now better supported by all APIs that implement IAsyncEnumerable<T>
• Installation API can now provide descriptions for TaskResults
• SeedTree is now collapsed with a stack instead of recursively.
• GameConfigurationExtensionProvider now implements method to fetch the collection GUID of an individual Value.
• GameConfigurationExtension now implement methods to fetch a configuration collection using the collection GUID rather than a profile name

Todo List

• Port object types over to Hot Chocolate and migrate them to S.F.Remoting
  • Stone Platforms
  • Game Records
  • File Records
  • Configuration
  • Controller
  • Meta-objects (plugins)
• Port mutations
  • Installation
  • Scraping
  • Game Record CRUD
  • Orchestration
  • Configuration
  • Input
• Errors
  • Replace ArgumentException in S.F.R.G.FrameworkQueries`
• Document input types
• Remove old GraphQL Remoting
• Write integration tests for S.F.R.GraphQL.
• Parameterized subscriptions
  • Orchestration
  • Installation
  • Scraping
• Figure out how to test resolvers

Hot Chocolate Wishlist features

• Auto-pagination would be nice.
• Should we consider dropping the QueryBuilder API in favour of using Hot Chocolate directly?
  • QueryBuilder is to be deprecated and removed in favour of full access to services in a GraphQL context.
• The Snowflake GraphQL models should be maintained internally within S.F.Remoting, leaving extension points.
  • Currently GraphQLFrameworkQueries is a plugin, but that means extending these GraphQL models require a reference to the plugin!
  • If we do do this, we will should switch to Hot Chocolate. Right now due to QueryBuilder type resolution is done at the function level at the latest possible moment, so third-party GraphQL extensions have no access to these types.
  • If nothing else, GameType needs an extension point.
• We'll need some way to extend the provided IServiceRepository for dependency injection or just give up on injecting internal dependencies for Query types, for example in InstallationQueryBuilder. We either consider polluting the IServiceRepository API by making it user extensible local to the instance, or give up entirely on injecting local dependencies, and have the parameterized constructor construct references.
  • We inject the Snowflake service container at query resolution time.
• If possible, we should autoresolve Uuid into the specific object instance required, for example in the configuration APIs. Perhaps we can do this with field middleware?
  • Not necessary outside of mutations, if we structure the query properly.
• GraphQL Schema Testing! We don't get LOC coverage with GraphQL.NET because of QueryBuilder shenanigans, but with Hot Chocolate testing is easier.
• Rewrite API to minimize top-level queries. Expect to see only these top level APIs in Query
  • games(GameFilter) : GameConnection
    • files(FileFilter) : [FileInfo!]
    • saves(SaveFilter)
    • record: GameRecord
    • orchestration
  • fs (See Filesystem Remoting API #645)
    • installables
  • stone
    • platforms
    • controllers
  • devices
    • mappings
  • plugins
  • meta
• Full Relay support for most objects

• Spin off GraphQL into a separate support plugin

Why should Snowflake.Framework.* be privileged within GraphQL?

The old GraphQL implementation started off which the philosophy that nothing should be privileged. Although I've made GraphQL progressively more and more privileged within Snowflake, I wanted to stay protocol-agnostic. While eventually Kestrel and the GraphQL.NET runtime were merged into Services for necessity, the actual GraphlQL interfaces remained separated.

The issue was that while the GraphQL interfaces adopted some of the philosophy, much of it was written to be protocol-agnostic. In many ways, it looked more like a REST API separated into units of work, and that was because it was descended from a REST API. There was no useful sense of hierarchy, that is to say, types and function calls were treated separately.

The issue with this approach is that if anyone were to want to extend existing GraphQL functionality, it would not be possible without referencing the existing types, and thus the GraphQLFrameworkQueries plugin. In essence, GraphQLFrameworkQueries was intended to be glue, but in reality was a core part of the API if GraphQL was to be priviledged.

With HotChocolate, I take a different approach. I blur the lines behind Services and Data, and implement some of the API within Snowflake.Framework.Remoting. Anything that can be returned by a method call should have a GraphQL analogue within Snowflake.Framework.Remoting, such that consumers can simply say "return [a Snowflake object]" in a field definition, without having to reference Plugin assemblies. Then, anything that provides access to such data, such as a library extension or a service call, will be put into GraphQLFrameworkQueries. A thin wrapper across HotChocolate provides complete access to the server for more advanced use cases.

[chyyran]

Moved from #636

[lgtm-com]

This pull request introduces 4 alerts when merging c4d7ea4 into 22df00b - view on LGTM.com

new alerts:

• 3 for Useless assignment to local variable
• 1 for Call to obsolete method

[chyyran]

Seems like the latest HC v11 Previews have support for TrySetServices, so we should port to v11 as soon as possible.

[chyyran]

Out of the queries, device and controller are the low-hanging fruit, followed by meta-objects such as querying plugins and version.

game { configuration } is quite daunting to port, but we'll see what we can do there. I don't think there will be too many API level improvements, since its already a rather complex API.

As for mutations, we'll need to do some research on subscriptions, but for now I want to preserve the pull nature of scraping and installation.

[lgtm-com]

This pull request introduces 10 alerts when merging 4f0584e into 3074c7a - view on LGTM.com

new alerts:

• 8 for Call to obsolete method
• 2 for Useless assignment to local variable

[lgtm-com]

This pull request introduces 10 alerts when merging e5f991a into 3074c7a - view on LGTM.com

new alerts:

• 8 for Call to obsolete method
• 2 for Useless assignment to local variable

[lgtm-com]

This pull request introduces 10 alerts when merging a17a19f into 3074c7a - view on LGTM.com

new alerts:

• 8 for Call to obsolete method
• 2 for Useless assignment to local variable

[lgtm-com]

This pull request introduces 10 alerts when merging 9a9578a into 3074c7a - view on LGTM.com

new alerts:

• 8 for Call to obsolete method
• 2 for Useless assignment to local variable

[lgtm-com]

This pull request introduces 10 alerts when merging df5cf13 into 3074c7a - view on LGTM.com

new alerts:

• 8 for Call to obsolete method
• 2 for Useless assignment to local variable

[lgtm-com]

This pull request introduces 10 alerts when merging 7ee4e97 into 3074c7a - view on LGTM.com

new alerts:

• 8 for Call to obsolete method
• 2 for Useless assignment to local variable

[codecov]

Codecov Report

Merging #663 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #663   +/-   ##
=======================================
  Coverage   71.64%   71.64%           
=======================================
  Files         484      484           
  Lines       15007    15007           
  Branches      848      848           
=======================================
  Hits        10752    10752           
  Misses       3916     3916           
  Partials      339      339           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3ba4f0...d3ba4f0. Read the comment docs.

[chyyran]

1ea2e14 and 6c68f23 fix #702

[lgtm-com]

This pull request introduces 21 alerts and fixes 6 when merging 6c68f23 into 3074c7a - view on LGTM.com

new alerts:

• 12 for Useless assignment to local variable
• 7 for Call to obsolete method
• 2 for Missing Dispose call on local IDisposable

fixed alerts:

• 3 for Useless assignment to local variable
• 1 for Call to obsolete method
• 1 for Dereferenced variable may be null
• 1 for Missing Dispose call on local IDisposable

[lgtm-com]

This pull request introduces 18 alerts and fixes 6 when merging e583a6c into 3074c7a - view on LGTM.com

new alerts:

• 9 for Useless assignment to local variable
• 7 for Call to obsolete method
• 2 for Missing Dispose call on local IDisposable

fixed alerts:

• 3 for Useless assignment to local variable
• 1 for Call to obsolete method
• 1 for Dereferenced variable may be null
• 1 for Missing Dispose call on local IDisposable

[lgtm-com]

This pull request introduces 15 alerts and fixes 6 when merging 3d41c6d into 3074c7a - view on LGTM.com

new alerts:

• 7 for Call to obsolete method
• 6 for Useless assignment to local variable
• 2 for Missing Dispose call on local IDisposable

fixed alerts:

• 3 for Useless assignment to local variable
• 1 for Call to obsolete method
• 1 for Dereferenced variable may be null
• 1 for Missing Dispose call on local IDisposable

[lgtm-com]

This pull request introduces 9 alerts and fixes 6 when merging 563aa39 into 3074c7a - view on LGTM.com

new alerts:

• 7 for Call to obsolete method
• 2 for Missing Dispose call on local IDisposable

fixed alerts:

• 3 for Useless assignment to local variable
• 1 for Call to obsolete method
• 1 for Dereferenced variable may be null
• 1 for Missing Dispose call on local IDisposable

[lgtm-com]

This pull request introduces 10 alerts and fixes 6 when merging 8154e4b into 3074c7a - view on LGTM.com

new alerts:

• 7 for Call to obsolete method
• 2 for Missing Dispose call on local IDisposable
• 1 for Useless assignment to local variable

fixed alerts:

• 3 for Useless assignment to local variable
• 1 for Call to obsolete method
• 1 for Dereferenced variable may be null
• 1 for Missing Dispose call on local IDisposable

[lgtm-com]

This pull request introduces 10 alerts and fixes 6 when merging ae5ec1a into 3074c7a - view on LGTM.com

new alerts:

• 7 for Call to obsolete method
• 2 for Missing Dispose call on local IDisposable
• 1 for Useless assignment to local variable

fixed alerts:

• 3 for Useless assignment to local variable
• 1 for Call to obsolete method
• 1 for Dereferenced variable may be null
• 1 for Missing Dispose call on local IDisposable

[lgtm-com]

This pull request introduces 10 alerts and fixes 6 when merging 38ca30b into 3074c7a - view on LGTM.com

new alerts:

• 7 for Call to obsolete method
• 2 for Missing Dispose call on local IDisposable
• 1 for Useless assignment to local variable

fixed alerts:

• 3 for Useless assignment to local variable
• 1 for Call to obsolete method
• 1 for Dereferenced variable may be null
• 1 for Missing Dispose call on local IDisposable