Fix broken object type serializer in QueryExecutor #6528
Conversation
Aaronontheweb
added
akka-persistence-sql-common
| @@ -1228,7 +1229,7 @@ private async Task<WriteMessagesResult> HandleWriteMessages(WriteMessages req, T | |||
| protected virtual void WriteEvent(TCommand command, IPersistentRepresentation persistent, string tags = "") | |||
| { | |||
| var payloadType = persistent.Payload.GetType(); | |||
| var serializer = _serialization.FindSerializerForType(payloadType, Setup.DefaultSerializer); | |||
| var serializer = _serialization.FindSerializerForType(payloadType); | |||
There was a problem hiding this comment.
This is the actual fix, we do not use the default serializer set in the persistence plugin HOCON settings for writes anymore.
| @@ -780,7 +781,7 @@ protected DbCommand GetCommand(DbConnection connection, string sql) | |||
| protected virtual void WriteEvent(DbCommand command, IPersistentRepresentation e, IImmutableSet<string> tags) | |||
| { | |||
|
|
|||
| var serializer = Serialization.FindSerializerForType(e.Payload.GetType(), Configuration.DefaultSerializer); | |||
| var serializer = Serialization.FindSerializerForType(e.Payload.GetType()); | |||
There was a problem hiding this comment.
This is the actual fix, we do not use the default serializer set in the persistence plugin HOCON settings for writes anymore.
| @@ -846,7 +847,10 @@ protected virtual IPersistentRepresentation ReadEvent(DbDataReader reader) | |||
| { | |||
| // Support old writes that did not set the serializer id | |||
| var type = Type.GetType(manifest, true); | |||
| #pragma warning disable CS0618 | |||
| // Backward compatibility code, we still need to use the old default serializer on read to support legacy data | |||
| var deserializer = Serialization.FindSerializerForType(type, Configuration.DefaultSerializer); | |||
There was a problem hiding this comment.
This is the only code in the SQL common journal that still uses the old settings. The purpose of this code is to make sure that really old (pre-v1.3.0) data are still readable by the journal. These pre-v1.3.0 event data does not have their serializer_id column populated.
There was a problem hiding this comment.
Another piece of code that still uses this still uses this setting is the snapshot store reader, just like this one, it is there to make sure that legacy data are still readable.
| @@ -224,22 +224,5 @@ protected override async Task DeleteAsync(string persistenceId, SnapshotSelectio | |||
| await QueryExecutor.DeleteBatchAsync(connection, nestedCancellationTokenSource.Token, persistenceId, criteria.MaxSequenceNr, criteria.MaxTimeStamp); | |||
| } | |||
| } | |||
|
|
|||
| private SnapshotEntry ToSnapshotEntry(SnapshotMetadata metadata, object snapshot) | |||
There was a problem hiding this comment.
Byte-rot method, this method is not being used anymore.
| } | ||
|
|
||
| [Fact(DisplayName = "Legacy v1.3.0 and below data (null serializer_id) should be read correctly")] | ||
| public void ReadLegacyDataTest() |
There was a problem hiding this comment.
The purpose of this test is to make sure that data saved from v1.3.0 can still be recovered successfully. The Sqlite file were generated using v.1.3.0 code and is being recovered in this unit test.
| await record.ReadAsync(); | ||
|
|
||
| // In the bug this fails, the serializer id is JSON id instead of MySerializer id | ||
| record[0].Should().Be(9999); |
There was a problem hiding this comment.
This unit test makes sure that the old serializer HOCON setting are not being used in journal writes anymore.
Fixes akkadotnet/Akka.Hosting#127
Fixes #6389
A Port of #6223 from v1.4 branch to dev branch
Changes
Issue
Any non-serializer mapped objects being serialized by
Akka.Persistence.Sql.Common.Journal.QueryExecutorwill use the JSON serializer and not the object type serializer.Problem is with this line:
akka.net/src/contrib/persistence/Akka.Persistence.Sql.Common/Journal/QueryExecutor.cs
Line 783 in 39a8f9e
which takes its value from the very obscure and very rarely used HOCON override here:
akka.net/src/core/Akka.Persistence/persistence.conf
Line 119 in 39a8f9e
Workaround
Override the HOCON setting:
or declare the default non-mapped object serializer in the journal/snapshot settings: