-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for querying last known sequence number by persistenceId. #267
Merged
pjfanning
merged 8 commits into
apache:main
from
janjaali:current-last-known-sequence-number
Feb 16, 2025
Merged
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
b4b4308
Add support for querying last known sequence number by persistenceId.
janjaali 49b7262
Align to existing code style.
janjaali b033218
Update license headers for new files.
janjaali f0cc6c5
Align to existing code style.
janjaali ce15d5e
Remove marker trait.
janjaali 9609a8f
mima issue
pjfanning 02197f8
Create CurrentLastKnownSequenceNumberByPersistenceIdTest.scala
pjfanning 711145f
Return java.lang.Long value in javadsl.
janjaali File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
19 changes: 19 additions & 0 deletions
19
core/src/main/mima-filters/1.1.x.backwards.excludes/LastKnownSequenceNumber.excludes
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
# Licensed to the Apache Software Foundation (ASF) under one | ||
# or more contributor license agreements. See the NOTICE file | ||
# distributed with this work for additional information | ||
# regarding copyright ownership. The ASF licenses this file | ||
# to you under the Apache License, Version 2.0 (the | ||
# "License"); you may not use this file except in compliance | ||
# with the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, | ||
# software distributed under the License is distributed on an | ||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
# KIND, either express or implied. See the License for the | ||
# specific language governing permissions and limitations | ||
# under the License. | ||
|
||
# https://github.com/apache/pekko-persistence-jdbc/pull/267 | ||
ProblemFilters.exclude[ReversedMissingMethodProblem]("org.apache.pekko.persistence.jdbc.query.dao.ReadJournalDao.lastPersistenceIdSequenceNumber") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58 changes: 58 additions & 0 deletions
58
...ache/pekko/persistence/jdbc/query/CurrentLastKnownSequenceNumberByPersistenceIdTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* contributor license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright ownership. | ||
* The ASF licenses this file to You under the Apache License, Version 2.0 | ||
* (the "License"); you may not use this file except in compliance with | ||
* the License. You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package org.apache.pekko.persistence.jdbc.query | ||
|
||
import org.scalatest.concurrent.ScalaFutures | ||
|
||
abstract class CurrentLastKnownSequenceNumberByPersistenceIdTest(config: String) extends QueryTestSpec(config) | ||
with ScalaFutures { | ||
|
||
it should "return None for unknown persistenceId" in withActorSystem { implicit system => | ||
val journalOps = new ScalaJdbcReadJournalOperations(system) | ||
|
||
journalOps | ||
.currentLastKnownSequenceNumberByPersistenceId("unknown") | ||
.futureValue shouldBe None | ||
} | ||
|
||
it should "return last sequence number for known persistenceId" in withActorSystem { implicit system => | ||
val journalOps = new ScalaJdbcReadJournalOperations(system) | ||
|
||
withTestActors() { (actor1, _, _) => | ||
actor1 ! 1 | ||
actor1 ! 2 | ||
actor1 ! 3 | ||
actor1 ! 4 | ||
|
||
eventually { | ||
journalOps | ||
.currentLastKnownSequenceNumberByPersistenceId("my-1") | ||
.futureValue shouldBe Some(4) | ||
|
||
// Just ensuring that query targets the correct persistenceId. | ||
journalOps | ||
.currentLastKnownSequenceNumberByPersistenceId("my-2") | ||
.futureValue shouldBe None | ||
} | ||
} | ||
} | ||
} | ||
|
||
class H2ScalaCurrentLastKnownSequenceNumberByPersistenceIdTest | ||
extends CurrentLastKnownSequenceNumberByPersistenceIdTest("h2-shared-db-application.conf") | ||
with H2Cleaner |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
36 changes: 36 additions & 0 deletions
36
...ekko/persistence/jdbc/integration/CurrentLastKnownSequenceNumberByPersistenceIdTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* | ||
* Licensed to the Apache Software Foundation (ASF) under one or more | ||
* license agreements; and to You under the Apache License, version 2.0: | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* This file is part of the Apache Pekko project, which was derived from Akka. | ||
*/ | ||
|
||
package org.apache.pekko.persistence.jdbc.integration | ||
|
||
import org.apache.pekko.persistence.jdbc.query.{ | ||
CurrentLastKnownSequenceNumberByPersistenceIdTest, | ||
MysqlCleaner, | ||
OracleCleaner, | ||
PostgresCleaner, | ||
SqlServerCleaner | ||
} | ||
|
||
// Note: these tests use the shared-db configs, the test for all (so not only current) events use the regular db config | ||
|
||
class PostgresScalaCurrentLastKnownSequenceNumberByPersistenceIdTest | ||
extends CurrentLastKnownSequenceNumberByPersistenceIdTest("postgres-shared-db-application.conf") | ||
with PostgresCleaner | ||
|
||
class MySQLScalaCurrentLastKnownSequenceNumberByPersistenceIdTest | ||
extends CurrentLastKnownSequenceNumberByPersistenceIdTest("mysql-shared-db-application.conf") | ||
with MysqlCleaner | ||
|
||
class OracleScalaCurrentLastKnownSequenceNumberByPersistenceIdTest | ||
extends CurrentLastKnownSequenceNumberByPersistenceIdTest("oracle-shared-db-application.conf") | ||
with OracleCleaner | ||
|
||
class SqlServerScalaCurrentLastKnownSequenceNumberByPersistenceIdTest | ||
extends CurrentLastKnownSequenceNumberByPersistenceIdTest("sqlserver-shared-db-application.conf") | ||
with SqlServerCleaner |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this should be java.lang.Long instead of scala.Long? scala.Long acts as an alias for the Java long primitive which is not the same as java.lang.Long. java.lang.Long is nullable but I still prefer the Optional wrapper.
wdyt @janjaali @ptrdom @raboof @He-Pin ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was considering
scala.Long
as the better suited one here, since theEventEnvelope
itself - which is the return type of multiple other methods in this class - carries the offset and specially the sequenceNr - the attribute that this method is actually interested in - as scala.Long.The Optional wrapper was "inspired" by the
GetObjectResult
for theDurableStateStore
javadsl.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick search in Pekko repository gives mixed results, some
javadsl
classes stick tojava.lang.Long
quite well, others do not. My guess is thatjavadsl
should usejava.lang.Long
, it is just difficult to enforce, because there is no automatic linting for it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that Pekko is inconsistent but I would prefer that we use java.lang.Long here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
java.lang.Long
in 711145f. I wasn't able to importjava.lang.Long
in top level sincescala.Long
already used in other query methods.