Following `$ref`s doesn't always work the way it's supposed to

[pmcelhaney]

In the code, a Requirement object refers to a node in the OpenAPI document. The Requirement#has(property) and Requirement#get(property) functions don't work when the node is a $ref.

This is not a straightforward fix because following references is an asynchronous operation, whereas has() and get() are currently synchronous. Following references is asynchronous because the reference may point to an external file.

However, I think I found a way to make following references synchronous. Using the @apidevtools/json-schema-ref-parser package, we can bundle() the OpenAPI document and all of the other documents to which it refers into one object at load time. That way, when Counterfact traverses the object it doesn't have to look to see if a $ref points to an external file and load it. Which means looking up references can be synchronous.

In theory, that will fix the bug, make the code simpler, and improve performance.

This function:

  async requirementAt(url, fromUrl = "") {
    debug("getting requirement at %s from %s", url, fromUrl);

    const [file, path] = url.split("#");
    const filePath = nodePath
      .join(fromUrl.split("#").at(0), file)
      .replaceAll("\\", "/")
      // eslint-disable-next-line regexp/prefer-named-capture-group
      .replace(/:\/([^/])/u, "://$1");
    const fileUrl = filePath === "." ? this.rootUrl : filePath;

    debug("reading specification at %s", fileUrl);

    const data = await this.loadFile(fileUrl);

    debug("done reading specification", fileUrl);

    const rootRequirement = new Requirement(data, `${fileUrl}#`, this);

    return rootRequirement.select(path.slice(1));
  }

Will be reduced to just:

  requirementAt(path) {
    return this.rootRequirement.select(path.slice(2));
  }

In order to ensure this.rootRequirement is defined, generate.js will have to call loadFile() on the specification immediately after instantiating it. We can make that cleaner by adding a static factory method to Specification.

[pmcelhaney]

Okay, the hard part is done: #1091

[pmcelhaney]

I think this is fixed in #1099. I added unit tests and made the changes to make them pass but I didn't get a chance to test the original issue today.

Also, the changes to the code are a little scary. I think at the end of the day this makes the Specification and Requirement classes simpler, but I need to refresh my mental model of how these two classes work.

[pmcelhaney]

Okay, I figured out what was confusing me in that code. There's a case where the value in Requirement#get(value) is not a string, so the code was blowing up on escapeJsonPointer(value). I had added a stupid special case to make the tests pass. Now that I've figured out what's going on I was able to remove the special case.