Refactor message metadata tests

Prior to this commit, there were several places to potentially add a new
message metadata test:

* `SharedMessageMetadataTests` module
* `MessageEncryptorMetadataTest` class
  (includes `SharedMessageMetadataTests`)
* `MessageEncryptorMetadataMarshalTest` class
  (subclasses `MessageEncryptorMetadataTest`)
* `MessageEncryptorMetadataJSONTest` class
  (subclasses `MessageEncryptorMetadataTest`)
* `MessageEncryptorMetadataJsonWithMarshalFallbackTest` class
  (subclasses `MessageEncryptorMetadataTest`)
* `MessageVerifierMetadataTest` class
  (includes `SharedMessageMetadataTests`)
* `MessageVerifierMetadataMarshalTest` class
  (subclasses `MessageVerifierMetadataTest`)
* `MessageVerifierMetadataJsonWithMarshalFallbackTest` class
  (subclasses `MessageVerifierMetadataTest`)
* `MessageVerifierMetadataJsonTest` class
  (subclasses `MessageVerifierMetadataTest`)
* `MessageVerifierMetadataCustomJSONTest` class
  (subclasses `MessageVerifierMetadataTest`)
* `MessageEncryptorMetadataNullSerializerTest` class
  (subclasses `MessageVerifierMetadataTest`)

This commit refactors the message metadata tests, reducing the list to:

* `MessageMetadataTests` module
* `MessageEncryptorMetadataTest` class (includes `MessageMetadataTests`)
* `MessageVerifierMetadataTest` class (includes `MessageMetadataTests`)

This makes it easier to add new tests, as well as new testing scenarios
(e.g. new encryptor / verifier configurations).

Additionally, this commit fixes two tests that were ineffective:

* The `test_passing_expires_in_less_than_a_second_is_not_expired` test
  (which is now simply a part of the "message expires with :expires_in"
  test) did not use the `with_usec: true` option.  Therefore, the time
  resulting from `travel 0.5.seconds` was truncated, effectively
  traveling 0 seconds.
* The `test_verify_with_use_standard_json_time_format_as_false` test
  (which is now named "expiration works with
  ActiveSupport.use_standard_json_time_format = false") did not specify
  an expiration time.  Therefore, the conditional branch that it was
  supposed to test was never exercised.

Author: jonathanhefner

activesupport/test/message_encryptor_test.rb

@@ -4,7 +4,6 @@
 require "openssl"
 require "active_support/time"
 require "active_support/json"
-require_relative "metadata/shared_metadata_tests"
 
 class MessageEncryptorTest < ActiveSupport::TestCase
   class JSONSerializer
@@ -404,44 +403,3 @@ def teardown
     super
   end
 end
-
-class MessageEncryptorMetadataTest < ActiveSupport::TestCase
-  include SharedMessageMetadataTests
-
-  setup do
-    @secret    = SecureRandom.random_bytes(32)
-    @encryptor = ActiveSupport::MessageEncryptor.new(@secret, **encryptor_options)
-  end
-
-  private
-    def generate(message, **options)
-      @encryptor.encrypt_and_sign(message, **options)
-    end
-
-    def parse(data, **options)
-      @encryptor.decrypt_and_verify(data, **options)
-    end
-
-    def encryptor_options; {} end
-end
-
-class MessageEncryptorMetadataMarshalTest < MessageEncryptorMetadataTest
-  private
-    def encryptor_options
-      { serializer: Marshal }
-    end
-end
-
-class MessageEncryptorMetadataJSONTest < MessageEncryptorMetadataTest
-  private
-    def encryptor_options
-      { serializer: MessageEncryptorTest::JSONSerializer.new }
-    end
-end
-
-class MessageEncryptorMetadataJsonWithMarshalFallbackTest < MessageEncryptorMetadataTest
-  private
-    def encryptor_options
-      { serializer: ActiveSupport::JsonWithMarshalFallback }
-    end
-end

activesupport/test/message_verifier_test.rb

@@ -4,7 +4,6 @@
 require "openssl"
 require "active_support/time"
 require "active_support/json"
-require_relative "metadata/shared_metadata_tests"
 
 class MessageVerifierTest < ActiveSupport::TestCase
   class JSONSerializer
@@ -289,91 +288,3 @@ def teardown
     ActiveSupport::MessageVerifier.default_message_verifier_serializer = @default_verifier
   end
 end
-
-class MessageVerifierMetadataTest < ActiveSupport::TestCase
-  include SharedMessageMetadataTests
-
-  setup do
-    @verifier = ActiveSupport::MessageVerifier.new("Hey, I'm a secret!", **verifier_options)
-  end
-
-  def test_verify_raises_when_purpose_differs
-    assert_raise(ActiveSupport::MessageVerifier::InvalidSignature) do
-      @verifier.verify(generate(data, purpose: "payment"), purpose: "shipping")
-    end
-  end
-
-  def test_verify_with_use_standard_json_time_format_as_false
-    format_before = ActiveSupport.use_standard_json_time_format
-    ActiveSupport.use_standard_json_time_format = false
-    assert_equal "My Name", @verifier.verify(generate("My Name"))
-  ensure
-    ActiveSupport.use_standard_json_time_format = format_before
-  end
-
-  def test_verify_raises_when_expired
-    signed_message = generate(data, expires_in: 1.month)
-
-    travel 2.months
-    assert_raise(ActiveSupport::MessageVerifier::InvalidSignature) do
-      @verifier.verify(signed_message)
-    end
-  end
-
-  private
-    def generate(message, **options)
-      @verifier.generate(message, **options)
-    end
-
-    def parse(message, **options)
-      @verifier.verified(message, **options)
-    end
-
-    def verifier_options
-      Hash.new
-    end
-end
-
-class MessageVerifierMetadataMarshalTest < MessageVerifierMetadataTest
-  private
-    def verifier_options
-      { serializer: Marshal }
-    end
-end
-
-class MessageVerifierMetadataJsonWithMarshalFallbackTest < MessageVerifierMetadataTest
-  private
-    def verifier_options
-      { serializer: ActiveSupport::JsonWithMarshalFallback }
-    end
-end
-
-class MessageVerifierMetadataJsonTest < MessageVerifierMetadataTest
-  private
-    def verifier_options
-      { serializer: JSON }
-    end
-end
-
-
-class MessageVerifierMetadataCustomJSONTest < MessageVerifierMetadataTest
-  private
-    def verifier_options
-      { serializer: MessageVerifierTest::JSONSerializer.new }
-    end
-end
-
-class MessageEncryptorMetadataNullSerializerTest < MessageVerifierMetadataTest
-  private
-    def data
-      "string message"
-    end
-
-    def null_serializing?
-      true
-    end
-
-    def verifier_options
-      { serializer: ActiveSupport::MessageEncryptor::NullSerializer }
-    end
-end

activesupport/test/messages/message_encryptor_metadata_test.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+require_relative "../abstract_unit"
+require_relative "message_metadata_tests"
+
+class MessageEncryptorMetadataTest < ActiveSupport::TestCase
+  include MessageMetadataTests
+
+  private
+    def make_codec(**options)
+      @secret ||= SecureRandom.random_bytes(32)
+      ActiveSupport::MessageEncryptor.new(@secret, **options)
+    end
+
+    def encode(data, encryptor, **options)
+      encryptor.encrypt_and_sign(data, **options)
+    end
+
+    def decode(message, encryptor, **options)
+      encryptor.decrypt_and_verify(message, **options)
+    rescue ActiveSupport::MessageVerifier::InvalidSignature
+      nil
+    end
+end

activesupport/test/messages/message_metadata_tests.rb

@@ -0,0 +1,139 @@
+# frozen_string_literal: true
+
+require "active_support/json"
+require "active_support/time"
+
+module MessageMetadataTests
+  extend ActiveSupport::Concern
+
+  included do
+    test "message :purpose must match specified :purpose" do
+      each_scenario do |data, codec|
+        assert_roundtrip data, codec, { purpose: "x" }, { purpose: "x" }
+
+        assert_no_roundtrip data, codec, { purpose: "x" }, { purpose: "y" }
+        assert_no_roundtrip data, codec, { purpose: "x" }, {}
+        assert_no_roundtrip data, codec, {}, { purpose: "x" }
+      end
+    end
+
+    test ":purpose can be a symbol" do
+      each_scenario do |data, codec|
+        assert_roundtrip data, codec, { purpose: :x }, { purpose: :x }
+        assert_roundtrip data, codec, { purpose: :x }, { purpose: "x" }
+        assert_roundtrip data, codec, { purpose: "x" }, { purpose: :x }
+      end
+    end
+
+    test "message expires with :expires_at" do
+      freeze_time do
+        each_scenario do |data, codec|
+          message = encode(data, codec, expires_at: 1.second.from_now)
+
+          travel 0.5.seconds, with_usec: true
+          assert_equal data, decode(message, codec)
+
+          travel 0.5.seconds, with_usec: true
+          assert_nil decode(message, codec)
+        end
+      end
+    end
+
+    test "message expires with :expires_in" do
+      freeze_time do
+        each_scenario do |data, codec|
+          message = encode(data, codec, expires_in: 1.second)
+
+          travel 0.5.seconds, with_usec: true
+          assert_equal data, decode(message, codec)
+
+          travel 0.5.seconds, with_usec: true
+          assert_nil decode(message, codec)
+        end
+      end
+    end
+
+    test ":expires_at overrides :expires_in" do
+      each_scenario do |data, codec|
+        message = encode(data, codec, expires_at: 1.hour.from_now, expires_in: 1.second)
+
+        travel 1.minute
+        assert_equal data, decode(message, codec)
+
+        travel 1.hour
+        assert_nil decode(message, codec)
+      end
+    end
+
+    test "messages do not expire by default" do
+      each_scenario do |data, codec|
+        message = encode(data, codec, purpose: "x")
+
+        travel 1000.years
+        assert_equal data, decode(message, codec, purpose: "x")
+      end
+    end
+
+    test "expiration works with ActiveSupport.use_standard_json_time_format = false" do
+      original_use_standard_json_time_format = ActiveSupport.use_standard_json_time_format
+      ActiveSupport.use_standard_json_time_format = false
+
+      each_scenario do |data, codec|
+        assert_roundtrip data, codec, { expires_at: 1.hour.from_now }
+      end
+    ensure
+      ActiveSupport.use_standard_json_time_format = original_use_standard_json_time_format
+    end
+
+    test "metadata works with NullSerializer" do
+      codec = make_codec(serializer: ActiveSupport::MessageEncryptor::NullSerializer)
+      assert_roundtrip "a string", codec, { purpose: "x", expires_in: 1.year }, { purpose: "x" }
+    end
+  end
+
+  private
+    class CustomSerializer
+      def self.dump(value)
+        JSON.dump(value) << "!"
+      end
+
+      def self.load(value)
+        JSON.load(value.chomp!("!"))
+      end
+    end
+
+    SERIALIZERS = [
+      Marshal,
+      JSON,
+      ActiveSupport::JSON,
+      ActiveSupport::JsonWithMarshalFallback,
+      CustomSerializer,
+    ]
+
+    DATA = [
+      "a string",
+      { "a_number" => 123, "a_time" => Time.local(2004), "an_object" => { "key" => "value" } },
+      ["a string", 123, Time.local(2004), { "key" => "value" }],
+    ]
+
+    def each_scenario
+      SERIALIZERS.each do |serializer|
+        codec = make_codec(serializer: serializer)
+        DATA.each do |data|
+          yield data, codec
+        end
+      end
+    end
+
+    def roundtrip(data, codec, encode_options = {}, decode_options = {})
+      decode(encode(data, codec, **encode_options), codec, **decode_options)
+    end
+
+    def assert_roundtrip(data, codec, encode_options = {}, decode_options = {})
+      assert_equal data, roundtrip(data, codec, encode_options, decode_options)
+    end
+
+    def assert_no_roundtrip(data, codec, encode_options = {}, decode_options = {})
+      assert_nil roundtrip(data, codec, encode_options, decode_options)
+    end
+end

activesupport/test/messages/message_verifier_metadata_test.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+require_relative "../abstract_unit"
+require_relative "message_metadata_tests"
+
+class MessageVerifierMetadataTest < ActiveSupport::TestCase
+  include MessageMetadataTests
+
+  test "#verify raises when :purpose does not match" do
+    each_scenario do |data, verifier|
+      assert_equal data, verifier.verify(verifier.generate(data, purpose: "x"), purpose: "x")
+
+      assert_raises ActiveSupport::MessageVerifier::InvalidSignature do
+        verifier.verify(verifier.generate(data, purpose: "x"), purpose: "y")
+      end
+
+      assert_raises ActiveSupport::MessageVerifier::InvalidSignature do
+        verifier.verify(verifier.generate(data), purpose: "y")
+      end
+
+      assert_raises ActiveSupport::MessageVerifier::InvalidSignature do
+        verifier.verify(verifier.generate(data, purpose: "x"))
+      end
+    end
+  end
+
+  test "#verify raises when message is expired via :expires_at" do
+    freeze_time do
+      each_scenario do |data, verifier|
+        message = verifier.generate(data, expires_at: 1.second.from_now)
+
+        travel 0.5.seconds, with_usec: true
+        assert_equal data, verifier.verify(message)
+
+        travel 0.5.seconds, with_usec: true
+        assert_raises ActiveSupport::MessageVerifier::InvalidSignature do
+          verifier.verify(message)
+        end
+      end
+    end
+  end
+
+  test "#verify raises when message is expired via :expires_in" do
+    freeze_time do
+      each_scenario do |data, verifier|
+        message = verifier.generate(data, expires_in: 1.second)
+
+        travel 0.5.seconds, with_usec: true
+        assert_equal data, verifier.verify(message)
+
+        travel 0.5.seconds, with_usec: true
+        assert_raises ActiveSupport::MessageVerifier::InvalidSignature do
+          verifier.verify(message)
+        end
+      end
+    end
+  end
+
+  private
+    def make_codec(**options)
+      ActiveSupport::MessageVerifier.new("secret", **options)
+    end
+
+    def encode(data, verifier, **options)
+      verifier.generate(data, **options)
+    end
+
+    def decode(message, verifier, **options)
+      verifier.verified(message, **options)
+    end
+end