Skip to content

adds initial Open API 3.1 schema emitter - #5035

Merged
Christopher Radek (chrisradek) merged 13 commits into
microsoft:feature/openapi3.1from
chrisradek:add-openapi-3-1-emitter
Nov 19, 2024
Merged

Christopher Radek (chrisradek) merged 13 commits into
microsoft:feature/openapi3.1from
chrisradek:add-openapi-3-1-emitter

Conversation

@chrisradek

@chrisradek Christopher Radek (chrisradek) commented Nov 8, 2024 •

Copy link
Copy Markdown
Member

Related to #5009 and #5010

This PR starts the work of getting an Open API 3.1 emitter in the @typespec/openapi3 package.

There are a few changes introduced in this PR:

  1. Added the OpenAPIDocument3_1 and related types. The biggest change here is updating the Open API schema to be based on Json Schema 2020-12. The Json Schema types don't currently exist in @typespec/json-schema so I added them to @typespec/openapi3 for now while iterating. Can move to @typespec/json-schema now or later if we want.
  2. Created the OpenAPISchemaEmitter3_1 class to target Open API 3.1 schemas. Currently completely standalone from Json Schema and Open API 3.0 schema emitters.
  3. Updated nearly all tests to now run against both Open API 3.x versions. Intentionally left out the enum tests and schema examples since those will be updated shortly and be different between the two versions.
  4. Added support for type: "null" in the 3.1 schema emitter. I squeezed this change in because the changes involved were actually really small, and this way TypeScript was happy we were emitting 3.1 schemas that matched the defined interface (no nullable). I copied the behavior that the Json Schema emitter uses for handling nulls.
  5. Updated exclusiveMinimum/Maximum to be a number value instead of a boolean to match the Open API 3.1 spec.

Note to reviewers: Hide whitespace - otherwise the test changes will look monstrous.

Comment thread packages/openapi3/test/circular-references.test.ts Outdated
Comment thread packages/openapi3/src/types.ts Outdated
@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:openapi3 Issues for @typespec/openapi3 emitter label Nov 8, 2024
Comment thread packages/openapi3/src/schema-emitter-3-1.ts
Comment thread packages/openapi3/src/schema-emitter.ts
Comment thread packages/openapi3/src/schema-emitter-3-1.ts
Comment thread packages/openapi3/src/schema-emitter-3-0.ts
}

return this.#applyConstraints(union, schema);
unionSchema(union: Union): ObjectBuilder<Schema> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like we have some parts to share, and the framework keep the same.
We can consider make the different part to be an abstract function (also need to mark the class as abstract), and resue the framework like

interface Schema1 { a: string }
interface Schema2 { b: string }

type Schemas = Schema1 | Schema2;

abstract class SchemaEmitterBase<T extends Schemas> {
    core() {
        // ...
        this.impl();
        // ...
    }
    protected abstract impl(): void;
}

class SchemaEmitter1 extends SchemaEmitterBase<Schema1> {
    public test() {
        super.core();
    }

    impl() {
        console.log("2")
    }
}

class SchemaEmitter2 extends SchemaEmitterBase<Schema2> {
    public test() {
        super.core();
    }
    impl() {
        console.log("1")
    }
}

const s1 = new SchemaEmitter1();
const s2 = new SchemaEmitter2();

