Switch from ValueSemantics to normal OOP Ruby

Amolith created

The more I tried to clean up and improve the use of ValueSemantics, the
more I came to the conclusion it would either have way too much
duplication or deduplicating would be too messy and complex.

Change summary

lib/message_event.rb   | 160 ++++++++++++++++++++++---------------------
test/test_component.rb |   4 -
2 files changed, 83 insertions(+), 81 deletions(-)

Detailed changes

lib/message_event.rb 🔗

@@ -1,11 +1,10 @@
 # frozen_string_literal: true
 
 require "json"
-require "value_semantics"
 
 module MessageEvent
 	module Emittable
-		def emit(redis=LazyObject.new { REDIS }, stream: "messages")
+		def emit(redis, stream: "messages")
 			args = to_redis_fields.flat_map { |k, v| [k, v] }
 			redis.xadd(stream, "*", *args).catch do |e|
 				puts "WARN Failed to emit message event: #{e.message}"
@@ -15,101 +14,108 @@ module MessageEvent
 
 	module ValidTel
 		def self.===(value)
-			return value.match?(/\A\+1\d{10}\z/) if value.is_a?(String)
-			return value.to_s.match?(/\A\+1\d{10}\z/) if value.respond_to?(:to_s)
-
-			false
+			value.to_s.match?(/\A\+1\d{10}\z/)
 		end
 	end
 
-	class MessageFields
-		include ValueSemantics.for_attributes {
-			event String
-			timestamp Time, coerce: true
-			from ValidTel
-			to ArrayOf(ValidTel)
-			message_id String
-			body String
-			media_urls ArrayOf(String), coerce: true
-		}
-
-		def self.coerce_timestamp(value)
-			value.is_a?(String) ? Time.parse(value) : value
-		end
+	class Base
+		include Emittable
+
+		def initialize(event:, timestamp:, message_id:)
+			raise ArgumentError, "event must be a String" unless event.is_a?(String)
+			raise ArgumentError, "message_id must be a String" unless message_id.is_a?(String)
 
-		def self.coerce_media_urls(value)
-			value.is_a?(String) ? JSON.parse(value) : value
+			Time.iso8601(timestamp) if timestamp.is_a?(String)
+			@event = event
+			@timestamp = timestamp
+			@message_id = message_id
 		end
 
+		attr_reader :event, :timestamp, :message_id
+
+		# We use to_redis_fields instead of to_h because we want to serialize values
+		# (e.g., Time -> ISO8601 string, arrays -> JSON). A plain to_h would return
+		# raw Ruby objects, which is less useful for parsing from other
+		# projects/languages.
 		def to_redis_fields
 			{
-				"event" => event,
-				"timestamp" => timestamp.iso8601,
-				"from" => from,
-				"to" => JSON.dump(to),
-				"message_id" => message_id,
-				"has_media" => media_urls.any?.to_s,
-				"body" => body,
-				"media_urls" => JSON.dump(media_urls)
+				"event" => @event,
+				"timestamp" => @timestamp.is_a?(Time) ? @timestamp.iso8601 : @timestamp,
+				"message_id" => @message_id
 			}
 		end
 	end
 
-	%w[In Out Thru].each { |n|
-		const_set(n, Class.new(MessageFields) {
-			include Emittable
+	class Message < Base
+		def initialize(to:, from:, body:, media_urls: [], **kwargs)
+			raise ArgumentError, "from must be a valid US telephone number" unless ValidTel === from
+			raise ArgumentError, "to must be an array" unless to.is_a?(Array)
+			to.each do |tel|
+				raise ArgumentError, "each to must be a valid US telephone number" unless ValidTel === tel
+			end
+			raise ArgumentError, "body must be a String" unless body.is_a?(String)
+			raise ArgumentError, "media_urls must be an Array" unless media_urls.is_a?(Array)
+
+			@from = from
+			@to = to
+			@body = body
+			@media_urls = media_urls
+			super(**kwargs)
+		end
 
-			const_set(n, lambda { |v|
-				v == n.downcase
-			})
+		attr_reader :from, :to, :body, :media_urls
 
-			define_method(:initialize) { |**a|
-				super(**a.merge(event: n.downcase))
-			}
-		})
-	}
+		def to_redis_fields
+			super.merge(
+				"from" => @from,
+				"to" => JSON.dump(@to),
+				"body" => @body,
+				"media_urls" => JSON.dump(@media_urls)
+			)
+		end
+	end
 
-	class Delivered
-		include Emittable
-		Delivered = lambda { |v| v == "delivered" }
-		include ValueSemantics.for_attributes {
-			event Delivered
-			timestamp Either(Time, String)
-			message_id String
-		}
-
-		def initialize(**a)
-			super(**a.merge(event: "delivered"))
+	class In < Message
+		def initialize(**kwargs)
+			super(event: "in", **kwargs)
 		end
+	end
 
-		def to_redis_fields = {
-			"event" => event,
-			"timestamp" => timestamp.iso8601,
-			"message_id" => message_id
-		}
+	class Out < Message
+		def initialize(**kwargs)
+			super(event: "out", **kwargs)
+		end
 	end
 
-	class Failed
-		include Emittable
-		Failed = lambda { |v| v == "failed" }
-		include ValueSemantics.for_attributes {
-			event Failed
-			timestamp Either(Time, String)
-			message_id String
-			error_code String
-			error_description String
-		}
-
-		def initialize(**a)
-			super(**a.merge(event: "failed"))
+	class Thru < Message
+		def initialize(**kwargs)
+			super(event: "thru", **kwargs)
+		end
+	end
+
+	class Delivered < Base
+		def initialize(**kwargs)
+			super(event: "delivered", **kwargs)
+		end
+	end
+
+	class Failed < Base
+		def initialize(error_code:, error_description:, **kwargs)
+			raise ArgumentError, "error_code must be a String" unless error_code.is_a?(String)
+			raise ArgumentError, "error_description must be a String" unless error_description.is_a?(String)
+
+			@error_code = error_code
+			@error_description = error_description
+			super(event: "failed", **kwargs)
 		end
 
-		def to_redis_fields = {
-			"event" => event,
-			"timestamp" => timestamp.is_a?(Time) ? timestamp.iso8601 : timestamp,
-			"message_id" => message_id,
-			"error_code" => error_code,
-			"error_description" => error_description
-		}
+		attr_reader :error_code, :error_description
+
+		def to_redis_fields
+			super.merge(
+				"error_code" => @error_code,
+				"error_description" => @error_description
+			)
+		end
 	end
 end

test/test_component.rb 🔗

@@ -537,7 +537,6 @@ class ComponentTest < Minitest::Test
 		assert_equal "+15550000000", event["from"]
 		assert_equal JSON.dump(["+15551234567"]), event["to"]
 		assert_equal "bw-msg-123", event["message_id"]
-		assert_equal "false", event["has_media"]
 		assert_equal "Hello world", event["body"]
 	end
 	em :test_outbound_message_emits_to_stream
@@ -561,7 +560,6 @@ class ComponentTest < Minitest::Test
 		assert_equal "+15550000000", event["from"]
 		assert_equal JSON.dump(["+15559999999"]), event["to"]
 		assert_equal "passthru-stanza-456", event["message_id"]
-		assert_equal "false", event["has_media"]
 		assert_equal "Pass through", event["body"]
 	end
 	em :test_passthrough_message_emits_to_stream
@@ -607,7 +605,6 @@ class ComponentTest < Minitest::Test
 		assert_equal "+15551234567", event["from"]
 		assert_equal JSON.dump(["+15550000000"]), event["to"]
 		assert_equal "bw-in-123", event["message_id"]
-		assert_equal "false", event["has_media"]
 		assert_equal "Hello from outside", event["body"]
 		assert_equal JSON.dump([]), event["media_urls"]
 	end
@@ -640,7 +637,6 @@ class ComponentTest < Minitest::Test
 
 		event = entries.first[:fields]
 		assert_equal "in", event["event"]
-		assert_equal "true", event["has_media"]
 		assert_equal JSON.dump(["https://example.com/image.jpg"]), event["media_urls"]
 	end
 	em :test_inbound_mms_emits_to_stream_and_filters_smil