Unify timestamp storage #674

mostlyobvious · 2019-10-04T00:03:52Z

[#627]

Breaking changes so far:

format of serialized message for async jobs (problematic when deploying changes — workers need strategy to handle both formats for a while, also RES client must be able to deserialize both)
ROM timestamps changed to local time as Sequel expects, where previously put as UTC interpreted as a local time — mostlī harmless if you acknowledge time skew before certain point in time

Doubts:

SerializedRecord#timestamp as a Time object or just an ISO8601 string? The latter better for async jobs, the former suits adapters better and requires less instantiations — do we miss some other kind of SerializedRecord or need a different internal Record?
repositories use SerializedRecord#timestamp as a source for persisted timestamp (created_at in schema)

Future extensions on in this PR:

exposing link time (created_at in event_store_events_in_streams)
bi-temporal modeling, need one (or more) additional timestamp
filtering by timestamp

Event timestamp in an instance of Time.

swistak35

👍 I haven't reviewed protobuf part and ROM part

rails_event_store_active_record/lib/rails_event_store_active_record/event_repository.rb

mostlyobvious · 2020-08-21T10:17:42Z

rails_event_store_active_record/lib/rails_event_store_active_record/event_repository.rb

@@ -18,7 +18,7 @@ def append_to_stream(events, stream, expected_version)
        event_ids << event.event_id
      end
      add_to_stream(event_ids, stream, expected_version, true) do
-        Event.import(records)
+        Event.import(records, timestamps: false)


what does a false do here? we definitely want timestamps from an event, not a database/activerecord generated one

https://github.com/zdennis/activerecord-import#activerecord-timestamps

Should you wish to specify those columns, you may use the option timestamps: false.

However next paragraph might cause this to be obsolete:

However, it is also possible to set just :created_at in specific records. In this case despite using timestamps: true, :created_at will be updated only in records where that field is nil. Same rule applies for record associations when enabling the option recursive: true

mostlyobvious · 2020-08-21T10:18:54Z

ruby_event_store/lib/ruby_event_store/client.rb

@@ -222,8 +222,16 @@ def with_metadata(metadata, &block)
    # {http://railseventstore.org/docs/subscribe/#async-handlers Read more}
    #
    # @return [Event, Proto] deserialized event
-    def deserialize(event_type:, event_id:, data:, metadata:)
-      mapper.serialized_record_to_event(SerializedRecord.new(event_type: event_type, event_id: event_id, data: data, metadata: metadata))
+    def deserialize(event_type:, event_id:, data:, metadata:, timestamp:)


missing test case for timestamp: nil and timestamp still passed as metadata

mostlyobvious · 2020-08-21T10:24:03Z

rails_event_store/spec/active_job_scheduler_spec.rb

-                "metadata"        => "---\n:timestamp: 2019-09-30 00:00:00.000000000 Z\n",
-                "_aj_symbol_keys" => %w[event_id data metadata event_type],
+                "metadata"        => "--- {}\n",
+                "timestamp"       => {"_aj_serialized"=>"ActiveJob::Serializers::TimeSerializer", "value"=>"2019-09-30T00:00:00Z"},


AJ specific, would prefer something more generic even if just for AJ

mostlyobvious · 2020-09-12T21:26:19Z

Probably needs database migrations for mysql to upgrade timestamp resolution.

mpraglowski · 2020-09-14T08:38:52Z

contrib/transformations/lib/transformations/with_indifferent_access.rb

@@ -8,7 +8,8 @@ def dump(item)
        event_id:   item.event_id,
        metadata:   ActiveSupport::HashWithIndifferentAccess.new(item.metadata).deep_symbolize_keys,
        data:       ActiveSupport::HashWithIndifferentAccess.new(item.data).deep_symbolize_keys,
-        event_type: item.event_type
+        event_type: item.event_type,


We should exclude this change in contrib project. It depends on already released RubyEventStore gem version and this will break the tests as timestamp changes has not yet been released. Also IMO we should avoid mixing contrib projects changes into PR with main gems code.

That helps repositories to separate it from metadata enabling better indexing and possibly filtering.

MySQL and Postgres seem to settle on microseconds.

Linux for example has 7 digits in fractional part.

mostlyobvious force-pushed the timestamp-like-a-boss branch 4 times, most recently from 537491e to c29af6f Compare October 8, 2019 22:02

mostlyobvious referenced this pull request Oct 18, 2019

Harmonize time data type.

85cb1c7

Event timestamp in an instance of Time.

mostlyobvious force-pushed the timestamp-like-a-boss branch from c29af6f to dabdbbf Compare February 27, 2020 23:17

swistak35 approved these changes Apr 30, 2020

View reviewed changes

rails_event_store_active_record/lib/rails_event_store_active_record/event_repository.rb Outdated Show resolved Hide resolved

mostlyobvious mentioned this pull request May 5, 2020

Wrong type of event's timestamp when using some mappers #729

Closed

fidel force-pushed the timestamp-like-a-boss branch from dabdbbf to 9863501 Compare June 8, 2020 21:01

mpraglowski force-pushed the timestamp-like-a-boss branch from 5893ed4 to 533f7ea Compare August 20, 2020 13:57

mostlyobvious force-pushed the timestamp-like-a-boss branch from 24d3ffc to 236507a Compare August 21, 2020 09:53

mostlyobvious requested review from swistak35, mpraglowski and fidel August 21, 2020 10:03

mostlyobvious commented Aug 21, 2020

View reviewed changes

mostlyobvious force-pushed the timestamp-like-a-boss branch 10 times, most recently from 48c0cfe to 29c8ed1 Compare September 12, 2020 21:22

mpraglowski reviewed Sep 14, 2020

View reviewed changes

Revert df643a2.

5005909

mostlyobvious added 16 commits September 15, 2020 10:38

Extend SerializedRecord and Record with timestamp.

02a9026

That helps repositories to separate it from metadata enabling better indexing and possibly filtering.

Adapt mappers to timestamp passed in SerializedRecord.

059b30a

ROM repository using timestamp from SerializedRecord.

846d978

RESAR repository using timestamp from SerializedRecord.

bb3fb0b

Adjust serialized object format in ActiveJob for new SerializedRecord.

c884c82

Satisfy mutant.

0b56431

Uniform timestamp precision.

1bf46a4

Simplify.

abc00e5

Verify timestamp precision of an adapter

d73ba8f

Do not exceed data store timestamp precision

de5eed6

MySQL and Postgres seem to settle on microseconds.

Make sure update API does not damage timestamps

2a4ade9

Make test comparisons easier 🙈

dd3cff1

Best available timestamp precision for MySQL

cdae1ed

Make test comparisons easier (across different OSes) 🙈

64df1b1

Linux for example has 7 digits in fractional part.

Cap timestamp precision to maximum supported by adapters.

fedf21d

Stop this crazy thing.

4850f75

mpraglowski force-pushed the timestamp-like-a-boss branch from 4ad0eec to 4850f75 Compare September 15, 2020 08:40

mpraglowski and others added 2 commits September 15, 2020 10:43

Kill mutations. Fetch over []

ec0ec11

Ensure no harm done from library on timestamps.

a4fcbb8

mostlyobvious merged commit 4c12b59 into master Sep 15, 2020

mostlyobvious deleted the timestamp-like-a-boss branch September 15, 2020 09:38

mostlyobvious mentioned this pull request Sep 15, 2020

Better pagination on browser #763

Open

mostlyobvious mentioned this pull request Dec 15, 2020

Ruby Event Store Browser not loading events because of nil error #762

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify timestamp storage #674

Unify timestamp storage #674

mostlyobvious commented Oct 4, 2019 •

edited

Loading

swistak35 left a comment

mostlyobvious Aug 21, 2020

mpraglowski Aug 21, 2020

mostlyobvious Aug 21, 2020

mostlyobvious Aug 21, 2020

mostlyobvious commented Sep 12, 2020

mpraglowski Sep 14, 2020

Unify timestamp storage #674

Unify timestamp storage #674

Conversation

mostlyobvious commented Oct 4, 2019 • edited Loading

swistak35 left a comment

Choose a reason for hiding this comment

mostlyobvious Aug 21, 2020

Choose a reason for hiding this comment

mpraglowski Aug 21, 2020

Choose a reason for hiding this comment

mostlyobvious Aug 21, 2020

Choose a reason for hiding this comment

mostlyobvious Aug 21, 2020

Choose a reason for hiding this comment

mostlyobvious commented Sep 12, 2020

mpraglowski Sep 14, 2020

Choose a reason for hiding this comment

mostlyobvious commented Oct 4, 2019 •

edited

Loading