Papercut: duplicate type names do not cause an error when building a Schema object

[njlr]

This script demonstrates the issue:

#r "nuget: FSharp.Data.GraphQL.Server, 1.0.7"

open FSharp.Data.GraphQL
open FSharp.Data.GraphQL.Execution
open FSharp.Data.GraphQL.Types

type Book =
  {
    Title : string
    Year : int
  }

let bookType1 =
  Define.Object(
    name = "Book",
    description = "Book type 1",
    fields =
      [
        Define.Field(
          "title",
          String,
          fun ctx (x : Book) -> x.Title
        )
      ]
  )

let bookType2 =
  Define.Object(
    name = "Book",
    description = "Book type 2",
    fields =
      [
        Define.Field(
          "title",
          String,
          fun ctx (x : Book) -> x.Title
        )
        Define.Field(
          "year",
          Int,
          fun ctx (x : Book) -> x.Year
        )
      ]
  )

let schemaType : ObjectDef<unit> =
  Define.Object(
    name = "Query",
    fields =
      [
        Define.Field(
          "books1",
          ListOf bookType1,
          fun _ () -> []
        )
        Define.Field(
          "books2",
          ListOf bookType2,
          fun _ () -> []
        )
      ]
  )

let schema = Schema(schemaType)
let executor = Executor(schema)

let query = """
{
  books2 {
    title
    year
  }
}
"""

let result =
  executor.AsyncExecute (query)
  |> Async.RunSynchronously

let output, errors =
  match result.Content with
  | GQLResponseContent.Direct (output, errors) -> output, errors
  | x -> failwith $"Unsupported: %A{x}"

printfn $"Output:\n%A{output}\n"

for error in errors do
  printfn $"Error:\n%A{error}\n"

Output:
map []

Error:
("Field 'year' is not defined in schema type 'Book'.", ["books2"; "year"])

The two book types share a name, and the library silently picks bookType1.

I think that duplicate names should cause schema building to fail, so that this error is caught earlier.

let schema = Schema(schemaType) // Should raise an exception on an invalid schema

Any thoughts?

[xperiandri]

Is it on NuGet release package or in GitHub CI package?

[njlr]

Is it on NuGet release package or in GitHub CI package?

This is NuGet

[xperiandri]

Try package from GitHub, it may be fixed there

[njlr]

Try package from GitHub, it may be fixed there

Is there any reason the latest is not pushed to NuGet?

[xperiandri]

There are 2 important PRs left to merge

[xperiandri]

Please review them and give your opinion

[njlr]

Still an issue:

#r "nuget: FSharp.Data.GraphQL.Server, 2.2.1"

open FSharp.Data.GraphQL
open FSharp.Data.GraphQL.Types

type Book =
  {
    Title : string
    Year : int
  }

let bookType1 =
  Define.Object(
    name = "Book",
    description = "Book type 1",
    fields =
      [
        Define.Field(
          "title",
          StringType,
          fun ctx (x : Book) -> x.Title
        )
      ]
  )

let bookType2 =
  Define.Object(
    name = "Book",
    description = "Book type 2",
    fields =
      [
        Define.Field(
          "title",
          StringType,
          fun ctx (x : Book) -> x.Title
        )
        Define.Field(
          "year",
          IntType,
          fun ctx (x : Book) -> x.Year
        )
      ]
  )

let schemaType : ObjectDef<unit> =
  Define.Object(
    name = "Query",
    fields =
      [
        Define.Field(
          "books1",
          ListOf bookType1,
          fun _ () -> []
        )
        Define.Field(
          "books2",
          ListOf bookType2,
          fun _ () -> []
        )
      ]
  )

let schema = Schema(schemaType)
let executor = Executor(schema)

let query = """
{
  books2 {
    title
    year
  }
}
"""

let result =
  executor.AsyncExecute (query)
  |> Async.RunSynchronously

let output, errors =
  match result.Content with
  | GQLResponseContent.Direct (output, errors) -> output, errors
  | GQLResponseContent.RequestError errors -> dict [], errors
  | x -> failwith $"Unsupported: %A{x}"

printfn $"Output:\n%A{output}\n"

for error in errors do
  printfn $"Error:\n%A{error}\n"

Output:
seq []

Error:
{ Message = "Field 'year' is not defined in schema type 'Book'."
  Exception = ValueNone
  Path = Include ["books2"; "year"]
  Locations = Skip
  Extensions = Include (seq [[kind, Validation]]) }

[xperiandri]

2.2.1 does not contain the fix
3.0.0 should

try with such nuget.config

<configuration>
  <packageSources>
    <add key="NuGet official package source" value="https://api.nuget.org/v3/index.json" />
    <add key="FSProjects" value="https://nuget.pkg.github.com/fsprojects/index.json" protocolVersion="3" />
  </packageSources>
  <packageSourceCredentials>
    <FSProjects>
      <add key="Username" value="<your user name>" />
      <add key="ClearTextPassword" value="<your token>" />
    </FSProjects>
  </packageSourceCredentials>
  <packageSourceMapping>
    <packageSource key="FSProjects">
      <package pattern="FSharp.Data.GraphQL*" />
    </packageSource>
    <packageSource key="NuGet official package source">
      <package pattern="*" />
    </packageSource>
  </packageSourceMapping>
</configuration>

[xperiandri]

We do testing of the latest 3.0.0 changes on our project and almost came to release

[xperiandri]

@njlr currently the logic is such

FSharp.Data.GraphQL/src/FSharp.Data.GraphQL.Shared/TypeSystem.fs

Lines 1905 to 1911 in 434b4be

	member _.AddType (def : NamedDef, ?overwrite : bool) =
	let overwrite = defaultArg overwrite false
	let add name def overwrite =
	if not (map.ContainsKey (name)) then
	map.Add (name, def)
	elif overwrite then
	map.[name] <- def

FSharp.Data.GraphQL/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs

Lines 68 to 82 in 434b4be

	let compileMiddleware (ctx : SchemaCompileContext) (next : SchemaCompileContext -> unit) =
	let modifyFields (object : ObjectDef<'ObjectType>) (fields : FieldDef<'ObjectType> seq) =
	let args = [ Define.Input("filter", Nullable ObjectListFilter) ]
	let fields = fields |> Seq.map (fun x -> x.WithArgs(args)) |> List.ofSeq
	object.WithFields(fields)
	let typesWithListFields =
	ctx.TypeMap.GetTypesWithListFields<'ObjectType, 'ListType>()
	if Seq.isEmpty typesWithListFields
	then failwith <| sprintf "No lists with specified type '%A' where found on object of type '%A'." typeof<'ObjectType> typeof<'ListType>
	let modifiedTypes =
	typesWithListFields
	|> Seq.map (fun (object, fields) -> modifyFields object fields)
	|> Seq.cast<NamedDef>
	ctx.TypeMap.AddTypes(modifiedTypes, overwrite = true)
	next ctx

FSharp.Data.GraphQL/src/FSharp.Data.GraphQL.Server.Middleware/MiddlewareDefinitions.fs

Line 81 in 434b4be

ctx.TypeMap.AddTypes(modifiedTypes, overwrite = true)

How would you suggest to change that?