Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: foundation for OpenApi Parser #1814

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

RohinBhargava
Copy link
Contributor

@RohinBhargava RohinBhargava commented Nov 14, 2024

Note: Ignore any of the files in temporary, those are there so that boilerplate does not need to be implemented again.

This PR introduces the base nodes that are necessary for parsing OpenApi Spec.

It introduces 3 abstractions:

  • (private) ApiNode: This defines the domain model for the parser. It contains a context, containing an errorCollector and a Logger. It defines an interface for how to model outputting FDR shapes.
  • InputNodes: These extend from ApiNodes map to OpenApi specific nodes, and can be incrementally updated, so that pathing and location is maintained.
  • OutputNodes: These extend from ApiNodes and do not do anything differently (aside from defining how the constructor is shaped).

The nodes created are:
Primitives:

  • String --> these correspond to any type: "string" nodes we encounter in OpenApi spec

  • Number --> these correspond to any type: "integer | number" nodes we encounter in OpenApi spec

    • Integer: These map to "integer" type nodes and handle discrimination into the right FDR shapes.
    • Float: These map to "float" type nodes and handle discrimination into the right FDR shapes.
  • Schema: An input node that helps us recognize schemas/references as we encounter them in OpenApi spec.

  • Reference: An input node that helps us to resolve references or pass them as Alias types.

  • Object (and all derivatives): An output node that helps us to parse any schema from OpenApi into the correct Fdr shapes.

Additionally, there are a few of types files introduced for cleaner organization:

  • OpenApi types: this helps us share types between OpenApi 2, 3, 3_1 parsers
  • OpenApi formats: this keeps an exhaustive inventory of potential formats encountered on different primitives
  • FdrShapes: Groups FdrShapes that are closely related, so that top level nodes can share logic for all children

Copy link

vercel bot commented Nov 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
rbac.ferndocs.com ❌ Failed (Inspect) Nov 14, 2024 0:27am
1 Skipped Deployment
Name Status Preview Updated (UTC)
fern-shell ⬜️ Ignored (Inspect) Nov 14, 2024 0:27am

Copy link

github-actions bot commented Nov 14, 2024

📦 Next.js Bundle Analysis for fern-platform-monorepo

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Copy link

github-actions bot commented Nov 14, 2024

PR Preview

Copy link

github-actions bot commented Nov 14, 2024

Playwright test results

passed  95 passed
flaky  3 flaky
skipped  3 skipped

Details

stats  101 tests across 10 suites
duration  1 minute, 58 seconds
commit  c3768ba

Flaky tests

chromium › forward-proxy/nextjs.spec.ts › capture the flag
chromium › smoke/favicon.spec.ts › Check if favicon exists and URL does not return 404 for api.qdrant.tech
chromium › smoke/favicon.spec.ts › Check if favicon exists and URL does not return 404 for docs.getkard.com

Skipped tests

chromium › posthog.spec.ts › Posthog loads successfully
chromium › proxy.spec.ts › multipart-form upload
chromium › proxy.spec.ts › json request

import { SchemaObject } from "../../../openapi/shared/openapi.types";
import { createMockContext } from "../../createMockContext.util";

describe("ObjectNode", () => {
Copy link
Member

Choose a reason for hiding this comment

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

mega nit: i think it might be nicer to put these test files through out the code base as opposed to the top, that way you can see object.node.ts and object.node.test.ts side-by-side


if (input.allOf != null) {
this.extends = input.allOf
.map((type) => (isReferenceObject(type) ? FdrAPI.TypeId(type.$ref) : undefined))
Copy link
Member

Choose a reason for hiding this comment

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

note: its going to be very important to handle non reference allOfs


if (input.allOf != null) {
this.extends = input.allOf
.map((type) => (isReferenceObject(type) ? FdrAPI.TypeId(type.$ref) : undefined))
Copy link
Member

Choose a reason for hiding this comment

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

if a $ref is #/components/schemas/Person, ideally we store a TypeId that is Person

import { OpenAPIV3_1 } from "openapi-types";
import { ApiNodeContext, InputApiNode } from "../../../ApiNode";

export class EnumNode extends InputApiNode<OpenAPIV3_1.SchemaObject, FdrAPI.api.v1.read.PrimitiveType.Enum> {
Copy link
Member

Choose a reason for hiding this comment

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

nit: think we should move enum outside of primitives folder

return;
}

if (!isOpenApiNumberTypeFormat(input.format)) {
Copy link
Member

Choose a reason for hiding this comment

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

i don't think this actually needs to be the case, i.e. you can have numbers without formats

case "float":
case "sf-decimal":
case undefined:
this.type = "double";
Copy link
Member

Choose a reason for hiding this comment

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

note: we'll eventually want to update FDR to store the actual format

`Expected format for integer primitive, but got "${input.format}"`,
accessPath,
);
return;
Copy link
Member

Choose a reason for hiding this comment

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

same comment as flat, dont think this need to be a large failure

export class ErrorCollector {
errors: ValidationError[] = [];

addError(error: string, accessPath: string[]): void {
Copy link
Member

Choose a reason for hiding this comment

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

I think it also makes sense to collect warnings + errors,

Could see an example of collector.warn() and collector.error() as the two methods on the ErrorCollector.

@@ -0,0 +1,3 @@
import { OpenAPIV2, OpenAPIV3, OpenAPIV3_1 } from "openapi-types";
export type SchemaObject = OpenAPIV3_1.SchemaObject | OpenAPIV3.SchemaObject | OpenAPIV2.SchemaObject;
export type ReferenceObject = OpenAPIV3_1.ReferenceObject | OpenAPIV3.ReferenceObject | OpenAPIV2.ReferenceObject;
Copy link
Member

Choose a reason for hiding this comment

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

can we delete this (since we are only optimizing for OpenAPIV3_1 for now) ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants