Upgrade package dependencies

[CumpsD]

Upgrade dependencies and make sure the code still works

[CumpsD]

@Aaronontheweb I'm a little bit stuck, if you have some time and could have a look if you find something please?

There are 3 failing tests:

The rest are ok:

If you debug at src\Akka.Persistence.Sql\Db\AkkaPersistenceDataConnectionFactory.cs inside the HasConversion funcs (you can temporarly use HasConversionFunc if the breakpoints don't get hit), you will see the DateTime gets converted into Ticks when the test prepares its snapshot data and writes it to MS Sqlite.

if (config.ProviderName.ToLower().Contains("sqlite") ||
    config.ProviderName.ToLower().Contains("postgresql"))
{
    builder
        .Member(r => r.Created)
        .HasDataType(DataType.Int64)
        .HasConversion(
            r => r.Ticks,
            r => new DateTime(r));
}

However, when it tries to read them, it fails with:

Got a message of the expected type <Akka.Persistence.LoadSnapshotResult>. Also expected the predicate to return true but the message {LoadSnapshotResult<toSeqNr: 9223372036854775807, snapshot: >} of type <Akka.Persistence.LoadSnapshotResult> did not match
Expected: True
Actual:   False
   at Akka.TestKit.Xunit2.XunitAssertions.AssertTrue(Boolean condition, String format, Object[] args)
   at Akka.TestKit.TestKitBase.AssertPredicateIsTrueForMessage[T](Predicate`1 isMessage, T m, String hint)
   at Akka.TestKit.TestKitBase.<>c__DisplayClass121_0`1.<ExpectMsgAsync>b__0(T m, IActorRef sender)
   at Akka.TestKit.TestKitBase.InternalExpectMsgEnvelopeAsync[T](Nullable`1 timeout, Action`2 assert, String hint, CancellationToken cancellationToken, Boolean shouldLog)
   at Akka.TestKit.TestKitBase.InternalExpectMsgAsync[T](Nullable`1 timeout, Action`2 assert, String hint, CancellationToken cancellationToken)
   at Akka.TestKit.TestKitBase.ExpectMsgAsync[T](Predicate`1 isMessage, Nullable`1 timeout, String hint, CancellationToken cancellationToken)
   at Akka.TestKit.TestKitBase.ExpectMsg[T](Predicate`1 isMessage, Nullable`1 timeout, String hint, CancellationToken cancellationToken)
   at Akka.Persistence.TCK.Snapshot.SnapshotStoreSpec.SnapshotStore_should_delete_all_snapshots_matching_upper_sequence_number_and_timestamp_bounds()
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

If you debug src\Akka.Persistence.Sql\Snapshot\ByteArraySnapshotDao.cs at SnapshotForMaxSequenceNrAndMaxTimestamp you will see the values being passed into it are fine, however the FirstOrDefaultAsync returns null, even though the row is there:

I currently have no idea yet why this is happening. I was trying to find a way to debug the exact query being sent to Sqlite, maybe it doesn't convert the timestamp parameter to ticks properly?

[CumpsD]

Of course, by typing out my problem I get a step closer to the answer :) I managed to find the query it sends to the database:

SELECT
	[r].[persistence_id],
	[r].[sequence_number],
	[r].[created],
	[r].[snapshot],
	[r].[manifest],
	[r].[serializer_id]
FROM
	[snapshot] [r]
WHERE
	[r].[persistence_id] = @persistenceId AND [r].[sequence_number] <= @sequenceNr AND
	DateTime([r].[created]) <= DateTime(@timestamp_1)
ORDER BY
	[r].[sequence_number] DESC
LIMIT @take

That DateTime([r].[created]) <= DateTime(@timestamp_1) is probably the problem, I'll try to figure out why it does that and not simply does the comparison with ticks.

[Arkatufus]

@CumpsD I'll look into it

[CumpsD]

Just checked with the dev branch, the query there is:

SELECT
	[r].[persistence_id],
	[r].[sequence_number],
	[r].[created],
	[r].[snapshot],
	[r].[manifest],
	[r].[serializer_id]
FROM
	[snapshot] [r]
WHERE
	[r].[persistence_id] = @persistenceId AND [r].[sequence_number] <= @sequenceNr AND
	[r].[created] <= @timestamp_1
ORDER BY
	[r].[sequence_number] DESC
LIMIT @take

I'll have a look at linq2db, I'm guessing something changed in the breaking changes from 3->5. I've read all the release notes though, guess I missed something

[CumpsD]

Upgrading linq2db to 3.7.0 already changed it to

SELECT
	[r].[persistence_id],
	[r].[sequence_number],
	[r].[created],
	[r].[snapshot],
	[r].[manifest],
	[r].[serializer_id]
FROM
	[snapshot] [r]
WHERE
	[r].[persistence_id] = @persistenceId AND [r].[sequence_number] <= @sequenceNr AND
	DateTime([r].[created]) <= DateTime(@timestamp_1)
ORDER BY
	[r].[sequence_number] DESC
LIMIT @take

So not even in the breaking changes ;) I will look further as to where it happened

[CumpsD]

3.5.2 is the last version the test succeeds.

3.6.0 broke it
Release notes: https://github.com/linq2db/linq2db/wiki/Release-Notes-3.0.0#release-360
PR: linq2db/linq2db#3343

I'll have a look at what changed, also asked it in their repo: linq2db/linq2db#3343 (comment)

[CumpsD]

Another tidbit of information: linq2db/linq2db#2432
Wrapping with DateTime() got added via linq2db/linq2db#478 at linq2db/linq2db@9f6d92c#diff-311134ff10546110237f8b2ee7c7fd039c27a57a2fccacf819266612e02363a1R133

[CumpsD]

For some reason, the azure checks don't run anymore

[Aaronontheweb]

I’ll need to fix that - repo name change broke itSent from my iPhoneOn Mar 21, 2023, at 4:05 AM, David Cumps ***@***.***> wrote: For some reason, the azure checks don't run anymore —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[CumpsD]

Just linking some for admin purposes.

• Fixes Bump Akka.Persistence.PostgreSql from 1.4.46 to 1.5.1 #162
• Fixes Bump Akka.Persistence.MySql from 1.4.42 to 1.5.0 #161
• Fixes Bump Microsoft.Data.Sqlite.Core from 6.0.4 to 7.0.4 #160
• Fixes Bump LanguageExt.Core from 3.4.15 to 4.4.2 #139
• Fixes Bump System.Linq.Async from 4.1.1 to 6.0.1 #113
• Fixes Bump Microsoft.Bcl.HashCode from 1.1.0 to 1.1.1 #112
• Fixes Bump JetBrains.dotMemoryUnit from 3.1.20200127.214830 to 3.2.20220510 #109
• Fixes Bump Npgsql from 4.1.4 to 6.0.1 #46
• Fixes Bump System.Reactive.Linq from 4.4.1 to 5.0.0 #43

[CumpsD]

@Arkatufus @Aaronontheweb it's ready for review now, the pipelines are fixed and ran everything succesfully.

Upgrading Linq2db will be in a different PR since it has more impact, and is harder to upgrade (see above)

[Aaronontheweb]

LGTM