Merge pull request #10 from lanej/deprecate-path-lookups

Deprecate constant lookups via .const_missing

Gemfile

@@ -1,3 +1,8 @@
 source 'https://rubygems.org'
 
 gemspec
+
+group :test do
+  gem "pry"
+  gem "pry-nav"
+end

README.md

@@ -41,9 +41,6 @@ features
 └── test.yml
 ```
 
-The classes `Feature::Test`, `Feature::Thing::One` and `Feature::Thing::Two` will be available for use within
-your application.
-
 You can call the `Toggles.init` method to force re-parsing the configuration and re-initializing all Features
 structures at any time. The `Toggles.reinit_if_necessary` method is a convenience helper which will only
 re-initialize of the top-level features directory has changed. Note that, in general, this will only detect
@@ -65,13 +62,13 @@ user:
 Check if the feature is enabled or disabled:
 
 ```ruby
-Feature::NewFeature::AvailableForPresentation.enabled_for?(user: OpenStruct.new(id: 12345)) # true
-Feature::NewFeature::AvailableForPresentation.enabled_for?(user: OpenStruct.new(id: 54321)) # true
-Feature::NewFeature::AvailableForPresentation.enabled_for?(user: OpenStruct.new(id: 7)) # false
+Feature.enabled?(:new_feature, :available_for_presentation, user: OpenStruct.new(id: 12345)) # true
+Feature.enabled?(:new_feature, :available_for_presentation, user: OpenStruct.new(id: 54321)) # true
+Feature.enabled?(:new_feature, :available_for_presentation, user: OpenStruct.new(id: 7)) # false
 
-Feature::NewFeature::AvailableForPresentation.disabled_for?(user: OpenStruct.new(id: 12345)) # false
-Feature::NewFeature::AvailableForPresentation.disabled_for?(user: OpenStruct.new(id: 54321)) # false
-Feature::NewFeature::AvailableForPresentation.disabled_for?(user: OpenStruct.new(id: 7)) # true
+Feature.disabled?(:new_feature, :available_for_presentation, user: OpenStruct.new(id: 12345)) # false
+Feature.disabled?(:new_feature, :available_for_presentation, user: OpenStruct.new(id: 54321)) # false
+Feature.disabled?(:new_feature, :available_for_presentation, user: OpenStruct.new(id: 7)) # true
 ```
 
 ## License

features/nested_foo/bar_baz.yml

@@ -0,0 +1,3 @@
+id:
+  lt:
+    5

lib/toggles.rb

@@ -1,5 +1,8 @@
 require "find"
 require "pathname"
+require "set"
+
+require "toggles/version"
 
 require "toggles/configuration"
 require "toggles/feature"
@@ -19,63 +22,25 @@ def configuration
     @configuration ||= Configuration.new
   end
 
-  # Dynamically create modules and classes within the `Feature` module based on
-  # the directory structure of `features`.
-  #
-  # For example if the `features` directory has the structure:
-  #
-  # features
-  # ├── thing
-  # |   ├── one.yml
-  # |   └── two.yml
-  # └── test.yml
-  #
-  # `Feature::Test`, `Feature::Thing::One`, `Feature::Thing::Two` would be
-  # available by default.
-  #
   def init
     return unless Dir.exists? configuration.features_dir
 
-    new_tree = Module.new
-
     top_level = File.realpath(configuration.features_dir)
     top_level_p = Pathname.new(top_level)
 
-    Find.find(top_level) do |path|
-      previous = new_tree
-      abspath = path
-      path = Pathname.new(path).relative_path_from(top_level_p).to_s
-      if path.match(/\.ya?ml\Z/)
-        base = path.chomp(File.extname(path)).split("/")
-        if base.size > 1
-          directories = base[0...-1]
-          filename = base[-1]
-        else
-          directories = []
-          filename = base[0]
-        end
-
-        directories.each do |directory|
-          module_name = directory.split("_").map(&:capitalize).join.to_sym
-          previous    = if previous.constants.include? module_name
-                          previous.const_get(module_name)
-                        else
-                          previous.const_set(module_name, Module.new)
-                        end
-        end
+    Feature.features.clear
 
-        cls = Class.new(Feature::Base) do |c|
-          c.const_set(:PERMISSIONS, Feature::Permissions.new(abspath))
-        end
-
-        previous.const_set(filename.split("_").map(&:capitalize).join.to_sym, cls)
+    Dir[File.join(top_level, "**/*.{yaml,yml}")].each do |abspath|
+      path = Pathname.new(abspath).relative_path_from(top_level_p).to_s
+      features = path.split("/")[0..-2].inject(Feature.features) { |a,e| a[e.to_sym] ||= {} }
+      feature_key = File.basename(path, File.extname(path)).to_sym
+      features[feature_key] = Class.new(Feature::Base) do |c|
+        c.const_set(:PERMISSIONS, Feature::Permissions.new(abspath))
       end
     end
 
     stbuf = File.stat(top_level)
     @stat_tuple = StatResult.new(stbuf.ino, stbuf.mtime)
-
-    Feature.set_tree(new_tree)
   end
 
   def reinit_if_changed

lib/toggles/constant_lookup.rb

@@ -0,0 +1,47 @@
+class Feature::ConstantLookup
+  Error = Class.new(NameError) do
+    attr_reader :sym
+
+    def initialize(sym)
+      @sym = sym
+      super(sym.join('::'))
+    end
+  end
+
+
+  # Return a tree walker that translates Module#const_missing(sym) into the next child node
+  #
+  # So Features::Cat::BearDog walks as:
+  # * next_features = Feature.features # root
+  # * const_missing(:Cat) => next_features = next_features['cat']
+  # * const_missing(:BearDog) => next_features['bear_dog']
+  #
+  # Defined at Toggles.features_dir + "/cat/bear_dog.yaml"
+  #
+  # @raise [Error] if constant cannot be resolved
+  def self.from(features, path)
+    Class.new {
+      class << self
+        attr_accessor :features
+        attr_accessor :path
+
+        def const_missing(sym)
+          subtree_or_feature = features.fetch(
+            # translate class name into path part i.e :BearDog #=> 'bear_dog'
+            sym.to_s.gsub(/([a-z])([A-Z])/) { |s| s.chars.join('_') }.downcase.to_sym,
+          )
+          if subtree_or_feature.is_a?(Hash)
+            Feature::ConstantLookup.from(subtree_or_feature, path + [sym])
+          else
+            subtree_or_feature
+          end
+        rescue KeyError
+          raise Error.new(path + [sym])
+        end
+      end
+    }.tap do |resolver|
+      resolver.features = features
+      resolver.path = path
+    end
+  end
+end

lib/toggles/feature.rb

@@ -12,13 +12,29 @@ module Feature
                 or:    Operation::Or,
                 range: Operation::Range}
 
-  @@tree = Module.new
+  Error = Class.new(StandardError)
+  Unknown = Class.new(Error)
 
-  def self.set_tree(tree)
-    @@tree = tree
+  def self.features
+    @features ||= {}
   end
 
+  # @deprecated This is an abuse of lazy dispatch that creates cryptic errors
   def self.const_missing(sym)
-    @@tree.const_get(sym, inherit: false)
+    ConstantLookup.from(features, [:Feature]).const_missing(sym)
+  end
+
+  def self.enabled?(*sym, **criteria)
+    sym
+      .inject(features) { |a, e| a.fetch(e) }
+      .enabled_for?(criteria)
+  rescue KeyError
+    raise Unknown, sym.inspect
+  end
+
+  def self.disabled?(*sym, **criteria)
+    !enabled?(*sym, **criteria)
   end
 end
+
+require 'toggles/constant_lookup'

lib/toggles/version.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+module Toggles
+  VERSION = '0.3.0'
+end

spec/spec_helper.rb

@@ -3,6 +3,8 @@
 
 require "toggles"
 
+Bundler.require(:test)
+
 RSpec.configure do |config|
   config.order = "random"
 

spec/toggles/feature/acceptance/type_spec.rb

@@ -1,7 +1,23 @@
 describe "Feature::Type" do
+  specify 'deprecated' do
+    aggregate_failures do
+      expect(Feature::Type.enabled_for?(user_id: 1)).to eq true
+      expect(Feature::Type.enabled_for?(user_id: 25)).to eq false
+      expect(Feature::Type.enabled_for?(user_id: nil)).to eq false
+    end
+  end
+
   specify do
-    expect(Feature::Type.enabled_for?(user_id: 1)).to eq true
-    expect(Feature::Type.enabled_for?(user_id: 25)).to eq false
-    expect(Feature::Type.enabled_for?(user_id: nil)).to eq false
+    aggregate_failures do
+      expect(Feature.enabled?(:type, user_id: 1)).to eq(true)
+      expect(Feature.disabled?(:type, user_id: 1)).to eq(false)
+      expect(Feature.enabled?(:type, user_id: 25)).to eq(false)
+      expect(Feature.enabled?(:nested_foo, :bar_baz, id: 25)).to eq(false)
+      expect(Feature.enabled?(:nested_foo, :bar_baz, id: 1)).to eq(true)
+      expect(Feature.disabled?(:type, user_id: 25)).to eq(true)
+      expect(Feature.disabled?(:nested_foo, :bar_baz, id: 25)).to eq(true)
+      expect(Feature.disabled?(:nested_foo, :bar_baz, id: 1)).to eq(false)
+      expect { Feature.disabled?(:nested_foo, :bar_boz, id: 1) }.to raise_error(Feature::Unknown)
+    end
   end
 end

spec/toggles/init_spec.rb

@@ -17,7 +17,7 @@
       Toggles.configure do |c|
         c.features_dir = temp_dir
       end
-      
+
       expect(Feature::Foo::Users.enabled_for?(id: 1)).to eq(true)
       expect(Feature::Bar::Users.enabled_for?(id: 3)).to eq(true)
     end
@@ -36,7 +36,7 @@
       Toggles.configure do |c|
         c.features_dir = temp_dir
       end
-      
+
       expect(Feature::Foo::Users.enabled_for?(id: 1)).to eq(true)
       expect(Feature::Foo::Children.enabled_for?(id: 1)).to eq(true)
 
@@ -49,7 +49,10 @@
       Toggles.init
 
       expect(Feature::Foo::Users.enabled_for?(id: 1)).to eq(false)
-      expect { Feature::Foo::Children.enabled_for?(id: 1) }.to raise_error(NameError)
+      expect { Feature::Bar::Children.enabled_for?(id: 1) }
+        .to raise_error(Feature::ConstantLookup::Error, 'Feature::Bar')
+      expect { Feature::Foo::Children.enabled_for?(id: 1) }
+        .to raise_error(Feature::ConstantLookup::Error, 'Feature::Foo::Children')
     end
   end
 

toggles.gemspec

@@ -1,26 +1,29 @@
+# frozen_string_literal: true
+
+lib = File.expand_path('lib', __dir__)
+$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
+require 'toggles/version'
+
 Gem::Specification.new do |s|
-  s.name = "toggles"
-  s.version = "0.1.2"
-  s.authors = ["Andrew Tribone", "James Brown"]
-  s.summary = "YAML backed feature toggles"
-  s.email = "[email protected]"
-  s.homepage = "https://github.com/EasyPost/toggles"
-  s.license = "ISC"
+  s.name = 'toggles'
+  s.version = Toggles::VERSION
+  s.authors = ['Andrew Tribone', 'James Brown', 'Josh Lane']
+  s.summary = 'YAML backed feature toggles'
+  s.email = '[email protected]'
+  s.homepage = 'https://github.com/EasyPost/toggles'
+  s.license = 'ISC'
   s.files = `git ls-files`.split($INPUT_RECORD_SEPARATOR)
-  s.test_files = s.files.grep(/^(spec)\//)
+  s.test_files = s.files.grep(%r{^(spec)/})
   s.description = <<-EOF
     YAML-backed implementation of the feature flags pattern. Build a
     hierarchy of features in YAML files in the filesystem, apply various
     conditions using boolean logic and a selection of filters, and easily
     check whether a given feature should be applied.
   EOF
 
-  s.add_development_dependency "bundler"
-  s.add_development_dependency "pry"
-  s.add_development_dependency "pry-nav"
-  s.add_development_dependency "pry-remote"
-  s.add_development_dependency "rake"
-  s.add_development_dependency "rspec"
-  s.add_development_dependency "rspec-its"
-  s.add_development_dependency "rspec-temp_dir"
+  s.add_development_dependency 'bundler'
+  s.add_development_dependency 'rake'
+  s.add_development_dependency 'rspec'
+  s.add_development_dependency 'rspec-its'
+  s.add_development_dependency 'rspec-temp_dir'
 end