Recursive validation schema compiles but fails at runtime

Question

Recursive validation schema compiles but fails at runtime

BenoitRanque opened this issue a year ago · 5 comments

BenoitRanque commented a year ago

Prerequisites

I have written a descriptive issue title
I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.23.2

Plugin version

No response

Node.js version

18.12.1

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

11

Description

The following error is thrown when validating a response using a response schema, when trying to return a response:

{
  "statusCode": 500,
  "error": "Internal Server Error",
  "message": "can't resolve reference #/definitions/Type from id merged_45bedfdc-0c19-4649-8739-66c2f819bb2e"
}

This is unexpected: it's my understanding that schemas should be validated on server startup.
The error also does not throw if the response does not include the problematic parts of the schema.

This is most likely an AJV bug, but I was not able to reproduce it trivially. I would appreciate help with isolating the Fastify-AJV interaction to reproduce this without the Fastify server at all.

Steps to Reproduce

Full sample code:

import Fastify from "fastify";

const schema = {
  $schema: "http://json-schema.org/draft-07/schema#",
  title: "Schema Response",
  type: "object",
  required: [],
  properties: {
    types: {
      description: "A list of types.",
      type: "object",
      additionalProperties: {
        $ref: "#/definitions/Type",
      },
    },
  },
  definitions: {
    Type: {
      title: "Type",
      description: "Types track the valid representations of values as JSON",
      oneOf: [
        {
          description: "A named type",
          type: "object",
          required: ["name", "type"],
          properties: {
            name: {
              description:
                "The name can refer to a primitive type or a scalar type",
              type: "string",
            },
            type: {
              type: "string",
              enum: ["named"],
            },
          },
        },
        {
          description: "A nullable type",
          type: "object",
          required: ["type", "underlying_type"],
          properties: {
            type: {
              type: "string",
              enum: ["nullable"],
            },
            underlying_type: {
              description: "The type of the non-null inhabitants of this type",
              allOf: [
                {
                  $ref: "#/definitions/Type",
                },
              ],
            },
          },
        },
      ],
    },
  },
};

const data = {
  types: {
    Int: {
      type: "named",
      name: "Int",
    },
    NullableString: {
      type: "nullable",
      underlying_type: {
        type: "named",
        name: "String",
      },
    },
  },
};

async function start_server() {
  const server = Fastify({
    logger: true,
  });

  server.get(
    "/",
    {
      schema: {
        response: {
          200: schema,
        },
      },
    },
    () => {
      return data;
    }
  );

  try {
    await server.listen({ port: 3030 });
  } catch (error) {
    server.log.error(error);
    process.exit(1);
  }
}

start_server();

Fastify version: 4.23.2
To reproduce the error, run the server, and perform a GET request against it.

Expected Behavior

The response should be validated successfully, or it should fail to validate, but not fail to resolve the validation schema, except on server startup.

Note, the following using only AJV does not error:

const schema = { /* same as above */ }
const data = { /* same as above */ }

const validate = ajv.compile(schema);
const valid = validate(data);
if (!valid) {
  console.log("validation failed");
  console.log(validate.errors);
} else {
  console.log("validation succeeded");
}

I strongly suspect this is an AJV issue, but as the above did not repro the problem I would appreciate any help in isolating the issue.

Workarounds

I've observed two ways to avoid this issue:

use $recursiveRef instead of $ref. I don't believe this is the correct use case for $recursiveRef, but it does make the problem go away
specify an $id on the root, then qualify $refs with the id before the #

Possibly related issues

Using discriminator in a definition causes failure to compile if oneOf contains a reference fastify/fastify#1899
Is it possible to create a recursive schema definition? fastify/fastify#659

Answer 1 · 2023-10-04T08:19:02.000Z

Ajv is good - the response schema is processed by https://github.com/fastify/fast-json-stringify and I'm not sure $ref is supported in the additionalProperties

Answer 2 · 2023-10-04T16:06:13.000Z

I get the exact same error with this version of the code, where types is an array instead.

import Fastify from "fastify";

const schema = {
  $schema: "http://json-schema.org/draft-07/schema#",
  title: "Schema Response",
  type: "object",
  required: [],
  properties: {
    types: {
      description: "A list of types.",
      type: "array",
      items: {
        $ref: "#/definitions/Type",
      },
    },
  },
  definitions: {
    Type: {
      title: "Type",
      description: "Types track the valid representations of values as JSON",
      oneOf: [
        {
          description: "A named type",
          type: "object",
          required: ["name", "type"],
          properties: {
            name: {
              description:
                "The name can refer to a primitive type or a scalar type",
              type: "string",
            },
            type: {
              type: "string",
              enum: ["named"],
            },
          },
        },
        {
          description: "A nullable type",
          type: "object",
          required: ["type", "underlying_type"],
          properties: {
            type: {
              type: "string",
              enum: ["nullable"],
            },
            underlying_type: {
              description: "The type of the non-null inhabitants of this type",
              allOf: [
                {
                  $ref: "#/definitions/Type",
                },
              ],
            },
          },
        },
      ],
    },
  },
};

const data = {
  types: [
    {
      type: "named",
      name: "Int",
    },
    {
      type: "nullable",
      underlying_type: {
        type: "named",
        name: "String",
      },
    },
  ],
};

async function start_server() {
  const server = Fastify({
    logger: true,
  });

  server.get(
    "/",
    {
      schema: {
        response: {
          200: schema,
        },
      },
    },
    () => {
      return data;
    }
  );

  try {
    await server.listen({ port: 3030 });
  } catch (error) {
    server.log.error(error);
    process.exit(1);
  }
}

start_server();

The original schema that gave me issues did not have types under additionalProperties, it was nested under a couple other types that I removed to get as minimal a reproduction as I could.

I also want to note that I found ways around this that suggest this is a bug:

use $recursiveRef instead of $ref. I don't believe this is the correct use case for $recursiveRef, but it does make the problem go away
specify an $id on the root, then qualify $refs with the id before the #

Answer 3 · 2023-10-04T16:25:44.000Z

Can confirm that using JSON.stringify instead does in fact work as expected:

  server.setSerializerCompiler(
    ({ schema, method, url, httpStatus, contentType }) => {
      return (data) => JSON.stringify(data);
    }
  );

Should I close this issue and re-open on the fast-json-stringify repo instead?

Answer 4 · 2023-10-07T21:47:25.000Z

@BenoitRanque Hi, I see where is the problem. Unfortunately you have a killing combo of recursive allOf and anyOf. It's a hard one. I don't have time to solve it right now, but I have a workaround for you. Why don't you drop an unnecessary allOf subschema? If it's needed for some reason, ping me. We will figure out something else.

Before:

const schema = {
  underlying_type: {
    description: 'The type of the non-null inhabitants of this type',
    allOf: [
      {
        $ref: '#/definitions/Type'
      }
    ]
  }
}

After:

const schema = {
  underlying_type: {
    description: 'The type of the non-null inhabitants of this type',
    $ref: '#/definitions/Type'
  }
}

Answer 5 · 2023-10-19T05:36:31.000Z

Hi @ivan-tymoshenko

Unfortunately, I cannot modify this schema easily, as this is generated automatically and is in practice much more complex.

I did manage to narrow down the use-case that results in this particular combination showing up:

    #[derive(Serialize, Deserialize, JsonSchema)]
    struct ObjectField {
        /// The type of this field
        #[serde(rename = "type")]
        r#type: Type,
    }

    #[derive(Serialize, Deserialize, JsonSchema)]
    #[serde(tag = "type", rename_all = "snake_case")]
    enum Type {
        Nullable { underlying_type: Box<Type> },
        Named { name: String },
    }

    #[test]
    fn test() {
        println!(
            "{}",
            serde_json::to_string_pretty(&schema_for!(ObjectField)).unwrap()
        );
    }
}

It turns out, what triggers this is specifically this documentation comment:

        /// The type of this field
        #[serde(rename = "type")]
        r#type: Type,

Somehow the generator decides that the presence of this comment justifies the allOf (rather than a direct reference). I do not know that this should be considered wrong, perhaps the generator is trying to cover some use-cases I cannot think of.

I don't know if that should be considered a bug. Either way, we are not currently blocked on this, and it's possible we won't be using this code at all, but I did want to follow up. Thanks for looking into it!