Repository navigation
feat: Add file loading code for flag overrides #449
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
Draft
kinyoklion
wants to merge
11
commits into
feat/overrides
Choose a base branch
from
rlamb/overrides-ruby-filedata
base: feat/overrides
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
72d5776
feat: Add file loading code for flag overrides
kinyoklion 821186b
fix: Expand value-only overrides to a flag served by fallthrough
kinyoklion 4546f9b
fix: Report an exception from apply as a failed reload
kinyoklion 4f0af39
fix: Watch a data file directory again after it is deleted
kinyoklion 9b7d0d0
fix: Report an entry that cannot be deserialized as a merge error
kinyoklion 753eed3
test: Make the file data specs portable across platforms and runtimes
kinyoklion 6c688e2
test: Run the deleted directory example only with inotify
kinyoklion 6f3e67e
fix: Start the watcher retry again when a directory is lost while the…
kinyoklion 2e31db3
test: Cover the deduplication of a repeated rejection by apply
kinyoklion ba5b655
test: Wait for the legacy V2 source's change detection without Queue#…
kinyoklion 73afbaa
fix: Watch each data file directory independently
kinyoklion File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "ldclient-rb/impl/file_data/document" | ||
| require "ldclient-rb/impl/file_data/merge" | ||
| require "ldclient-rb/impl/file_data/poller" | ||
| require "ldclient-rb/impl/file_data/reloader" | ||
| require "ldclient-rb/impl/file_data/watcher" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,188 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "ldclient-rb/impl/model/serialization" | ||
|
|
||
| require "yaml" | ||
|
|
||
| module LaunchDarkly | ||
| module Impl | ||
| # | ||
| # Shared file reading, parsing, and merging code for the components that load flag and | ||
| # segment data from local files. | ||
| # | ||
| # @private | ||
| # | ||
| module FileData | ||
| # | ||
| # Raised when a file cannot be read or parsed. It carries the path so that callers can | ||
| # tell a per-file failure from a failure to merge the files' contents. | ||
| # | ||
| class ReadError < StandardError | ||
| # @return [String] | ||
| attr_reader :path | ||
|
|
||
| # @return [Boolean] true when the file does not exist | ||
| attr_reader :missing | ||
|
|
||
| # | ||
| # @param path [String] | ||
| # @param message [String] | ||
| # @param missing [Boolean] | ||
| # | ||
| def initialize(path, message, missing: false) | ||
| super("#{message} [#{path}]") | ||
| @path = path | ||
| @missing = missing | ||
| end | ||
| end | ||
|
|
||
| # | ||
| # The parsed form of one data file. A document may contain full flag definitions, flag key | ||
| # to value entries, and segment definitions. Every hash has symbol keys. | ||
| # | ||
| class Document | ||
| # @return [Hash{Symbol => Hash}] | ||
| attr_reader :flags | ||
|
|
||
| # @return [Hash{Symbol => Object}] | ||
| attr_reader :flag_values | ||
|
|
||
| # @return [Hash{Symbol => Hash}] | ||
| attr_reader :segments | ||
|
|
||
| # | ||
| # @param flags [Hash{Symbol => Hash}] | ||
| # @param flag_values [Hash{Symbol => Object}] | ||
| # @param segments [Hash{Symbol => Hash}] | ||
| # | ||
| def initialize(flags: {}, flag_values: {}, segments: {}) | ||
| @flags = flags | ||
| @flag_values = flag_values | ||
| @segments = segments | ||
| end | ||
|
|
||
| # | ||
| # Parses the content of a data file. The content may be JSON or YAML. JSON is a subset of | ||
| # YAML, and the Ruby YAML parser handles it, so one parser serves both formats. | ||
| # | ||
| # An empty document is a document with no entries. A document that is not a mapping, or | ||
| # whose "flags", "flagValues", or "segments" member is not a mapping, is an error. | ||
| # | ||
| # @param content [String] | ||
| # @return [Document] | ||
| # @raise [ArgumentError] if the content is not a valid document | ||
| # @raise [Psych::SyntaxError] if the content cannot be parsed | ||
| # | ||
| def self.parse(content) | ||
| raw = YAML.safe_load(content) | ||
| raw = {} if raw.nil? | ||
| raise ArgumentError, "file content must be an object" unless raw.is_a?(Hash) | ||
|
|
||
| data = FileData.symbolize_keys(raw) | ||
| Document.new( | ||
| flags: section(data, :flags), | ||
| flag_values: section(data, :flagValues), | ||
| segments: section(data, :segments) | ||
| ) | ||
| end | ||
|
|
||
| # | ||
| # Reads and parses one data file. | ||
| # | ||
| # @param path [String] | ||
| # @return [Document] | ||
| # @raise [ReadError] if the file cannot be read or parsed | ||
| # | ||
| def self.read(path) | ||
| content = FileData.read_file(path) | ||
| FileData.parse_file(path, content) | ||
| end | ||
|
|
||
| private_class_method def self.section(data, name) | ||
| value = data[name] | ||
| return {} if value.nil? | ||
| raise ArgumentError, "\"#{name}\" must be an object" unless value.is_a?(Hash) | ||
|
|
||
| value | ||
| end | ||
| end | ||
|
|
||
| # | ||
| # Reads the raw content of one file. | ||
| # | ||
| # @param path [String] | ||
| # @return [String] | ||
| # @raise [ReadError] if the file cannot be read | ||
| # | ||
| def self.read_file(path) | ||
| File.read(path) | ||
| rescue Errno::ENOENT => e | ||
| raise ReadError.new(path, "unable to read file: #{e.message}", missing: true) | ||
| rescue SystemCallError, IOError => e | ||
| raise ReadError.new(path, "unable to read file: #{e.message}") | ||
| end | ||
|
|
||
| # | ||
| # Parses raw content that was read from the given path. | ||
| # | ||
| # @param path [String] | ||
| # @param content [String] | ||
| # @return [Document] | ||
| # @raise [ReadError] if the content cannot be parsed | ||
| # | ||
| def self.parse_file(path, content) | ||
| Document.parse(content) | ||
| rescue StandardError => e | ||
| raise ReadError.new(path, "error parsing file: #{e.message}") | ||
| end | ||
|
|
||
| # | ||
| # Recursively converts hash keys to symbols. The SDK expects all data model objects to | ||
| # have symbol keys. | ||
| # | ||
| # @param value [Object] | ||
| # @return [Object] | ||
| # | ||
| def self.symbolize_keys(value) | ||
| case value | ||
| when Hash | ||
| value.to_h { |k, v| [k.to_s.to_sym, symbolize_keys(v)] } | ||
| when Array | ||
| value.map { |v| symbolize_keys(v) } | ||
| else | ||
| value | ||
| end | ||
| end | ||
|
|
||
| # | ||
| # Expands a flag key to value entry into a full flag definition that returns the given | ||
| # value for every context. The flag is on, has the value as its only variation, and | ||
| # serves that variation as its fallthrough, so an evaluation reports the FALLTHROUGH | ||
| # reason kind. | ||
| # | ||
| # @param key [String] | ||
| # @param value [Object] | ||
| # @return [Hash] | ||
| # | ||
| def self.make_flag_with_value(key, value) | ||
| { | ||
| key: key, | ||
| on: true, | ||
| version: 1, | ||
| fallthrough: { variation: 0 }, | ||
| variations: [value], | ||
| } | ||
| end | ||
|
|
||
| # | ||
| # Converts each path to an absolute path. | ||
| # | ||
| # @param paths [Array<String>, String] | ||
| # @return [Array<String>] | ||
| # | ||
| def self.absolute_paths(paths) | ||
| Array(paths).map { |p| File.absolute_path(p.to_s) } | ||
| end | ||
| end | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require "ldclient-rb/impl/data_store" | ||
| require "ldclient-rb/impl/file_data/document" | ||
| require "ldclient-rb/impl/model/serialization" | ||
|
|
||
| module LaunchDarkly | ||
| module Impl | ||
| module FileData | ||
| # | ||
| # Values for the duplicate keys handling option. They select what happens when the same | ||
| # flag or segment key appears in more than one document. | ||
| # | ||
| module DuplicateKeysHandling | ||
| # A duplicated key makes the merge fail. | ||
| FAIL = :fail | ||
|
|
||
| # Only the first occurrence of a duplicated key is kept, in the order the documents were given. | ||
| IGNORE = :ignore | ||
|
|
||
| ALL = [FAIL, IGNORE].freeze | ||
| end | ||
|
|
||
| # | ||
| # Raised when documents cannot be combined, for example because a key is duplicated, an | ||
| # entry is not an object, or an entry cannot be deserialized into the data model. | ||
| # | ||
| class MergeError < StandardError | ||
| end | ||
|
|
||
| # | ||
| # Counts the entries the merge kept from one document. | ||
| # | ||
| DocumentSummary = Struct.new(:flags, :segments) | ||
|
|
||
| # | ||
| # Describes one configured file after a reload. `present` is false when the file does not | ||
| # exist and missing files are skipped. | ||
| # | ||
| FileSummary = Struct.new(:path, :present, :flags, :segments) | ||
|
|
||
| # | ||
| # The merged items from one or more documents. | ||
| # | ||
| class MergeResult | ||
| # @return [Hash{Symbol => LaunchDarkly::Impl::Model::FeatureFlag}] | ||
| attr_reader :flags | ||
|
|
||
| # @return [Hash{Symbol => LaunchDarkly::Impl::Model::Segment}] | ||
| attr_reader :segments | ||
|
|
||
| # @return [Array<DocumentSummary>] one entry per input document, in order | ||
| attr_reader :documents | ||
|
|
||
| # @return [Array<FileSummary>] set by the Reloader, one entry per configured file, in order | ||
| attr_accessor :files | ||
|
|
||
| def initialize(flags, segments, documents) | ||
| @flags = flags | ||
| @segments = segments | ||
| @documents = documents | ||
| @files = [] | ||
| end | ||
|
|
||
| # @return [Boolean] | ||
| def empty? | ||
| @flags.empty? && @segments.empty? | ||
| end | ||
| end | ||
|
|
||
| # | ||
| # Combines the items of the given documents into one set of flags and one set of segments. | ||
| # Flag key to value entries expand into full flag definitions. Entries are deserialized into | ||
| # the SDK's data model classes, which validate them. The documents are processed in order, | ||
| # and the configured duplicate keys handling applies when the same key appears more than once. | ||
| # | ||
| # Items are keyed by the key under which they appear in the document. An entry that has no | ||
| # "key" member receives that key. A missing "version" defaults to 1. | ||
| # | ||
| # @param documents [Array<Document>] | ||
| # @param duplicate_keys_handling [Symbol] one of the {DuplicateKeysHandling} values | ||
| # @param logger [Logger, nil] receives data model validation messages | ||
| # @return [MergeResult] | ||
| # @raise [MergeError] if the documents cannot be combined | ||
| # | ||
| def self.merge(documents, duplicate_keys_handling: DuplicateKeysHandling::FAIL, logger: nil) | ||
| flags = {} | ||
| segments = {} | ||
| summaries = [] | ||
|
|
||
| documents.each do |document| | ||
| summary = DocumentSummary.new(0, 0) | ||
|
|
||
| document.flags.each do |key, data| | ||
| data = prepare_entry("flag", key, data) | ||
| item = deserialize(DataStore::FEATURES, "flag", key, data, logger) | ||
| summary.flags += 1 if insert(flags, "flag", key, item, duplicate_keys_handling) | ||
| end | ||
|
|
||
| document.flag_values.each do |key, value| | ||
| data = make_flag_with_value(key.to_s, value) | ||
| item = Model.deserialize(DataStore::FEATURES, data, logger) | ||
| summary.flags += 1 if insert(flags, "flag", key, item, duplicate_keys_handling) | ||
| end | ||
|
|
||
| document.segments.each do |key, data| | ||
| data = prepare_entry("segment", key, data) | ||
| item = deserialize(DataStore::SEGMENTS, "segment", key, data, logger) | ||
| summary.segments += 1 if insert(segments, "segment", key, item, duplicate_keys_handling) | ||
| end | ||
|
|
||
| summaries << summary | ||
| end | ||
|
|
||
| MergeResult.new(flags, segments, summaries) | ||
| end | ||
|
|
||
| # | ||
| # Validates one full flag or segment entry and fills in its key and version. | ||
| # | ||
| private_class_method def self.prepare_entry(category, key, data) | ||
| raise MergeError, "#{category} \"#{key}\" is not an object" unless data.is_a?(Hash) | ||
|
|
||
| data = data.dup | ||
| data[:key] = key.to_s if data[:key].nil? | ||
| data[:version] = 1 if data[:version].nil? | ||
| data | ||
| end | ||
|
|
||
| # | ||
| # Deserializes one entry into its data model class. The model classes raise for a value | ||
| # that does not fit the schema, and that failure is reported as a merge error that names | ||
| # the entry. | ||
| # | ||
| private_class_method def self.deserialize(kind, category, key, data, logger) | ||
| Model.deserialize(kind, data, logger) | ||
| rescue => e | ||
| raise MergeError, "#{category} \"#{key}\" is not valid: #{e.message}" | ||
| end | ||
|
|
||
| # | ||
| # Adds an item unless its key was already seen. Returns true when it added the item. | ||
| # | ||
| private_class_method def self.insert(items, category, key, item, duplicate_keys_handling) | ||
| key = key.to_sym | ||
| if items.key?(key) | ||
| return false if duplicate_keys_handling == DuplicateKeysHandling::IGNORE | ||
|
|
||
| raise MergeError, "#{category} key \"#{key}\" was used more than once" | ||
| end | ||
|
|
||
| items[key] = item | ||
| true | ||
| end | ||
| end | ||
| end | ||
| end |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YAML on key silently disables flags
Medium Severity
YAML.safe_loadtreats unquotedonas a boolean, andsymbolize_keysthen stores that field as:true. Full flag definitions never see:on, soFeatureFlagtreats them as off. Natural YAML likeon: truetherefore loads as a disabled flag instead of failing or preserving theonfield.Additional Locations (1)
lib/ldclient-rb/impl/file_data/document.rb#L75-L87Reviewed by Cursor Bugbot for commit 2e31db3. Configure here.