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

Share flag_value JsonNode extraction logic through each extended class #8

Merged
merged 2 commits into from
May 6, 2024

Conversation

lavenderses
Copy link
Owner

What

Use shared flag_value getting method through all extenion parser for each feature flag type.

Why

Thanks to #7, parser logic is separeted to each feature flag type.
But, this provider's expected JSON schema from AWS AppConfig is same (following schema, see also AppConfigValue).

So, the logic which get flag_value value as JsonNode should be same, but now it is separeted.

{
  // feature flag key name in OpenFeature world
  "{key name}": {
    // is this feature flag enabled in AWS AppConfig world
    "enabled": boolean,
    // the actual feature flag value in OpenFeature world and this provider implementation world
    "flag_value": number | string | boolean | regex
  }
}

How

Move extraction logic to AbstractAttributeParser#getValidFlagValueNode. AbstractAttributeParser is the abstract implementation for each feature flag type.
Extended parser class use this to get flag_value value as JsonNode.

Only Object type feature flag does not have expected JsonNode type, so this null check is introduced.
https://github.com/lavenderses/AWSAppConfig-openfeature-provider-java/blob/54c934a29737cfc00fb5be9fa24fcf3ab63715a0/src/main/java/io/github/lavenderses/aws_app_config_openfeature_provider/parser/AbstractAttributeParser.java#L98

Notes

N/A

@lavenderses lavenderses added the refactor Refactor something, without any spec change label May 6, 2024
@lavenderses lavenderses self-assigned this May 6, 2024
Copy link
Owner Author

@lavenderses lavenderses left a comment

Choose a reason for hiding this comment

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

LGTM

@lavenderses lavenderses merged commit eb112bb into main May 6, 2024
4 checks passed
@lavenderses lavenderses deleted the lavenderses/issue-5-3 branch May 6, 2024 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor something, without any spec change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant