refactor: extract KeyValueStore to common #3621
There are no files selected for viewing
This file contains hidden or 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,11 @@ | ||
| ALTER TABLE validator_internal_config | ||
| RENAME TO key_value_store; | ||
|
|
||
| ALTER TABLE key_value_store | ||
| RENAME COLUMN config_key TO key; | ||
|
|
||
| ALTER TABLE key_value_store | ||
| RENAME COLUMN config_value TO value; | ||
|
|
||
| ALTER TABLE key_value_store | ||
| RENAME CONSTRAINT uc_validator_internal_config TO uc_key_value_store; |
This file contains hidden or 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,72 @@ | ||
| // Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package org.lfdecentralizedtrust.splice.store | ||
|
|
||
| import cats.data.OptionT | ||
| import cats.implicits.catsSyntaxOptionId | ||
| import com.digitalasset.canton.lifecycle.CloseContext | ||
| import com.digitalasset.canton.logging.{ErrorLoggingContext, NamedLoggerFactory, NamedLogging} | ||
| import com.digitalasset.canton.resource.DbStorage | ||
| import com.digitalasset.canton.tracing.TraceContext | ||
| import io.circe.{Decoder, Encoder} | ||
| import org.lfdecentralizedtrust.splice.store.db.{DbKeyValueStore, StoreDescriptor} | ||
|
|
||
| import scala.concurrent.{ExecutionContext, Future} | ||
|
|
||
| trait KeyValueStore extends NamedLogging { | ||
|
|
||
| def setValue[T](key: String, value: T)(implicit | ||
| tc: TraceContext, | ||
| encoder: Encoder[T], | ||
| ): Future[Unit] | ||
|
|
||
| def getValue[T]( | ||
| key: String | ||
| )(implicit tc: TraceContext, decoder: Decoder[T]): OptionT[Future, Decoder.Result[T]] | ||
|
|
||
| def deleteKey(key: String)(implicit | ||
| tc: TraceContext | ||
| ): Future[Unit] | ||
|
|
||
| def readValueAndLogOnDecodingFailure[T: Decoder]( | ||
| key: String | ||
| )(implicit tc: TraceContext, ec: ExecutionContext): OptionT[Future, T] = { | ||
| getValue[T](key) | ||
| .subflatMap( | ||
| _.fold( | ||
| { failure => | ||
| logger.warn(s"Failed to decode $key from the db $failure") | ||
| None | ||
| }, | ||
| _.some, | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| } | ||
|
|
||
| object KeyValueStore { | ||
|
|
||
| def apply( | ||
| descriptor: StoreDescriptor, | ||
| storage: DbStorage, | ||
| loggerFactory: NamedLoggerFactory, | ||
| )(implicit | ||
| ec: ExecutionContext, | ||
| lc: ErrorLoggingContext, | ||
| cc: CloseContext, | ||
| tc: TraceContext, | ||
| ): Future[KeyValueStore] = { | ||
| storage match { | ||
| case storage: DbStorage => | ||
| DbKeyValueStore( | ||
| descriptor, | ||
| storage, | ||
| loggerFactory, | ||
| ) | ||
|
|
||
| case storageType => throw new RuntimeException(s"Unsupported storage type $storageType") | ||
| } | ||
| } | ||
| } |
This file contains hidden or 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 hidden or 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,133 @@ | ||
| // Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package org.lfdecentralizedtrust.splice.store | ||
|
|
||
| import com.digitalasset.canton.lifecycle.FutureUnlessShutdown | ||
| import com.digitalasset.canton.resource.DbStorage | ||
| import com.digitalasset.canton.tracing.TraceContext | ||
| import com.digitalasset.canton.HasExecutionContext | ||
| import org.lfdecentralizedtrust.splice.store.db.{SplicePostgresTest, StoreDescriptor} | ||
| import org.scalatest.matchers.should.Matchers | ||
|
|
||
| class KeyValueStoreTest | ||
| extends StoreTest | ||
| with Matchers | ||
| with HasExecutionContext | ||
| with SplicePostgresTest { | ||
| private val storeDescriptor = | ||
| StoreDescriptor( | ||
| version = 1, | ||
| name = "DbMultiDomainAcsStoreTest", | ||
| party = dsoParty, | ||
| participant = mkParticipantId("participant"), | ||
| key = Map(), | ||
| ) | ||
| private val storeDescriptor2 = | ||
| StoreDescriptor( | ||
| version = 2, | ||
| name = "DbMultiDomainAcsStoreTest", | ||
| party = dsoParty, | ||
| participant = mkParticipantId("participant"), | ||
| key = Map(), | ||
| ) | ||
|
|
||
| private def mkStore(descriptor: StoreDescriptor) = KeyValueStore( | ||
| descriptor, | ||
| storage, | ||
| loggerFactory, | ||
| ) | ||
|
|
||
| "KeyValueStore" should { | ||
|
|
||
| val configKey = "key1" | ||
| val configValue = "jsonPayload1" | ||
| val otherKey = "key2" | ||
| val otherValue = "jsonPayload2" | ||
|
|
||
| "set and get a payload successfully" in { | ||
|
Contributor
There was a problem hiding this comment. Suggestion for one more test: create two stores with different descriptors, check that they don't affect each others data.
Contributor
Author
There was a problem hiding this comment. 👍 |
||
| for { | ||
| store <- mkStore(storeDescriptor) | ||
| _ <- store.setValue(configKey, configValue) | ||
| retrievedValue <- store.getValue[String](configKey).value | ||
| } yield { | ||
| retrievedValue.value.value shouldBe configValue | ||
| } | ||
| } | ||
|
|
||
| "return None for a non-existent key" in { | ||
| for { | ||
| store <- mkStore(storeDescriptor) | ||
| retrievedValue <- store.getValue[String]("non-existent-key").value | ||
| } yield { | ||
| retrievedValue shouldBe None | ||
| } | ||
| } | ||
|
|
||
| "update an existing payload" in { | ||
| for { | ||
| store <- mkStore(storeDescriptor) | ||
| _ <- store.setValue(configKey, configValue) | ||
| _ <- store.setValue(configKey, otherValue) | ||
| retrievedValue <- store.getValue[String](configKey).value | ||
| } yield { | ||
| retrievedValue.value.value shouldBe otherValue | ||
| } | ||
| } | ||
|
|
||
| "handle multiple different keys independently" in { | ||
| for { | ||
| store <- mkStore(storeDescriptor) | ||
| _ <- store.setValue(configKey, configValue) | ||
| _ <- store.setValue(otherKey, otherValue) | ||
|
|
||
| configKeyValue <- store.getValue[String](configKey).value | ||
| otherKeyValue <- store.getValue[String](otherKey).value | ||
| } yield { | ||
| configKeyValue.value.value shouldBe configValue | ||
| otherKeyValue.value.value shouldBe otherValue | ||
| } | ||
| } | ||
|
|
||
| "delete single key" in { | ||
| for { | ||
| store <- mkStore(storeDescriptor) | ||
| _ <- store.setValue(configKey, configValue) | ||
| _ <- store.setValue(otherKey, otherValue) | ||
|
|
||
| _ <- store.deleteKey(configKey) | ||
| configKeyValue <- store.getValue[String](configKey).value | ||
| otherKeyValue <- store.getValue[String](otherKey).value | ||
| } yield { | ||
| configKeyValue shouldBe None | ||
| otherKeyValue.value.value shouldBe otherValue | ||
| } | ||
| } | ||
|
|
||
| "different storeDescriptors should not interfere" in { | ||
| for { | ||
| store1 <- mkStore(storeDescriptor) | ||
| store2 <- mkStore(storeDescriptor2) | ||
| _ <- store1.setValue("key", "value1") | ||
| _ <- store2.setValue("key", "value2") | ||
| _ <- store2.setValue("key2", "something") | ||
|
|
||
| k1s1 <- store1.getValue[String]("key").value | ||
| k2s1 <- store1.getValue[String]("key2").value | ||
|
|
||
| k1s2 <- store2.getValue[String]("key").value | ||
| k2s2 <- store2.getValue[String]("key2").value | ||
| } yield { | ||
| k1s1.value.value shouldBe "value1" | ||
| k2s1 shouldBe None | ||
| k1s2.value.value shouldBe "value2" | ||
| k2s2.value.value shouldBe "something" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| override protected def cleanDb( | ||
| storage: DbStorage | ||
| )(implicit traceContext: TraceContext): FutureUnlessShutdown[?] = | ||
| resetAllAppTables(storage) | ||
| } | ||
This file contains hidden or 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
Oops, something went wrong.
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.
changing the table name, what about existing data? (does this matter for the validator store version?)
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.
We're only renaming a table and some columns/constraints, that shouldn't affect existing data, no?
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.
The table itself is also renamed accordingly in flyway, so data would be preserved
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.
thanks missed that (flyway)