Counterfact unable to parse escaped characters in paths

[jivewise]

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch counterfact@1.1.1 for the project I'm working on.

It looks like sometimes, if a path for the schema/api contains escapable characters, counterfact will have issues with generating the types, and throw an error into the generated file.

For example, when I run:

npx counterfact open-api.yaml ./api

with the following in open-api.yaml:

paths:
  '/api/v2/{TestId}/cancel':
    put:
      operationId: Cancel
      summary: API to cancel
      description: Method to cancel
      parameters:
        - name: TestId
          in: path
          description: >-
            A unique identifier
          required: true
          schema:
            type: string
        - name: X-Request-ID
          description: >
            The `X-Request-Id` is used to track and identify individual requests
            as they traverse various systems to enable tracing
          in: header
          required: false
          schema:
            type: string
            format: uuid
            example: 368b8d8f-5d07-4612-9523-d92fd57b0341
      responses:
        '200':
          description: success
          headers:
            X-Request-ID:
              description: >
                The `X-Request-Id` is used to track and identify individual
                requests as they traverse various systems to enable tracing
              required: false
              schema:
                type: string
                format: uuid
                example: 368b8d8f-5d07-4612-9523-d92fd57b0341
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/CancelResponse'
        '400':
          description: >
            Bad Request: The server could not understand the request due to an
            invalid request.
          headers:
            X-Request-ID:
              $ref: >-
                #/paths/~1api~1v2~1%7BTestId%7D~1cancel/put/responses/200/headers/X-Request-ID
              required: true
          content:
            application/problem+json:
              schema:
                type: object
                required:
                  - type
                  - title
                properties:
                  type:
                    description: >-
                      A string path which uniquely classifies the domain of the
                      error.
                    type: string
                  title:
                    description: A user readable string title of the error type.
                    type: string
                  details:
                    description: >-
                      An array of strings which provide additional details for
                      the error message.
                    type: array
                    items:
                      type: string
                  data:
                    description: >-
                      An array of strings which provide additional data for the
                      error message.
                    type: array
                    items:
                      type: object
                      properties:
                        description:
                          type: string
                        element:
                          type: string
                        elementValue:
                          type: string
components:
  schemas:
    CancelResponse:
      type: object
      properties:
        message:
          type: string
          format: string
          description: Message
          example: Cancelled successfully.

I will get the following error in api/types/paths/\~1api\~1v2\~1%7BTestId%7D\~1cancel/put/responses/200/headers/X-Request-ID.ts:

export type X_Request_ID = {
  /* error creating export "X_Request_ID" for types/paths/~1api~1v2~1%7BTestId%7D~1cancel/put/responses/200/headers/X-Request-ID.ts: TypeError: Cannot read properties of undefined (reading 'data')
    at SchemaTypeCoder.writeCode (file:///Users/jing/jha/wires-ui/node_modules/counterfact/dist/typescript-generator/schema-type-coder.js:83:64)
    at SchemaTypeCoder.write (file:///Users/jing/jha/wires-ui/node_modules/counterfact/dist/typescript-generator/type-coder.js:7:21)
    at file:///Users/jing/jha/wires-ui/node_modules/counterfact/dist/typescript-generator/script.js:50:51
    at async Promise.all (index 0)
    at async Promise.all (index 3)
    at async Repository.finished (file:///Users/jing/jha/wires-ui/node_modules/counterfact/dist/typescript-generator/repository.js:31:13)
    at async Repository.writeFiles (file:///Users/jing/jha/wires-ui/node_modules/counterfact/dist/typescript-generator/repository.js:45:9)
    at async generate (file:///Users/jing/jha/wires-ui/node_modules/counterfact/dist/typescript-generator/generate.js:56:5)
    at async CodeGenerator.generate (file:///Users/jing/jha/wires-ui/node_modules/counterfact/dist/typescript-generator/code-generator.js:17:9)
    at async start (file:///Users/jing/jha/wires-ui/node_modules/counterfact/dist/app.js:43:13) */
};

Here is the diff that solved my problem:

diff --git a/node_modules/counterfact/dist/typescript-generator/requirement.js b/node_modules/counterfact/dist/typescript-generator/requirement.js
index 78183e4..b8fcfde 100644
--- a/node_modules/counterfact/dist/typescript-generator/requirement.js
+++ b/node_modules/counterfact/dist/typescript-generator/requirement.js
@@ -21,7 +21,7 @@ export class Requirement {
     }
     select(path, data = this.data, basePath = "") {
         const [head, ...tail] = path.split("/");
-        const branch = data[this.unescapeJsonPointer(head)];
+        const branch = data[unescape(this.unescapeJsonPointer(head))];
         if (!branch) {
             return undefined;
         }
diff --git a/node_modules/counterfact/dist/typescript-generator/schema-type-coder.js b/node_modules/counterfact/dist/typescript-generator/schema-type-coder.js
index da1a28e..709a130 100644
--- a/node_modules/counterfact/dist/typescript-generator/schema-type-coder.js
+++ b/node_modules/counterfact/dist/typescript-generator/schema-type-coder.js
@@ -80,13 +80,13 @@ export class SchemaTypeCoder extends TypeCoder {
     }
     writeCode(script) {
         // script.comments = READ_ONLY_COMMENTS;
-        const { allOf, anyOf, oneOf, type } = this.requirement.data;
+        const { allOf, anyOf, oneOf, type, schema } = this.requirement.data;
         if (allOf ?? anyOf ?? oneOf) {
             return this.writeGroup(script, { allOf, anyOf, oneOf });
         }
         if (this.requirement.has("enum")) {
             return this.writeEnum(script, this.requirement.get("enum"));
         }
-        return this.writeType(script, type);
+        return this.writeType(script, schema?.type || type);
     }
 }

This issue body was partially generated by patch-package.

[pmcelhaney]

Hooray! I'm a user of patch-package but this is the first time it's been used to create an issue against one of my own repos. I'll make this change and have a new release out by the end of the week.

[jivewise]

Great, thank you @pmcelhaney. Let me know if you have any questions.

FWIW, I'm not sure this is the right place to escape things, but it was the best I could find for now. Our docs are bundled together by swagger-cli which seems to escape the parameters and urls in these cases.

[pmcelhaney]

Actually I think the problem is the curly braces should not be URL encoded here.

                #/paths/~1api~1v2~1%7BTestId%7D~1cancel/put/responses/200/headers/X-Request-ID

i.e. { and } should not be escaped as %7B and %7D.

If you change the line to this does it break other tooling?

                #/paths/~1api~1v2~1{TestId}~1cancel/put/responses/200/headers/X-Request-ID

[jivewise]

@pmcelhaney Yea, I was chatting with @dethell about this. I agree it probably shouldn't be escaped, and originally it isn't, but that's what the output looks like after running swagger-cli bundle on the yaml file to include all the components/schemas we would need. I think it's a pretty common use case to use this tool on generated documentation, but let me know if you disagree.

I did try a different bundler (@redocly/cli) which seemed to work better, but that one seems to run into naming conflicts more than swagger-cli.

[pmcelhaney]

Okay, pragmatism over pedantry. I imagine that scenario is 1000X more common than having a % in a path that's not meant to be an escape character. I'll go ahead and implement your fix. Thanks!

[pmcelhaney]

Actually, it's kind of weird that you have a $ref that points to an object under paths. It would be better to put the definition of X-Request-Header and put it under components.

openapi: 3.0.0
info:
  title: Cancellation API
  version: 2.0.0
paths:
  '/api/v2/{TestId}/cancel':
    put:
      operationId: cancelTest
      summary: Cancel a Test
      description: Method to cancel a test by its unique identifier.
      parameters:
        - name: TestId
          in: path
          description: A unique identifier for the test to be canceled.
          required: true
          schema:
            type: string
        - name: X-Request-ID
          in: header
          description: >
            The `X-Request-ID` is used to track and identify individual requests
            as they traverse various systems to enable tracing.
          required: false
          schema:
            $ref: '#/components/headers/X-Request-ID/schema'
      responses:
        '200':
          description: Successful cancellation of the test.
          headers:
            X-Request-ID:
              $ref: '#/components/headers/X-Request-ID'
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/CancelResponse'
        '400':
          description: Bad Request - Invalid input provided.
          headers:
            X-Request-ID:
              $ref: '#/components/headers/X-Request-ID'
          content:
            application/problem+json:
              schema:
                type: object
                required:
                  - type
                  - title
                properties:
                  type:
                    description: >
                      A string path which uniquely classifies the domain of the
                      error.
                    type: string
                  title:
                    description: A user-readable string title of the error type.
                    type: string
                  details:
                    description: >-
                      An array of strings providing additional details for the
                      error message.
                    type: array
                    items:
                      type: string
                  data:
                    description: >-
                      An array of objects providing additional data for the
                      error message.
                    type: array
                    items:
                      type: object
                      properties:
                        description:
                          type: string
                        element:
                          type: string
                        elementValue:
                          type: string
components:
  headers:
    X-Request-ID:
      description: >
        The `X-Request-ID` is used to track and identify individual requests as
        they traverse various systems to enable tracing.
      schema:
        type: string
        format: uuid
        example: 368b8d8f-5d07-4612-9523-d92fd57b0341
  schemas:
    CancelResponse:
      type: object
      properties:
        message:
          type: string
          description: Message confirming cancellation.
          example: Cancelled successfully.

This should work, except the type definition for your header will be "undefined" instead of "string" because of another bug I just discovered.

[jivewise]

@pmcelhaney Thanks for looking into this. I really think it's a question of whether we want this tool to work with swagger-cli and it's bundle function. If not, then I don't think it's worth fixing this. Those $refs are right in the original swagger files, but have been updated due to the bundler.

I actually don't think it's necessary to support it in particular. FWIW, I used the @redocly/cli bundler and the output from that tool worked way better and was more readable. Maybe we can add that into the docs somewhere?

[pmcelhaney]

Eh, I'll go ahead and implement the workaround. If someone later runs into an issue because they have a % in an object name and it's not meant to be an escape character, we can reevaluate.