Skip to content

Commit

Permalink
Addressed comments
Browse files Browse the repository at this point in the history
  • Loading branch information
bgaidioz committed Nov 26, 2024
1 parent 65418dc commit 9752521
Showing 1 changed file with 24 additions and 11 deletions.
35 changes: 24 additions & 11 deletions src/main/scala/com/rawlabs/das/server/DASSdkManager.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,28 +66,28 @@ class DASSdkManager(implicit settings: RawSettings) extends StrictLogging {
/**
* Registers a new DAS instance with the specified type and options.
*
* @param dasType The type of the DAS to register.
* @param options A map of options for configuring the DAS.
* @param dasType The type of the DAS to register.
* @param options A map of options for configuring the DAS.
* @param maybeDasId An optional DAS ID, if not provided a new one will be generated.
* @return The registered DAS ID.
*/
def registerDAS(dasType: String, options: Map[String, String], maybeDasId: Option[DASId] = None): DASId = {
// Start from the provided DAS ID, or create a new one.
val dasId = maybeDasId.getOrElse(DASId.newBuilder().setId(java.util.UUID.randomUUID().toString).build())
// Then make sure that the DAS is not already registered with a different config.
val config = DASConfig(dasType, options.filterKeys(!_.startsWith("das_")))
val config = DASConfig(dasType, stripOptions(options))
dasSdkconfigCacheLock.synchronized {
dasSdkConfigCache.get(dasId) match {
case Some(registeredConfig) => if (registeredConfig != config) {
logger.error(
s"DAS with ID $dasId is already registered. Registered configuration is $registeredConfig and new config is $config"
)
throw new IllegalArgumentException(s"DAS with id $dasId already registered")
}
logger.error(
s"DAS with ID $dasId is already registered. Registered configuration is $registeredConfig and new config is $config"
)
throw new IllegalArgumentException(s"DAS with id $dasId already registered")
}
case None => dasSdkConfigCache.put(dasId, config)
}
}
// If everything is fine at dasId/config level, create (or use previously cached instance) an SDK with the config.
// Everything is fine at dasId/config level. Create (or use previously cached instance) an SDK with the config.
dasSdkCache.get(config) // If the config didn't exist, that blocks until the new DAS is loaded
dasId
}
Expand All @@ -102,9 +102,12 @@ class DASSdkManager(implicit settings: RawSettings) extends StrictLogging {
dasSdkConfigCache.get(dasId) match {
case Some(config) =>
logger.debug(s"Unregistering DAS with ID: $dasId")
dasSdkCache.invalidate(config)
dasSdkConfigCache.remove(dasId)
logger.debug(s"DAS unregistered successfully with ID: $dasId")
if (!dasSdkConfigCache.values.exists(_ == config)) {
// It was the last DAS with this config, so invalidate the cache
dasSdkCache.invalidate(config)
}
case None => {
logger.warn(s"Tried to unregister DAS with ID: $dasId, but it was not found.")
}
Expand Down Expand Up @@ -193,7 +196,17 @@ class DASSdkManager(implicit settings: RawSettings) extends StrictLogging {
* @param dasId The DAS ID to retrieve.
* @return The DAS instance.
*/
private def getDASFromRemote(dasId: DASId): Option[DasConfig] = {
private def getDASFromRemote(dasId: DASId): Option[DASConfig] = {
None
}

/**
* Strips the DAS-specific options (das_*) from the provided options map. They aren't needed
* for the SDK instance creation and shouldn't be used as part of the cache key.
* @param options The options sent by the client.
* @return The same options with the DAS-specific entries removed.
*/
private def stripOptions(options: Map[String, String]): Map[String, String] = {
options.filterKeys(!_.startsWith("das_"))
}
}

0 comments on commit 9752521

Please sign in to comment.