Add first/last to read specification #399

mpraglowski · 2018-07-18T13:01:28Z

Now it will be possible:

first_event_in_dummy_stream = event_store.read.stream("Dummy").first
last_event_published = event_store.read.last

mpraglowski · 2018-07-18T13:02:28Z

@joelvh please review
d39bbd3

mostlyobvious · 2018-07-18T13:02:41Z

Getting closer including Enumerable 😉

joelvh · 2018-07-18T15:48:42Z

ruby_event_store-rom/lib/ruby_event_store/rom/repositories/events.rb

+              .ordered(direction, stream, offset_entry_id)
+              .combine(:event)
+              .map_with(:stream_entry_to_serialized_record, auto_struct: false)
+              .to_ary.first


@mpraglowski I think you don't want .to_ary here -- just first should get you one record

right, first is implemented here

joelvh · 2018-07-18T15:50:07Z

ruby_event_store-rom/lib/ruby_event_store/rom/repositories/events.rb

+              .ordered(direction, stream, offset_entry_id)
+              .combine(:event)
+              .map_with(:stream_entry_to_serialized_record, auto_struct: false)
+              .to_ary.last


@mpraglowski I think you don't want .to_ary here -- just last should get you one record. Internally, it reverses the SQL ORDER and LIMITs to one record. I'm not sure if that means of reversing the query is always accurate. Presumably your tests check for that.

however here last without to_ary results in failing tests:

NoMethodError: undefined method `last' for #<ROM::Relation::Composite:0x00007fd4226cecf8> Shared Example Group: :event_repository called from ./lib/ruby_event_store/spec/rom/event_repository_lint.rb:26

and

Sequel::DatabaseError: SQLite3::SQLException: near "DESC": syntax error Shared Example Group: :event_repository called from ./lib/ruby_event_store/spec/rom/event_repository_lint.rb:26

joelvh · 2018-07-20T23:01:08Z

@mpraglowski worked up a fix to DRY things up. To solve the last issue, I switch direction and use first. Tests pass. However, can you make sure we have tests that sufficiently test first/last with an offset, to make sure the proper last event is retrieved?

paneq · 2018-07-22T07:38:50Z

ruby_event_store/lib/ruby_event_store/specification.rb

@@ -7,10 +7,16 @@ class Specification
    NO_LIMIT = Object.new.freeze
    # @private
    # @api private
-    NO_BATCH = Object.new.freeze
+    FIRST    = Object.new.freeze


just use private_const instead!

…ries

That's just an implementation details that should never leak out of the Specification class. Still used in specification tests however should it also should be avoided.

mpraglowski · 2018-07-23T16:24:16Z

However, can you make sure we have tests that sufficiently test first/last with an offset, to make sure the proper last event is retrieved?

@joelvh I will add that test later today.

joelvh · 2018-07-23T16:55:14Z

thanks @mpraglowski !

…last event

Limit is a count value or infinity. Becuase when we do not set count for specification our limit is infinity. It cound be clearly spotted in total_limit method of AR eveny repository.

…ading whole stream

mostlyobvious · 2018-07-26T09:08:17Z

rails_event_store-rspec/lib/rails_event_store/rspec/publish.rb

@@ -79,7 +79,7 @@ def match_events?
      end

      def last_event(spec)
-        spec.backward.limit(1).each.first
+        spec.last
      end


inline method maybe?

mostlyobvious · 2018-07-26T09:09:08Z

...nt_store_active_record-legacy/lib/rails_event_store_active_record/legacy/event_repository.rb

@@ -76,7 +80,7 @@ def read(spec)
      private

      def total_limit(specification)
-        specification.limit? ? specification.count : Float::INFINITY
+        specification.limit
      end


would inline this as well

mostlyobvious · 2018-07-26T09:12:44Z

ruby_event_store/lib/ruby_event_store/specification.rb

    end
    alias :in_batches_of :in_batches

+    def read_first


doc missing

mostlyobvious · 2018-07-26T09:12:49Z

ruby_event_store/lib/ruby_event_store/specification.rb

+      Specification.new(repository, mapper, result.dup.tap { |r| r.read_as = :first })
+    end
+
+    def read_last


doc missing

mostlyobvious · 2018-07-26T09:21:11Z

ruby_event_store/spec/specification_spec.rb

+    specify do
+      repository.append_to_stream([test_record], Stream.new("Dummy"), ExpectedVersion.none)
+
+      expect(specification.result.batched?).to be_falsey


mostlyobvious · 2018-07-26T09:21:19Z

ruby_event_store/spec/specification_spec.rb

+      expect(specification.result.last?).to be_falsey
+
+      expect(specification.read_first.result.batched?).to be_falsey
+      expect(specification.read_first.result.first?).to be_truthy


Method first & last returns nil when no event is found based on given read specification. No error is raised here.

mpraglowski requested a review from joelvh July 18, 2018 13:02

joelvh reviewed Jul 18, 2018

View reviewed changes

mpraglowski force-pushed the read-spec-last branch from 3a18e2d to 9a87f65 Compare July 19, 2018 21:13

paneq reviewed Jul 22, 2018

View reviewed changes

mpraglowski and others added 16 commits July 23, 2018 18:19

New methods to read first & last events from specified read spec

619b912

Implementation of first & last read spec methond on AR event reposito…

1fc66bf

…ries

Additional requirements for event repository implementations

b96360a

Handle first/last reads in ROM event repository

afaaf64

Simplify fetching last event from a stream using new API

d19b46c

Unnecessary code remove

8af5d0d

Killing the mutants

5de1b93

No need for that if here

a02ff95

DRY'd up and work around last to use first

7082e7a

Additional test to kill mutations

fbc5c81

API documentation

eb9bf12

Stop using Specification private internals in ROM repository

8f1f88e

Specification is immutable - never modify it directly

3c6b205

Fix expectation - no mutatable specification

14b2936

Symbols over constants

970452d

That's just an implementation details that should never leak out of the Specification class. Still used in specification tests however should it also should be avoided.

Killing mutants

b319a24

mpraglowski force-pushed the read-spec-last branch from 89a1af2 to b319a24 Compare July 23, 2018 16:19

mpraglowski requested a review from mostlyobvious July 23, 2018 16:23

mpraglowski added 2 commits July 23, 2018 22:47

Additional tests to verify offset works correctly when reading first/…

083b4ec

…last event

Use specification limit instead of count

8c10eab

Limit is a count value or infinity. Becuase when we do not set count for specification our limit is infinity. It cound be clearly spotted in total_limit method of AR eveny repository.

mpraglowski added 2 commits July 25, 2018 17:22

The read last optimalization for ROM event repository fix when not re…

757b4ac

…ading whole stream

Kill a few more mutants

a677aa5

mostlyobvious reviewed Jul 26, 2018

View reviewed changes

mpraglowski added 7 commits July 26, 2018 23:11

Inline methods.

951fb59

More precise expectations.

a1179ef

Missing docs added.

bf4decb

Be like enumerable - nils over errors

bbac69c

Method first & last returns nil when no event is found based on given read specification. No error is raised here.

And actually implement nils over errors in event repositories.

15636c0

No need to check for nils here - build in into build_event_entity

f489dd0

Killing mutants

9d0b696

mpraglowski merged commit 6cc5ad5 into RailsEventStore:master Jul 30, 2018

mpraglowski deleted the read-spec-last branch July 30, 2018 16:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add first/last to read specification #399

Add first/last to read specification #399

mpraglowski commented Jul 18, 2018

mpraglowski commented Jul 18, 2018

mostlyobvious commented Jul 18, 2018 •

edited

Loading

joelvh Jul 18, 2018

mpraglowski Jul 19, 2018

joelvh Jul 18, 2018

mpraglowski Jul 19, 2018

joelvh commented Jul 20, 2018

paneq Jul 22, 2018

mpraglowski commented Jul 23, 2018

joelvh commented Jul 23, 2018

mostlyobvious Jul 26, 2018

mostlyobvious Jul 26, 2018

mostlyobvious Jul 26, 2018

mostlyobvious Jul 26, 2018

mostlyobvious Jul 26, 2018

mostlyobvious Jul 26, 2018

Add first/last to read specification #399

Add first/last to read specification #399

Conversation

mpraglowski commented Jul 18, 2018

mpraglowski commented Jul 18, 2018

mostlyobvious commented Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joelvh commented Jul 20, 2018

Choose a reason for hiding this comment

mpraglowski commented Jul 23, 2018

joelvh commented Jul 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mostlyobvious commented Jul 18, 2018 •

edited

Loading