Mutable drops should fallback to their own methods when a mutation isn't present (#6350)

Merge pull request 6350

lib/jekyll/drops/drop.rb

@@ -104,11 +104,9 @@ module Jekyll
       #
       # Returns true if the given key is present
       def key?(key)
-        if self.class.mutable?
+        return false if key.nil?
-          @mutations.key?(key)
+        return true if self.class.mutable? && @mutations.key?(key)
-        else
+        respond_to?(key) || fallback_data.key?(key)
-          !key.nil? && (respond_to?(key) || fallback_data.key?(key))
-        end
       end
 
       # Generates a list of keys with user content as their values.

test/test_drop.rb

@@ -10,12 +10,12 @@ class DropFixture < Jekyll::Drops::Drop
   end
 
   def fallback_data
-    @fallback_data ||= {}
+    @fallback_data ||= { "baz" => "buzz" }
   end
 end
 
 class TestDrop < JekyllUnitTest
-  context "a document drop" do
+  context "Drops" do
     setup do
       @site = fixture_site({
         "collections" => ["methods"],
@@ -52,38 +52,67 @@ class TestDrop < JekyllUnitTest
       end
     end
 
-    context "fetch" do
+    context "a document drop" do
-      should "raise KeyError if key is not found and no default provided" do
+      context "fetch" do
-        assert_raises KeyError do
+        should "raise KeyError if key is not found and no default provided" do
-          @document_drop.fetch("not_existing_key")
+          assert_raises KeyError do
+            @document_drop.fetch("not_existing_key")
+          end
+        end
+
+        should "fetch value without default" do
+          assert_equal "Jekyll.configuration", @document_drop.fetch("title")
+        end
+
+        should "fetch default if key is not found" do
+          assert_equal "default", @document_drop.fetch("not_existing_key", "default")
+        end
+
+        should "fetch default boolean value correctly" do
+          assert_equal false, @document_drop.fetch("bar", false)
+        end
+
+        should "fetch default value from block if key is not found" do
+          assert_equal "default bar", @document_drop.fetch("bar") { |el| "default #{el}" }
+        end
+
+        should "fetch default value from block first if both argument and block given" do
+          assert_equal "baz", @document_drop.fetch("bar", "default") { "baz" }
+        end
+
+        should "not change mutability when fetching" do
+          assert @drop.class.mutable?
+          @drop["foo"] = "baz"
+          assert_equal "baz", @drop.fetch("foo")
+          assert @drop.class.mutable?
+        end
+      end
+    end
+
+    context "key?" do
+      context "a mutable drop" do
+        should "respond true for native methods" do
+          assert @drop.key? "foo"
+        end
+
+        should "respond true for mutable keys" do
+          @drop["bar"] = "baz"
+          assert @drop.key? "bar"
+        end
+
+        should "return true for fallback data" do
+          assert @drop.key? "baz"
         end
       end
 
-      should "fetch value without default" do
+      context "a document drop" do
-        assert_equal "Jekyll.configuration", @document_drop.fetch("title")
+        should "respond true for native methods" do
-      end
+          assert @document_drop.key? "collection"
+        end
 
-      should "fetch default if key is not found" do
+        should "return true for fallback data" do
-        assert_equal "default", @document_drop.fetch("not_existing_key", "default")
+          assert @document_drop.key? "title"
-      end
+        end
-
-      should "fetch default boolean value correctly" do
-        assert_equal false, @document_drop.fetch("bar", false)
-      end
-
-      should "fetch default value from block if key is not found" do
-        assert_equal "default bar", @document_drop.fetch("bar") { |el| "default #{el}" }
-      end
-
-      should "fetch default value from block first if both argument and block given" do
-        assert_equal "baz", @document_drop.fetch("bar", "default") { "baz" }
-      end
-
-      should "not change mutability when fetching" do
-        assert @drop.class.mutable?
-        @drop["foo"] = "baz"
-        assert_equal "baz", @drop.fetch("foo")
-        assert @drop.class.mutable?
       end
     end
   end