TagUniquenessValidationRule not working properly

[latalkdesk]

The validation of the TagUniquenessValidationRule is giving NullPointerException when document does not contain tags.
Also it is not validating the correct spec rule as it is comparing the current node tag with the root node tags.
It should validate the current list of tags if they are unique by name.

For instance this is a slice of a valid Document:

  "components": {
    "messages": {
      "message1": {
        "name": "message_name",
        "tags": [{ "name": "tag_unique" }]
      }
    }
  },
  "tags": [ { "name": "tag_unique" } ]

And this is an invalid tag definition:

  "tags": [ { "name": "tag_unique" }, { "name": "tag_unique" } ]

The class

public void visitTag(Tag node) {
    List<Tag> tags = ((Document) node.root()).getTags();  <--- Fetching the wrong tags. Should be the parent node.
    int tcount = 0;
    for (Tag tag : tags) {            // <--- NullPointerException when document does not have tags
        if (equals(tag.getName(), node.getName())) {
            tcount++;
        }
    }
    this.reportIf(tcount > 1, node, node.getName(), map("tagName", node.getName()));
}

Fetching the parent Node requires checking the Node type to fetch tags as there is no common Interface for Node with tags.
We could modify the traversal and add the current list to the context when visiting lists and then access it in the rule. 🤷

What is the best way to proceed?

Thanks! 👍

[EricWittmann]

I will investigate! I'll start with some new tests and we can go from there. Thank you for the report.

[EricWittmann]

OK I see - this rule was clearly written for OpenAPI and is being misapplied to AsyncAPI. I didn't realize that the two specs differed in this way: OpenAPI uses a simple array of strings to apply tags to an operation whereas AsyncAPI allows an array of Tag objects at the root and when applying/categorizing e.g. messages. Interestingly, only the root collection of tags says anything about uniqueness. It's not clear whether the uniqueness rule should actually apply to the Tag objects defined in Server, Operation, and Message entities.

A simple fix for this would be to add a check if the Tag node's parent is the root node:

I think that would result in a technically accurate implementation. But I do wonder about the spirit of the AsyncAPI spec, that uniqueness should be enforced at each level, not just at the root level.

What do you think?