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

Targeting sub-schema doesn't work alone #165

Closed
toddbaert opened this issue Jun 14, 2024 · 2 comments · Fixed by #168
Closed

Targeting sub-schema doesn't work alone #165

toddbaert opened this issue Jun 14, 2024 · 2 comments · Fixed by #168
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@toddbaert
Copy link
Member

The targeting sub-schema doesn't work if used by itself. It's entirely correct and works well if used as part of the parent schema, but doesn't work in isolation.

This seems to be because we only DEFINE "targeting" in this schema, but don't use it to validate the top-level object.

We need to:

  • use the existing definition to validate the top-level object
  • add 1 positive and 1 negative test to make sure the schema works as expected
    • we already have lots of test coverage here where we iterate over all the JSON in the positive and negative dirs as part of the parent, we just need this new single positive/negative test to load the targeting schema in isolation and make sure it works alone as well. This means a new test which loads just the targeting schema instead of the definition schema, similar to this line:
      compiledSchema, err = schemaLoader.Compile(gojsonschema.NewStringLoader(flagd_definitions.FlagSchema))
@toddbaert toddbaert added bug Something isn't working good first issue Good for newcomers labels Jun 14, 2024
@toddbaert
Copy link
Member Author

cc @aepfli

@aepfli
Copy link
Member

aepfli commented Jun 14, 2024

Thank you for creating the bug - below i added a small spoiler for the changes to the targeting.json ;)

behind this is a spoiler for the solution

At line 6 of the targeting.json we need to add an

 "type": "object",
 "anyOf": [
   {
     "$ref": "#/definitions/targeting"
   }
 ],

https://github.com/open-feature/flagd-schemas/blob/main/json/targeting.json#L6

disclaimer: maybe a allOf would be even enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants