-
Notifications
You must be signed in to change notification settings - Fork 125
Additional value mapping (Hanami 2 integration) #1297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is only relevant when running tests as i is only used in test configuration
Reason: In hanami integration, for some reason, I get hash instead of String in those two fields. Possibly there is sth wrong with the configuration itself but just for now I added this quick fix.
gem "pg", ">= 1.2.2" | ||
gem "ruby_event_store", path: "../../ruby_event_store" | ||
gem "sqlite3", "1.4.2" | ||
gem "activesupport" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gemfile is purely for test. We're not using bundler groups at all anywhere in the RES codebase.
You can have a look on ROM dependencies at:
rails_event_store/contrib/ruby_event_store-rom/ruby_event_store-rom.gemspec
Lines 29 to 36 in be89cf8
spec.add_dependency "dry-container", ">= 0.6" | |
spec.add_dependency "dry-initializer", ">= 3.0" | |
spec.add_dependency "dry-types", ">= 1.0" | |
spec.add_dependency "rom-changeset", ">= 5.0" | |
spec.add_dependency "rom-repository", ">= 5.0" | |
spec.add_dependency "rom-sql", ">= 3.0" | |
spec.add_dependency "sequel", ">= 5.11.0" | |
spec.add_dependency "ruby_event_store", ">= 2.0.0", "< 3.0.0" |
map_value :valid_at, ->(time) { Time.iso8601(time).localtime } | ||
map_value :data, ->(value) { value.class.name == "Hash" ? value.to_json : value } | ||
map_value :metadata, ->(value) { value.class.name == "Hash" ? value.to_json : value } | ||
accept_keys %i[event_id data metadata event_type created_at valid_at] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps worth adding a test recreating the situation you've encountered?
Long story short:
|
Can you first check that switching https://github.com/RailsEventStore/ecommerce/blob/6de6008878decfb4332a01e052202fb4a473bb4f/hanami_application/config/providers/event_repository.rb#L9 |
Loudly thinking — if there's a way to pass ActiveRecord data/metadata as strings for json/jsonb columns, that do not get additional to-string serialization, that would greatly reduce complexity around serailizers for JSON across all known repositories. Same interface for all (as initially designed) — repository gets serialized-data-as-string and has to put it in right place, without insisting on serializing it once again (as AR does here). |
Btw. that SQL payload was the most valuable part of the exception as it shows data in raw shape as we insert it. Too bad it was trimmed.
|
Verified: RailsEventStore/ecommerce#110 (comment) |
RailsEventStore/rails_event_store#1297 (comment) Co-authored-by: Paweł Świątkowski <katafrakt@users.noreply.github.com>
Overview
Tweaks for Hanami 2 integration.
Background
During configuring of Hanami 2 project to work with RES-ROM in order to integrate ecommerce application RailsEventStore/ecommerce#110, I have encountered Sequel problem: sequel-error.txt. Command below accepts named arguments, so there is no way to call the command handler directly passing invalid parameters to it.
However, calling command bus with this command returns an Sequel Error: Column not found (details below).
I have found that
CreateEvents
changeset receives Hash as a value for JSONB columns (data and metadata) in case of Hanami 2 integration, and String in Rails or tests.To solve it, I added two additional MAP_VALUE rules.
It sems to solve the problem, but I am not happy with this as I believe, I did not find the source of the problem. I expect that some kind of value transformation was not triggered.
Additional notes
Summary
I keep this PR as a monkey patch for now, until the proper fix is found, to make Hanami 2 integration possible. Help welcome!