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

Improve validation for matrix selectors #641

Open
bethesque opened this issue Nov 7, 2023 · 1 comment
Open

Improve validation for matrix selectors #641

bethesque opened this issue Nov 7, 2023 · 1 comment

Comments

@bethesque
Copy link
Member

This query should have returned a 400

{
  "q": [
    {
      "pacticipant": "X",
      "version": "123",
      "environment": "B"
    }
  ]
}

TODO: Work out if this is an error or a valid query - it causes an SQL error.

{
  "q": [
    {
      "pacticipant": "x",
      "version": "134234"
    },
    {
      "pacticipant": "x",
      "latest": "true",
      "environment": "B"
    }
  ]
}

This is a placeholder issue - it needs the rules written out properly before it can be picked up.

@bethesque
Copy link
Member Author

Low priority.

Almost all matrix queries come via the can-i-deploy CLI, which will protect against most invalid queries. Full validation is generally only relevant if someone hand crafts a matrix query, which we occasionally see via the Postman interface when someone is playing around with the API. I believe this particular error (selectors with duplicate pacticipant names) may be able to be produced via the CLI by inputting two selectors with the same pacticipant name, but I have not checked this.

A quick fix for this particular error would be to update the validate_selectors method in the PactBroker::Matrix::Service class to ensure that there were no selectors with duplicate pacticipant names.

If this was to be implemented properly, the validate_selectors code should be moved into a Dry Validation contract class, and the relevant business rules regarding what fields were supported together should be implemented using the same pattern as ConsumerVersionSelectorContract.

I think it's probably not worth it, for the number of times a year that it happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant