Fix change detection for databags (#134)

Summary:
For changes detection we don't care if file was added or modified, we only need to understand
if file was modified or deleted. So using same logic as in `BetweenMeals::Changes::Role`

Also extending tests to support Mercurial statuses.
And enforcing allowed statuses for changes in setter.

This would fix an issue with Grocery Delivery uploads when adding new databag.

Signed-off-by: Alexey Savva <alexeys@fb.com>

lib/between_meals/changes/change.rb

@@ -24,13 +24,26 @@ module BetweenMeals
   module Changes
     # Common functionality
     class Change
+      # Since we either need to upload or delete, we only accept two statuses.
+      # VCSs will differentiate between various kinds of modifies, adds, etc.
+      # so instead of handling all possibilities here, we expect the caller to
+      # collapse them into `:modified` or `:deleted`.
+      ALLOWED_STATUSES = [:modified, :deleted].freeze
       @@logger = nil
-      attr_accessor :name, :status
+      attr_accessor :name
+      attr_reader :status
 
       def to_s
         @name
       end
 
+      def status=(value)
+        unless ALLOWED_STATUSES.include?(value)
+          fail "#{self.class} status attribute can only be one of #{ALLOWED_STATUSES} not #{value}"
+        end
+        @status = value
+      end
+
       # People who use us through find() can just pass in logger,
       # for everyone else, here's a setter
       def logger=(log)

lib/between_meals/changes/cookbook.rb

@@ -119,9 +119,9 @@ def initialize(files, cookbook_dirs)
                %{^(#{cookbook_dirs.join('|')})/[^/]+/metadata\.(rb|json)$},
              )
            end.none?
-          @status = :deleted
+          self.status = :deleted
         else
-          @status = :modified
+          self.status = :modified
         end
       end
 

lib/between_meals/changes/databag.rb

@@ -32,8 +32,8 @@ def self.name_from_path(path, databag_dir)
       end
 
       def initialize(file, databag_dir)
-        @status = file[:status]
-        @name, @item = self.class.name_from_path(file[:path], databag_dir)
+        self.status = file[:status] == :deleted ? :deleted : :modified
+        self.name, self.item = self.class.name_from_path(file[:path], databag_dir)
       end
 
       def self.find(list, databag_dir, logger)

lib/between_meals/changes/role.rb

@@ -30,8 +30,8 @@ def self.name_from_path(path, role_dir)
       end
 
       def initialize(file, role_dir)
-        @status = file[:status] == :deleted ? :deleted : :modified
-        @name = self.class.name_from_path(file[:path], role_dir)
+        self.status = file[:status] == :deleted ? :deleted : :modified
+        self.name = self.class.name_from_path(file[:path], role_dir)
       end
 
       # Given a list of changed files

spec/cookbook_spec.rb

@@ -161,6 +161,18 @@ def repo_path
         ['cb_one', :modified],
       ],
     },
+    {
+      :name => 'adding recipe',
+      :files => [
+        {
+          :status => :added,
+          :path => 'cookbooks/one/cb_one/recipe/extra.rb',
+        },
+      ],
+      :result => [
+        ['cb_one', :modified],
+      ],
+    },
     {
       :name => 'skipping non-cookbook files',
       :files => [

spec/databag_spec.rb

@@ -68,9 +68,13 @@
           :status => :deleted,
           :path => 'databags/two/databag3.json',
         },
+        {
+          :status => :added,
+          :path => 'databags/three/databag1.json',
+        },
       ],
       :result => [
-        ['one', :modified], ['two', :deleted]
+        ['one', :modified], ['two', :deleted], ['three', :modified]
       ],
     },
   ]

spec/role_spec.rb

@@ -76,13 +76,18 @@
           :status => :modified,
           :path => 'roles/test.rb',
         },
+        {
+          :status => :added,
+          :path => 'roles/test2.rb',
+        },
         {
           :status => :modified,
           :path => 'cookbooks/one/cb_one/recipes/test3.rb',
         },
       ],
       :result => [
         ['test', :modified],
+        ['test2', :modified],
       ],
     },
     {