Skip to content

feat: add JSON Schema validation for Flagd provider when in-process mode is used - #373

Merged
askpt merged 12 commits into
open-feature:mainfrom
kylejuliandev:feat/add-flagd-inprocess-json-schema-validation
May 12, 2025
Merged

feat: add JSON Schema validation for Flagd provider when in-process mode is used#373
askpt merged 12 commits into
open-feature:mainfrom
kylejuliandev:feat/add-flagd-inprocess-json-schema-validation

Conversation

@kylejuliandev

@kylejuliandev kylejuliandev commented Apr 27, 2025

Copy link
Copy Markdown
Contributor

This PR

  • Adds NJsonSchema to validate Flagd configurations when in-process mode is used. Schemas used are: Flags and targeting

Related Issues

Fixes #226

Notes

Not yet quite finished. Needs some unit tests. Initial testing locally seems to suggest it is working as expected

image

I'm not sure how keen I am on extending FlagdConfig.cs with ILogger. An alternative approach might be to make it more first class by tweaking the constructor of the FlagdProvider.cs

public FlagdProvider(FlagdConfig config, ILogger logger)
{
    // ... other config setup

    if (logger == null)
    {
        logger = NullLogger.Instance;
    }
    _logger = logger
}

Follow-up Tasks

How to test

@kylejuliandev
Add draft code for validation Flagd config when in-process mode is used
3955dbb 
* Fix unit tests failing due to change in constructor
* Add initial documentation to README
* Add ILogger to provider config

Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@github-actions
github-actions Bot requested review from bacherfl and toddbaert April 27, 2025 12:48
@kylejuliandev
Address dotnet format issues
f86edfe 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@kylejuliandev
Tidy up README for Flagd provider
b4ddf10 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@kylejuliandev
Add initial unit tests for JsonSchemaValidator
11b3854 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@kylejuliandev
Add remaining unit tests for JsonSchemaValidator
c7f7aa5 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@kylejuliandev
Apply dotnet format fixes
3fbcdd8 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@kylejuliandev
kylejuliandev marked this pull request as ready for review April 28, 2025 18:17
@kylejuliandev
kylejuliandev requested review from a team as code owners April 28, 2025 18:17
@kylejuliandev
Merge branch 'main' into feat/add-flagd-inprocess-json-schema-validation
50b796f 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@kylejuliandev
Run dotnet format to address linting issues
c6bb032 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
askpt
askpt reviewed Apr 30, 2025
View reviewed changes

@askpt askpt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor comment in relation to the NJsonSchema version. The rest are minor nitpicks. Great job!

Comment thread src/OpenFeature.Contrib.Providers.Flagd/OpenFeature.Contrib.Providers.Flagd.csproj Outdated
<!-- The generated files will be placed in ./obj/Debug/netstandard2.0/Protos -->
<PackageReference Include="JsonLogic" Version="5.4.0" />
<PackageReference Include="murmurhash" Version="1.0.3" />
<PackageReference Include="NJsonSchema" Version="11.2.0" />

@askpt askpt Apr 30, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you get a lot of breaking changes if you lower to 11.0.0? Since we are publishing this dependency (NJsonSchema), we shouldn't enforce newest versions.

@kylejuliandev kylejuliandev May 1, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have bumped it down to 11.0.0, you're right there is no need to tie Flagd to 11.2 if it works just as well at 11.0

Comment thread src/OpenFeature.Contrib.Providers.Flagd/Resolver/InProcess/JsonSchemaValidator.cs
return;
}

#if NET5_0_OR_GREATER

@askpt askpt Apr 30, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not as part of this PR, but we should bump the target framework...

Comment thread test/OpenFeature.Contrib.Providers.Flagd.Test/JsonEvaluatorTest.cs Outdated
Comment on lines +18 to +20
var jsonSchemaValidator = Substitute.For<IJsonSchemaValidator>();

var jsonEvaluator = new JsonEvaluator(fixture.Create<string>());
var jsonEvaluator = new JsonEvaluator(fixture.Create<string>(), jsonSchemaValidator);

@askpt askpt Apr 30, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of copy-paste here. I think we should move into a ctor, and control the flow here if needs any change.

@kylejuliandev kylejuliandev May 1, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have refactored these tests now to have the constructor initialise the JsonEvaluator. A lot of duplicated code removed now 😄

@kylejuliandev
Pin NJsonSchema to 11.0.0
1ce95fe 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
@kylejuliandev
Reduce repetitiveness in tests by shifting Arrange to constructor
ddd3994 
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
kylejuliandev and others added 2 commits May 2, 2025 20:35
@kylejuliandev
Merge branch 'main' into feat/add-flagd-inprocess-json-schema-validation
0629653
@askpt
Merge branch 'main' into feat/add-flagd-inprocess-json-schema-validation
60dccd9 
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>

# Conflicts:
#	src/OpenFeature.Contrib.Providers.Flagd/Resolver/InProcess/InProcessResolver.cs
@askpt
askpt merged commit 5037f60 into open-feature:main May 12, 2025
@openfeaturebot openfeaturebot mentioned this pull request May 9, 2025
Closed
@kylejuliandev
kylejuliandev deleted the feat/add-flagd-inprocess-json-schema-validation branch May 12, 2025 07:58
weyert pushed a commit to weyert/dotnet-sdk-contrib that referenced this pull request May 30, 2025
@kylejuliandev @askpt @weyert
feat: add JSON Schema validation for Flagd provider when in-process m…
4a19b82 
…ode is used (open-feature#373)

Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
weyert pushed a commit to weyert/dotnet-sdk-contrib that referenced this pull request Jun 19, 2025
@kylejuliandev @askpt
feat: add JSON Schema validation for Flagd provider when in-process m…
89a8322 
…ode is used (open-feature#373)

Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com>
weyert pushed a commit to weyert/dotnet-sdk-contrib that referenced this pull request Jun 19, 2025
@kylejuliandev @askpt @weyert
feat: add JSON Schema validation for Flagd provider when in-process m…
0347592 
…ode is used (open-feature#373)

Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Co-authored-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: Weyert de Boer <weyert@innerfuse.biz>
@openfeaturebot openfeaturebot mentioned this pull request Jul 4, 2025
Merged
This was referenced Dec 18, 2025
Closed
Closed
This was referenced Jan 14, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
This was referenced Jan 21, 2026
Open
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@dependabot dependabot Bot mentioned this pull request Aug 17, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@askpt askpt askpt approved these changes
@bacherfl bacherfl Awaiting requested review from bacherfl
@toddbaert toddbaert Awaiting requested review from toddbaert

Assignees

@bacherfl bacherfl

@toddbaert toddbaert

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[flagd] Add targeting validation and log warning

4 participants

@kylejuliandev @askpt @bacherfl @toddbaert