Removes logical deletion

[nvollmar]

References #557

[ignasi35]

Thanks @nvollmar for puting this in motion.

This is a rather important behavioral change so, IMHO it should be prominently explained on the docs and release notes. Maybe even justifies the bump of the MAJOR version of the plugin (as a user I'd be very pissed if a PATCH release included such a change sneakily).

Would you be able to review the docs for places where delete is mentioned and amend them? Thanks!

[ignasi35]

In general, SQL queries must still filter by logicalDeletion.enable = false but maybe when the flag to hard-delete is enabled we can remove the filter by journal.deleted=false criteria from the SLQ statement. Not on this PR though, that could be a future improvement.

[ignasi35]

Do we already have documentation on the safest procedure to move from logicalDeletion.enable = false to logicalDeletion.enable = true (and back in case of a rollback)? Maybe for a separate PR too.

[nvollmar]

I agree that this would be quite a significant change that should be mentioned prominently in the release notes. As mentioned in Issue #557, this feature is deprecated and warned. If a major release is warranted, then the feature might also be removed at the same time to clean everything up.

I didn't find any relevant mentioning of deletion in the docs.

[nvollmar]

There is no mentioning of logical deletion anywhere in the docs folder of the project

[ignasi35]

Thanks for checking.

Maybe, since there's already a deprecation warning and changing the default value of the setting already requires a MAJOR version bump, maybe we could just drop the setting and the feature altogether so users migrating to that major version will have to adopt the new behavior.

[nvollmar]

I've added the removal of this feature to this PR

[ignasi35]

Nice! thanks @nvollmar

[octonato]

Thanks @nvollmar

I'm ok to remove it since deprecated for quite some time. I don't think we will need a 6.0 version. I think it's ok if removed in 5.1

[octonato]

@nvollmar, we will need MiMa exclusions.

[nvollmar]

@octonato Added mima excludes

[patriknw]

LGTM

[patriknw]

@nvollmar sorry for the delay here. Would you be able to fix the conflicts? I think this is ready to be merged after that.

[patriknw]

@nvollmar OracleScalaEventAdapterTest and a few other Oracle tests are failing here. Do you think it can be related to the changes in this PR? I have re-run it at least twice.

Also ran it successfully on master in #672

[patriknw]

Note that the failing job is running with some custom parameters:

sbt "++2.13.4 It/testOnly akka.persistence.jdbc.integration.Oracle*"   -Djdbc-journal.dao=akka.persistence.jdbc.journal.dao.legacy.ByteArrayJournalDao -Djdbc-snapshot-store.dao=akka.persistence.jdbc.snapshot.dao.legacy.ByteArraySnapshotDao -Djdbc-read-journal.dao=akka.persistence.jdbc.query.dao.legacy.ByteArrayReadJournalDao

[nvollmar]

I'll have to backtrack the changes and see what might cause this

[patriknw]

I'm quite convinced that the Oracle case in akka.persistence.jdbc.query.dao.legacy.ByteArrayReadJournalDao was broken already before this change when using logicalDelete=false
That shows up now when the default has been changed.
I verified that in #672

I will merge this and then mark the failing tests as pending in a separate PR. I don't think we care that much about the legacy.ByteArrayReadJournalDao