Skip to content

Commit 4f31d95

Browse files
mostlyobviousfidel
andcommitted
Enable backwards compatibility — no serialization case
Co-authored-by: Szymon Fiedler <[email protected]>
1 parent 06972ae commit 4f31d95

File tree

2 files changed

+57
-2
lines changed

2 files changed

+57
-2
lines changed

ruby_event_store-active_record/lib/ruby_event_store/active_record/event_repository.rb

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,30 @@ class EventRepository
77

88
def initialize(model_factory: WithDefaultModels.new, serializer:)
99
@serializer = serializer
10-
1110
@event_klass, @stream_klass = model_factory.call
12-
@event_klass.include(SkipJsonSerialization)
11+
if serializer == NULL && json_data_type?
12+
warn <<~MSG
13+
The data or metadata column is of a JSON/B type and expects a JSON string.
14+
15+
Yet the repository serializer is configured as #{serializer} and it would not
16+
produce the expected JSON string.
17+
18+
In ActiveRecord there's an implicit serialization to JSON for JSON/B column types
19+
that made it work so far. This behaviour is unfortunately also a source of undesired
20+
double serialization — first in the EventRepository, second in the ActiveRecord.
21+
22+
In the past we've advised workarounds that introduced configuration incosistency
23+
with other data types and serialization formats, i.e. explicitly passing NULL serializer
24+
just for the JSON/B data types.
25+
26+
As of now this special ActiveRecord behaviour is disabled. You should be using JSON
27+
serializer back again:
28+
29+
RubyEventStore::ActiveRecord::EventRepository.new(serializer: JSON)
30+
MSG
31+
else
32+
@event_klass.include(SkipJsonSerialization)
33+
end
1334
@repo_reader = EventRepositoryReader.new(@event_klass, @stream_klass, serializer)
1435
@index_violation_detector = IndexViolationDetector.new(@event_klass.table_name, @stream_klass.table_name)
1536
end
@@ -164,6 +185,12 @@ def append_to_stream_(records, stream, expected_version)
164185
end
165186
add_to_stream(event_ids, stream, expected_version) { @event_klass.insert_all!(hashes) }
166187
end
188+
189+
private
190+
191+
def json_data_type?
192+
%i[data metadata].any? { |attr| @event_klass.column_for_attribute(attr).type.start_with?("json") }
193+
end
167194
end
168195
end
169196
end

ruby_event_store-active_record/spec/event_repository_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,34 @@ module ActiveRecord
283283
)
284284
end
285285

286+
specify "JSON/B backwards compatibility — explicit NULL serializer as advised before introduction of JSONClient" do
287+
skip unless %w[json jsonb].include?(ENV["DATA_TYPE"])
288+
289+
expect { repository = EventRepository.new(serializer: NULL) }.to output(<<~MSG).to_stderr
290+
The data or metadata column is of a JSON/B type and expects a JSON string.
291+
292+
Yet the repository serializer is configured as RubyEventStore::NULL and it would not
293+
produce the expected JSON string.
294+
295+
In ActiveRecord there's an implicit serialization to JSON for JSON/B column types
296+
that made it work so far. This behaviour is unfortunately also a source of undesired
297+
double serialization — first in the EventRepository, second in the ActiveRecord.
298+
299+
In the past we've advised workarounds that introduced configuration incosistency
300+
with other data types and serialization formats, i.e. explicitly passing NULL serializer
301+
just for the JSON/B data types.
302+
303+
As of now this special ActiveRecord behaviour is disabled. You should be using JSON
304+
serializer back again:
305+
306+
RubyEventStore::ActiveRecord::EventRepository.new(serializer: JSON)
307+
MSG
308+
309+
expect {
310+
repository.append_to_stream([SRecord.new], Stream.new("stream"), ExpectedVersion.any)
311+
}.not_to raise_error
312+
end
313+
286314
private
287315

288316
def with_precision(time)

0 commit comments

Comments
 (0)