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

$ref in $ref causes path hash to be repeated. #52

Closed
supertong opened this issue Oct 11, 2017 · 3 comments
Closed

$ref in $ref causes path hash to be repeated. #52

supertong opened this issue Oct 11, 2017 · 3 comments

Comments

@supertong
Copy link

supertong commented Oct 11, 2017

Hey, first of all, thanks for the great library. I think I spot an edge case bug when using it.
Here is the JSON file that I have simplified a lot to demo the problem.

{
  "$ref": "#/definitions/doc_node",
  "definitions": {
    "top_level_node": {
      "type": "object"
    },
    "doc_node": {
      "type": "object",
      "properties": {
        "content": { "$ref": "#/definitions/top_level_node" }
      }
    }
  }
}

And when I tried to bundle it

const parser = new RefParser();
parser
  .bundle('./bug-schema.json')
  .then((s) => {
    console.log(JSON.stringify(s, null, 2))
  });

it generates output

{
  "$ref": "#/definitions/doc_node",
  "definitions": {
    "top_level_node": {
      "type": "object"
    },
    "doc_node": {
      "type": "object",
      "properties": {
        "content": {
          "$ref": "#/definitions/top_level_node/definitions/top_level_node"
        }
      }
    }
  }
}

As you can see, the 'definitions/top_level_node' is duplicated in doc_node. And I also did some debugging myself. It turns out that problem happens around here

Pointer.prototype.resolve = function(obj, options) {
  var tokens = Pointer.parse(this.path);

  // Crawl the object, one token at a time
  this.value = obj;
  for (var i = 0; i < tokens.length; i++) {
    if (resolveIf$Ref(this, options)) {
      // The $ref path has changed, so append the remaining tokens to the path
      this.path = Pointer.join(this.path, tokens.slice(i));  <== This line
    }

    var token = tokens[i];
    if (this.value[token] === undefined) {
      throw ono.syntax('Error resolving $ref pointer "%s". \nToken "%s" does not exist.', this.path, token);
    }
    else {
      this.value = this.value[token];
    }
  }

  // Resolve the final value
  resolveIf$Ref(this, options);
  return this;
};

I was trying to figure out what is the code doing here. My immature assumption is that resolveIf$Ref is checking if current object has a $ref and if it has one, it will be resolved. However, one thing I don't understand is the reason behind this path updating. May be it has something todo with line here ? Can we not update the path?

I understand you are busy and hope you could shed some light on this. I am happy to create a PR if you could understand the purpose of the path updating logic.

Cheers

@supertong
Copy link
Author

Ahh, I think I kind of understand how the path changes better now. And, according to the comment, I think we shall return false here?

function resolveIf$Ref(pointer, options) {
  // Is the value a JSON reference? (and allowed?)

  if ($Ref.isAllowed$Ref(pointer.value, options)) {
    var $refPath = url.resolve(pointer.path, pointer.value.$ref);

    if ($refPath === pointer.path) {
      // The value is a reference to itself, so there's nothing to do.
      pointer.circular = true;
    }
    else {
      var resolved = pointer.$ref.$refs._resolve($refPath, options);

      if ($Ref.isExtended$Ref(pointer.value)) {
        // This JSON reference "extends" the resolved value, rather than simply pointing to it.
        // So the resolved path does NOT change.  Just the value does.
        pointer.value = $Ref.dereference(pointer.value, resolved.value);
        return false;  <====== HERE
      }
      else {
        // Resolve the reference
        pointer.$ref = resolved.$ref;
        pointer.path = resolved.path;
        pointer.value = resolved.value;
      }

      return true;
    }
  }
}

@supertong
Copy link
Author

Hey, I raised a PR for it. Let me know if it's more complicated than I thought. :)

@JamesMessinger
Copy link
Member

Thank you very much for the PR! Great catch!

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

No branches or pull requests

2 participants