search

prooph / pdo-event-store

PDO implementation of ProophEventStore http://getprooph.org
BSD 3-Clause "New" or "Revised" License
111 stars 56 forks source link

Aggregate root with `.` in ID will break PosgreSQL PostgresAggregateStreamStrategy #226

Closed patrickkusebauch closed 2 years ago

patrickkusebauch commented 3 years ago

I was able to drill it down to this line: https://github.com/prooph/pdo-event-store/blob/8f8c3621a8dacd5fc9ee8299e42dc552188b0ea4/src/PersistenceStrategy/PostgresAggregateStreamStrategy.php#L104

Current behavior/ Steps to reproduce/issue:

  1. Create an aggregate that has . in aggregateRootId (example: patrick.kusebauch).
  2. Try to store it with PostgresAggregateStreamStrategy
  3. The name of the table before the referenced line: _ff0a332cf5da03cc35df5dcceb1b87ba21af8c95 (41 chars)
  4. The name of the table after the referenced line: Full\Path\To\The\Class\Aggregates\User-patrick._ff0a332cf5da03cc35df5dcceb1b87ba21af8c95
  5. The event_streams table's column stream_name is char(41)
  6. Try to store the "wrong" table name
  7. Prooph\EventStore\Pdo\Exception\RuntimeException('Error 22001. Maybe the event streams table is not setup? Error-Info: ERROR: value too long for type character(41)')

Expected behavior

Able to create an aggregate with an ID that contains .

prolic commented 3 years ago

Can you try to change the function here? https://github.com/prooph/pdo-event-store/blob/master/src/Util/PostgresHelper.php#L44

patrickkusebauch commented 3 years ago

@prolic I do not understand what is the purpose of that function in the context of this package, as to me, from the bug description, it seems as this function should no be there at all.

prolic commented 3 years ago

What schema are you using in postgres? I would assume if you never used this value it should be default to public? At least as a workaround, try this public.patrick.kusebauch instead of this patrick.kusebauch. In postgres schema and table name are separated by a dot, hence we extract the schema name from the string as everything that is coming before the first dot.

patrickkusebauch commented 3 years ago

@prolic sorry for the delay. I use the Postgres:12 (git) docker container, with no extra settings. For table definitions, I use the provided SQL.

codeliner commented 3 years ago

@patrickkusebauch Can you try to set up a failing test case? You can add it here: https://github.com/prooph/pdo-event-store/blob/master/tests/PostgresEventStoreTest.php#L41

patrickkusebauch commented 3 years ago

@codeliner I have created a pull request. For the stream name, the heuristic is AggregateRootClass-AggregateId. This would represent one stream per aggregate strategy.