Prefer injected, typed config over Config

ignasi35 commented 6 years ago

AkkaDiscoveryServiceLocator uses the Config from the ActorSystem. Instead, it should receive a strongly typed custom configuration instance.

dwijnand commented 6 years ago

By "a strongly typed custom configuration instance" do you mean a custom case class, or one of those Scala wrappers of Config?

ignasi35 commented 6 years ago

By "a strongly typed custom configuration instance" do you mean a custom case class, or one of those Scala wrappers of Config?

A custom case class.

ignasi35 commented 5 years ago

How about mapping the settings to something like:

case class LagomAkkaDiscoveryConfigDefaults(portName: Option[String],
                                            portProtocol: Option[String],
                                            scheme: Option[String])

case class ServiceNameMapping(lookup: Option[String],
                              scheme: Option[String])

case class LagomAkkaDiscoveryConfig(defaults: LagomAkkaDiscoveryConfigDefaults,
                                    serviceNameMappings: Map[String, ServiceNameMapping],
                                    lookupTimeout: Duration)

(just a proposal, we should not use case classes because of the bin compat issues)

Then:

- private[lagom] class ServiceNameMapper(config: Config) {
+ private[lagom] class ServiceNameMapper(config: LagomAkkaDiscoveryConfig) {

- private[lagom] class AkkaDiscoveryHelper(config: Config, serviceDiscovery: ServiceDiscovery)(
+ private[lagom] class AkkaDiscoveryHelper(config: LagomAkkaDiscoveryConfig, serviceDiscovery: ServiceDiscovery)(

etc...

Most of the complexity in ServiceMapper would move away from that class into a separate Config loading function.

lagom / lagom-akka-discovery-service-locator

Prefer injected, typed config over Config #2