s1.test();
s2.test();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I split out the discriminator logic into the base class (and was actually able to re-use that in one other place as well). The variant collection, merge check, and assertions are different between the 2 though due to how null is treated differently between the 2 specs. wrapWithObjectBuilder has only one difference right now (due to the nullable check that isn't relevant for 3.1) but as I mention elsewhere I think it's likely this function will change more.

If there's anything specifically you had in mind though please share!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got it. I feel like for general schema generation, we can split the function into several steps. Put the shared steps in base class and put changed steps in 3.0 or 3.1 class, so that the changes can be easily located and update.

createRootDoc(program, serviceType, serviceVersion) {
return createRoot(program, serviceType, specVersion, serviceVersion);
},
createSchemaEmitterCtor: createWrappedOpenAPI3SchemaEmitterClass,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about Playground? We don't need to passing constructors.

function createEmitter(v: OpenApiVersion): () => SchemaEmitter<BaseOpenAPI3SchemaEmitter<SchemaCreator>> {
  switch (v) {
    case "3.1.0": {
      const sh = () => ({b: "1"}); // can be used to create a schema in base class
      const oas3Emiter = new OpenAPI3_1SchemaEmitter(sh);
      const emitter = new SchemaEmitter(oas3Emiter);
      return () => emitter;
    }
    case "3.0.0": 
    default: {
      const sh = () => ({a: "0"});
      const oas3Emiter = new OpenAPI3SchemaEmitter(sh);
      const emitter = new SchemaEmitter(oas3Emiter);
      return () => emitter;
    }
  }
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure I fully understand what you're suggesting - is it to have createSchemaEmitter return the AssetEmitter directly instead of the constructor that's passed to the current createAssetEmitter function? (Not sure what sh is).

I can do that - so instead of:

schemaEmitter = createAssetEmitter(
      program,
      createSchemaEmitterCtor(metadataInfo, visibilityUsage, options, xmlModule),
      context,
    );

you'd have

schemaEmitter = createSchemaEmitter(program, context, metadataInfo, visibilityUsage, options, xmlModule);

Might need to actually use named props instead given the number of positional args but is that what you meant here?

@wanlwanl Wanpeng Li (wanlwanl) Nov 19, 2024 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry about the previous comment is not clear, I'll use real names next time.
I'm suggesting add a new constructor:

class OpenAPI3SchemaEmitter {
  constructor(
    emitter: AssetEmitter<Record<string, any>, OpenAPI3EmitterOptions>,
    metadataInfo: MetadataInfo,
    visibilityUsage: VisibilityUsageTracker,
    options: ResolvedOpenAPI3EmitterOptions,
    xmlModule: XmlModule | undefined,
  ) {
    super(emitter, metadataInfo, visibilityUsage, options, xmlModule);
    ...
  }
}

and then use it in openapi-spec-mappings.ts:

export function getOpenApiSpecProps(specVersion: OpenAPIVersion): OpenApiSpecSpecificProps {
  switch (specVersion) {
    case "3.1.0": {
      const createOpenAPI3Schema = () => ({...}); // can be used to create a schema in base class
      const oas3Emiter = new OpenAPI3_1SchemaEmitter(sh);
      const emitter = new OpenAPI3SchemaEmitter(oas3Emiter);
      return () => emitter;
    }
    case "3.0.0": 
    default: {
      const createOpenAPI3Schema = () => ({root: ...});
      const oas3Emiter = new OpenAPI3SchemaEmitter(sh);
      const emitter = new OpenAPI31SchemaEmitter(oas3Emiter);
      return () => emitter;
    }
  }
}

The sh is used to create schema of oas 3.0/3.1 in base class (OpenAPI3SchemaEmitterBase)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The AssetEmitter needs to be passed a constructor though - you can't give it an instance of a class.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see, there's a limit in emitter framework of createAssetEmitter function, it needs constructor.
I feel like passing an interface to create type emitter is a common pattern? So that we don't need to extend OpenAPI31SchemaEmitter in order to pass the constructor. I feel like customers also face the same difficulty to create a type emitter with more parameters.
In the below example, the interface is typeEmitterCreator: TypeEmitterCreator<T, TOptions>

// packages/openapi3/src/schema-emitter-3-1.ts
export class OpenAPI31SchemaEmitter extends OpenAPI3SchemaEmitterBase<OpenAPISchema3_1> { 
  constructor(
    assetEmitter: AssetEmitter<OpenAPI3Schema, OpenAPI3EmitterOptions>, 
    metadataInfo: MetadataInfo,
    visibilityUsage: VisibilityUsageTracker,
    options: ResolvedOpenAPI3EmitterOptions,
    xmlModule: XmlModule | undefined
  ) {}
}

class OpenAPI3TypeEmitterCreator implements TypeEmitterCreator<OpenAPI3Schema, OpenAPI3EmitterOptions> {
  constructor(
    assetEmitter: AssetEmitter<OpenAPI3Schema, OpenAPI3EmitterOptions>, 
    metadataInfo: MetadataInfo,
    visibilityUsage: VisibilityUsageTracker,
    options: ResolvedOpenAPI3EmitterOptions,
    xmlModule: XmlModule | undefined
  ) {}
  create: (assetEmitter) => new OpenAPI3Emitter(this.assetEmitter, this.metadataInfo, this.visibilityUsage, this.options, this.xmlModule);
};

// packages/openapi3/src/openapi-spec-mappings.ts
export function getOpenApiSpecProps(specVersion: OpenAPIVersion): OpenApiSpecSpecificProps {
  switch (specVersion) {
    case "3.1.0": {
      return new openAPI3TypeEmitterCreator(assetEmitter, metadataInfo, visibilityUsage, options, xmlModule);
    }
    case "3.0.0": 
    default: {
      // ...
    }
  }
}

// packages/compiler/src/emitter-framework/asset-emitter.ts
interface TypeEmitterCreator<T, TOptions extends object> { 
  create: (assetEmitter: AssetEmitter<T, TOptions>) => TypeEmitter<T, TOptions>
}

export function createAssetEmitter<T, TOptions extends object>(
  program: Program,
  //TypeEmitterClass: typeof TypeEmitter<T, TOptions>,
  typeEmitterCreator: TypeEmitterCreator<T, TOptions>,
  emitContext: EmitContext<TOptions>,
): AssetEmitter<T, TOptions> {
  // ...
  const typeEmitter_ = typeEmitterCreator.create(assetEmitter);
  // ...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh sure - I don't disagree that the pattern I've seen is to pass a factory function instead of an actual constructor, but createAssetEmitter is part of the entity framework and not really something that should be changed as part of this PR.

Comment thread packages/openapi3/src/schema-emitter.ts Outdated
Comment thread packages/openapi3/src/types.ts
Comment thread packages/openapi3/test/works-for.ts Outdated
@chrisradek
Christopher Radek (chrisradek) merged commit f33affe into microsoft:feature/openapi3.1 Nov 19, 2024
github-merge-queue Bot pushed a commit that referenced this pull request Jan 14, 2025
This PR adds support for Open API 3.1 in the `@typespec/openapi3`
package.

The changes in this PR includes:
#4946 - adds `output-spec-versions` emitter option
#5035 - adds initial open api 3.1 schema emitter
#5245 - add support for tuples
#5246 - add support for enums backed by multiple types
#5310 - playground support for array of constants emitter options 
#5407 - add support for json-schema decorators
#5549 - add support for binary types in Open API 3.1

---------

Co-authored-by: Christopher Radek <Christopher.Radek@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:openapi3 Issues for @typespec/openapi3 emitter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants