asyncapi / diff Goto Github PK

Reason/Context

Please try answering few of those questions

• Why we need this improvement?
• How will this change help?
• What is the motivation?

The current overrides functionality provides a simple way to classify the addition, removal or edit of a JsonPath as non-breaking, breaking or unclassified. I'd like to be able to extend diff to classify changes to message payloads and classify these as non-breaking, breaking or unclassified.

As AsyncAPI supports arbitrary payload formats, classifying changes can be arbitrarily complex. For example, to classify a change to JsonSchema, one would need to detect the addition or removal of schema properties and also whether those properties are required. This is technically achievable using extension to JsonPath, such as complex filters, this becomes quickly cumbersome and error prone.

This would also have applications outside of the message payload.

Description

Please try answering few of those questions

• What changes have to be introduced?
• Will this be a breaking change?
• How could it be implemented/designed?

Strawman design:

• Extend overrides so that add, remove and edit could also take a callback value as well as the existing strings:

overrides: {
  "/servers/*/protocol": {
    add: addProtocolCallback,
    remove: "breaking",
    edit: editProtocolCallback,
  },
}

• Optionally extend overrides so that a single callback can be used to evaluate any change to a jsonPath:

overrides: {
  "/servers/*/protocol": protocolCallback,
},

• The call back should take the following arguments:
  • firstDocumentFragment - the subtree of the first document from the jsonPath
  • secondDocumentFragment - the subtree of the second document from the jsonPath
  • context
    • firstDocument - first document root
    • secondDocument - second document root
    • path - the JsonPointer to the current instance of the override rule
    • operation - one of "add", "remove" and "edit"
• The call back should return one of: "non-breaking", "breaking" and "unclassified"

This would be a non breaking change.

Inspiration could be taken from spectral's custom functions

A tool that compares two AsyncAPI specifications and checks compatibility. I am thinking about an equivalent to OpenAPI-Diff or Swagger-Diff.
https://stackoverflow.com/questions/64372598/compare-two-asyncapi-specifications

This issue defines a list of tasks that need to be performed in this repo to make sure it's ci/cd automation works long term without any issues.

It is up to maintainers to decide if it must be addressed in one or multiple PRs.

Below are 3 different sections describing 3 different important ci/cd changes.

IMPORTANT-START
For GitHub workflows that contain This workflow is centrally managed in https://github.com/asyncapi/.github/ you do not have to perform any work. These workflows were already updated through the update in .github. The only exception is the workflows related to nodejs release. More details in Upgrade Release pipeline - in case of nodejs projects section
IMPORTANT-END

Deprecation of way data is shared between steps

Every single GitHub Action workflow that has echo "::set-output name={name}::{value}" need to be updated to follow echo "{name}={value}" >> $GITHUB_OUTPUT

We do not yet know when set-output will stop working. Previous disable date was 31.05 but now then say community needs more time.

For more details read official article from GitHub

Deprecation of node12

2nd bullet point is still relevant for you even if your projects in not nodejs project

• Every single workflow that uses setup-node action needs an update to follow v3 version of this action, and make sure minimum node 14 is used
• Now this part is more complex. Problem with node12 is that node-based GitHub Actions were using it in majority as a runtime environment. Look for example at this action.yaml file for setup-node action v2. So the job that you have to do is go through all the workflows, and verify every single action that you use, make sure you are using the latest version that is not based on node12. I already did review a lot of actions as part of this PR so maybe you will find some actions there and can copy from me. For example action/checkout needs to be updated to v3.

Node12 end of support in action is probably September 27th.

For more details read official article from GitHub

Upgrade Release pipeline - in case of nodejs projects

ignore this section if your project is not nodejs project

You have 2 options. You can:

A. choose to switch to new release pipeline using instruction from asyncapi/.github#205

B. stay with old release pipeline, and manually update GitHub workflows and actions used in it, you can inspire a lot from this PR asyncapi/.github#226

I definitely recommend going with A

Workflows related to release:

• .github/workflows/if-nodejs-release.yml
• .github/workflows/if-nodejs-version-bump.yml
• .github/workflows/bump.yml

Reason/Context

This is to replace the old AsyncAPI logo in this repo's README with the banner attached below that represents the new branding.

Here are a few guidelines for this change as well:

1. Make sure you are using Markdown syntax only
2. Be sure to remove the old logo as well as the old title of the repo as this image will replace both elements
3. Make sure you link this image to the website: https://www.asyncapi.com
4. If there is any description text below the repo title, let's make it left-aligned if it isn't already, so as to match the left-alignment of the content in the new banner image

Download the image file:
github-repobanner-diff.png.zip

Banner preview

Please note that this is only a preview of the image, the contributor should download and use the above zip file

More info on this issue can be found here

We should have an option to skip the parsing step, that we if we have a parsed document we can just provide it, and perform the diff on it.

I'm thinking of something like:

function main(firstDocument, secondDocument, config) {
    if(config.skipParsing) // skip parsing
}

Describe the bug

When using allOf in a model, breaking changes are not detected.

How to Reproduce

Using these 2 spec files:

spec1.yaml
spec1.yaml

Breaking change rules:

Breaking Change Rules

I then use this command:
asyncapi diff ./spec1.yaml ./spec2.yaml -o breaking-changes-override.json -t breaking -f json

Notice that no breaking changes are detected even if we deleted a required statement.

Expected behavior

The breaking changes should be detected by this section of the rules:

"/components/schemas/*/required": {
    "add": "breaking",
    "remove": "breaking",
    "edit": "breaking"
  },
  "/components/schemas/*/required/*": {
    "add": "breaking",
    "remove": "breaking",
    "edit": "breaking"
  },

Deciding the output of this library

Main points to look at:

• Extensible(in case we need to extend the format, but without breaking the previous version).
• Integration with different tools.

Classification

Our diff library will have three types of change classification:

• Breaking change
• Non-breaking change
• Unclassified
  Some changes (for example: JSON schema object in message payload) can be changed or extended with external libraries, in which case it becomes hard to document every change as breaking/non-breaking.
  Thus, some changes will be considered unclassified. Which we are basically telling the user to handle the changes themselves.

Output

{
  "breaking": [
    {
      "type": "breaking",
      "action": "delete",
      "entity": "<the root level property name>",
      "path": "<JSON-path>",
      "before": "<the value of deleted field>"
    }
  ],
  "nonbreaking": [
    {
      "type": "nonbreaking",
      "action": "edit",
      "entity": "<the root level property name>",
      "path": "<JSON-path>",
      "before": "<the value of original field>",
      "after": "<the value of edited field>"
    }
  ],
  "unclassified": [],
  "breakingDifferencesFound": true
}

• type will tell what type of change it is. While it is evident from the JSON itself, it might help in our wrapper function.
• action will have
  • add - Something has been added
  • delete - Something has been deleted
  • edit - Something has been edited
• entity will contain the root level property in which the change has been introduced. This might be helpful when we want to get a change in a certain field.
• path - The JSON path where the change has been made.
• before - The original data
• after - The changed data

So, this will be the output of our library. I made this issue because there might be some points I may have missed, so I would like your feedback on the output :)

Real world example:

firstSpec.yml

asyncapi: 2.1.0
info:
    title: Title
servers:
    production:
        protocol: ‘http’

secondSpec.yml

asyncapi: 2.1.0
info:
    title: Title
servers:
    production:
        protocol: ‘https’

Diff output:

{
    "breaking" : [
    {
        "type": "breaking",
        "action": "modify",
        "entity": "servers",
        "path": "/servers/production/protocol",
        "before": "http",
        "after": "https"
        
}
],
    "nonbreaking": [],
    "unclassified": [],
    "breakingDifferencesFound": true
}

Reason/Context

This Issue is used to track changes needed to support AsyncAPI v3. As a code owner, please edit this list of TODO tasks in order to properly track the progress 🙂 Once this issue is closed it means that v3 is now fully supported in this library.

Remaining tasks:

• Update to parser v2
• Figure out how to handle comparing v3 with v2 (if at all)
• Add diff for v3 documents
• Once asyncapi/parser-js#780 is solved, update parser dependency (might be automated)

This library currently supports JSON, & YAML output.

We need to support HTML output.

Check #75

The format of HTML output

The goal is to generate a HTML(or jsx?) that can be reused in AsyncAPI studio as well. Thus, we need to have a detailed discussion on the format.

The current API Documentation, which is generated automatically using jsdoc2md, doesn't include the underlying types.
For example,

It is not clear what the DiffOutputItem "looks" like.

The reason:

JSDoc2md does not include TypeScript types when it looks into JSDoc comments, or rather, It doesn't find the typescript types when generating the documentation.

Request:

We want the underlying TypeScript types to be present inside the auto-generated API documentation.

The output will depend on how jsdoc2md parses the types. One can look into jsdoc2md documentation to see how to parse the TypeScript types :)

Since this library can be used in the playground as well, we should have the ability to pass the spec document as a string as well.
That way we can get the spec from an input field as well.

Just creating an issue here for this. Will be working on it in the upcoming PR.

Describe the bug

Was trying this library out for the demo, and imports were not working. Looking at node_modules directory, the fetched data is this repo's TS source code instead JS ouput.

How to Reproduce

npm install @asyncapi/diff

Expected behavior

The imports should work as expected and the code fetched from npm should be the compiled JS source code.

I'm thinking that some scripts may be missing from the package.json file.

@derberg this needs immediate attention.

For auto-generation of API docs & README.

Example asyncapi/optimizer#21

Reason/Context

I came across this tool https://diff2html.xyz/, and after playing around with it, it's a great way to show diff using HTML.

I wonder if people would find it useful to be able to use AsyncAPI diff tool to output the diff into HTML?

Maybe a new command, that output a directory with the files in it so they can host them, or view them etc...

Why we need this improvement?

Guess we don't I just think it would be nice to offer users another output vs the cli/console.

How will this change help?

Maybe attract more people to use the lib?

What is the motivation?

Some great work done here, just wanted to see if we could extend it :)

As said in #9 , we will map the standard to a standard.json file, so during the "classification of change" process, our library will read this standard.json file and classify the changes based on that.

So, now we need to decide the format of this standard file.

Here's what I came up with:

{
  "asyncapi": {
    "add": "non-breaking",
    "delete": "breaking",
    "edit": "breaking"
  },
  "servers": {
    "add": "non-breaking",
    "delete": "breaking",
    "edit": "breaking",
    "{placeholder}": {
      "add": "non-breaking",
      "delete": "breaking",
      "edit": "breaking",
      "protocol": {
        "add": "non-breaking",
        "delete": "breaking",
        "edit": "breaking"
      }
    }
  }
}

This is a part of the standard.

We have some root level fields such as asyncapi and servers. Each field has a certain action and the corresponding classification associated with it.
For example, if we remove a server, the servers -> delete will say that it's a breaking change.

Since some properties allow patterned field, we have a {placeholder} for that.

Example: if we have a production server, and we change its protocol. Our standard will map it like servers -> {placeholder} -> protocol -> edit.

From #10 (comment) @magicmatatjahu pointed out another format of the standard.

"/servers/{serverName}": {
  "add": "non-breaking",
  "remove": "breaking",
  "update": "breaking",
},
"/servers/{serverName}/protocol": {
  "add": "breaking",
  "remove":"'breaking",
  "update":"'breaking",
}

Which is basically a "flattened" version of our previous standard format.

But there might be some problem with this format when we want to get a specific data.

Taking the previous example, changing the protocol in our production server, the JSON path will be /servers/production/protocol. Parsing this path will raise the question on which path will be considered a placeholder. The placeholder can be /production(/servers/{placeholder}/protocol) or /protocol (/servers/production/{placeholder}), therefore getting the data from standard might be a bit problematic.

Reason/Context

• Why we need this improvement?
  Since, this library currently supports showing diff in a JSON format. I think it would be good to have more different ways of showing the output. For example, we can show the output in YAML, HTML or Markdown format.
• How will this change help?
  This library will have more ways of showing the output, and since, this library will be integrated into AsyncAPI CLI and playground, we will have the ability to show different type of output on those platforms as well.
• What is the motivation?
  Extend this library :)

Description

• Detailed description:
  Current JSON format only goes so far, we need to provide more ways to show the diff. For starters, we can show the output in HTML as well as Markdown format. And then, as need arises, we can focus on having other different types of output as well(like YAML, etc.)

• How could it be implemented/designed?
  The implementation is not really clear right now(I'll be working on that now), but since the diff generates JSON output, we need to convert this JSON output into HTML, Markdown.
  As for how this HTML, Markdown format will look like, we need to have a detailed discussion on that as well. I'll be working on an initial draft for the output, and from there we can flesh out the design.

Description

The plan is to have this library as a github action, where a change in an AsyncAPI document present in a repo can tell us what kind of changes we are making. Helpful with info like breaking changes, etc.

• What changes have to be introduced?
  None in the library itself.

This issue will act as the medium to have discussion about the action, what kind of action we want, what should the format of the output, etc.

I have a few questions:

• What kinds of input do we want?
  For example, what files should the action check? What type of change should the action output?
• What kind of output do we want?
• Where do we want the output to be?
• Where do we want to source code to live? (In the org, or my personal github account?)

I'll be working on this issue.

CC @jonaslagoni

Having a discussion with @onbit-syn, we found some problems with having the standard file in our application as well as inside this repository:

1. How would we be able to read this file in a browser?
2. The standard file will have to be updated frequently, then how would we be able to present those changes to use diff library without publishing another version of this library.

Also, we wanted to have our user provide their own standard file, either through a URL or a file.

So, to overcome these problems, we have thought of an approach:

Have the standard file hosted somewhere else, and let our library fetch it when it is invoked.

We will just provide our library with a default URL of the standard file, which it will fetch when generating the diff. It will overcome the problem of updating the library, because we can just update the standard file, and our library will fetch it during its usage.

This approach will also be best suited in browser environments as well.

What are your thoughts on it?

Hi code owners!

@aayushmau5 @vinitshahdeo @onbit-uchenik @magicmatatjahu @derberg

I'm looking to use this project in my PRs (via the AsyncAPI CLI) to check if the contract has changed between the main spec and the branch spec that is currently in PR. When the contract is broken, the PR validation fails.
The problem right now is that it seems my definition of breaking changes is not the same as yours.

When I check the standard.ts file, I see a lot of things that are considered breaking but that do not break the contract.
A simple example of this would be changing /info/version that is considered breaking. I can see this happening pretty often when a developer see that there is a new version of AsyncAPI and update the spec to the latest version.

I also see that there are currently no validation for breaking changes for components and schemas which is, in my opinion, the main reasons for breaking changes...

Thus, I would like to know what is your definition of a breaking change? I think it might be interesting to add it to the readme too after the discussion here is done.

In my opinion, a breaking change should be anything that do not respect the contract of the "old" spec.
A service using the "new" spec that passed the test (no breaking changes) should have no impact on any clients either as a publisher or a subscriber.

This is usually by preventing the removal of fields, adding required fields, removing/renaming complete components/schemas, changing or deleting channels, etc...

When I look at the standard right now, it doesn't seem to match this definition at all so I would really like to have your feedback on what is considered a breaking change before I propose a PR.

===

P.S: If we have different "breaking" definitions, I could propose a concept for "presets" which would allow anyone to use different "standard" following a specific preset depending on its definition of a breaking change?

Let me know what you think, and I will help with a PR very soon.
Thanks

In order to generate the difference between two JSON data, either we can use an existing library or implement our own JSON diffing library, but before we make that decision, I think it's important that we look at some great existing JSON diffing libraries.
If none of them work according to our needs, then can look into implementing our own.

After looking at some most popular as well as some most suitable libraries, I came up with these diffing libraries:

1. https://github.com/flitbit/diff (deep-diff)
2. https://github.com/andreyvit/json-diff (diff)
3. https://github.com/benjamine/jsondiffpatch (jsondiffpatch)
4. https://github.com/chbrown/rfc6902 (rfc6902)
5. https://github.com/kpdecker/jsdiff (diff)
6. https://github.com/Starcounter-Jack/JSON-Patch (fast-json-patch)
7. https://www.npmjs.com/package/just-diff (just-diff)

After having tried them all, the most suitable library to me was: deep-diff

Reason:

1. No external dependencies, thus not having to worry about frequently updating the library.
2. Suitable diff output, this included the type of change, what's the current values, what was the previous one, and the path where the change has been made.

But before we agree on this, we have to make sure that this library is really suitable for us in terms of performance, the output, etc.
Therefore, this issue has been made in order to start a discussion on it. Also, whether we should really go with a ready-made library or make our own.

CC @vinitshahdeo

This library currently supports JSON, & YAML output.
We need to support Markdown output as well.

Check #75

Suggested solution:

Headings are names of changes, and then under each heading I guess you get a table of changes? could be nicely handled with IonicaBizau/json2md, so you just first slightly modify the default JSON output that you get as input for Markdown, and then run it through json2md to get markdown. No complicated Markdown templating needed, you only need to add some props to JSON

The format of md output

Subject to change, but my initial thoughts are as follows:

• If user request a breaking, non-breaking or unclassified changes:

Breaking Changes

• If user requests all changes

All changes

If you have other suggestions, drop them below :)