paradigmatic / Configrity

Simple, immutable and flexible configuration library for scala.
Other
132 stars 19 forks source link

Add 'Configuration#detachAll' #19

Closed jvirtanen closed 11 years ago

jvirtanen commented 11 years ago

For each unique prefix in the configuration, it produces a new configuration containing the values whose keys have the common prefix. It returns a map from prefixes to configurations.

This is useful in the following scenario:

accounts {
  alice {
    group: admins
    name:  Alice
  }
  bob {
    group: users
    name:  Bob
  }
}

Here the account identifiers, alice and bob, should obviously not be hard-coded into the application. Configuration#detachAll on the accounts sub-configuration provides a convenient discovery mechanism for the configured accounts.

paradigmatic commented 11 years ago

Nice feature. Thank you very much for the contribution. Still not sure about the name. I m trying to avoid method with the same name but different behaviors. What do you think about detachAll ?

Another question: what do you think about having a return type of Map[String,Configuration] instead of Set[(String,Configuration)] ?

lalloni commented 11 years ago

The detach/attach metaphor goes only so far... ¿what are we detaching? maybe "children" might work.

trait Configuration {
  def detach(prefix: String): Configuration
  def attach(prefix: String, cfg:Configuration): Configuration
  def prefixes: Set[String]
  def children: Map[String, Configuration]
}

It doesn't feel really good, particularly because it is somewhat an arbitrary interpretation of what a prefix is: key.split(".").head

What if I use "_" or "-" instead of "."?

Maybe this belongs to client code, and configuration should only expose all its keys (already exposed with cfg.data.keys) and allow clients do the "prefix" discovery with client specific algorithms.

cfg.data.keys.filter(_.contains(".")).map(_.split(".").head).map(prefix => prefix -> cfg.detach(prefix)).toMap

I agree that specifically for some Formats this feature seems quite natural as is.

Or... maybe we could detach by something more flexible than a prefix String... for instance:

  def detachMatching(extractor: Regex): Map[String, Configuration] =
    (for {
      key ← data.keys
      val extractor(prefix) ← key
    } yield prefix → detach(prefix)).toMap

  def detachBy(selector: String ⇒ Option[String]): Map[String, Configuration] =
    (for {
      key ← data.keys
      prefix ← selector(key)
    } yield prefix → detach(prefix)).toMap

Then you could...

  val conf = Configuration("a.b" → 3, "a.c" → 4, "x" → 6)

  val dd1 = conf.detachMatching("""^([^.]+)""".r)

  println(dd1) // == Map(a -> Configuration(Map(b -> 3, c -> 4)))

  val dd2 = conf.detachBy(s ⇒ if (s.contains('.')) Some(s.split('.').head) else None)

  println(dd2) // == Map(a -> Configuration(Map(b -> 3, c -> 4)))
jvirtanen commented 11 years ago

@paradigmatic Thank you for your comments! detachAll sounds good, and Map is indeed more natural return type than Set. I have updated the pull request accordingly.

jvirtanen commented 11 years ago

@plalloni Thank you for your comments! I prefer "detachAll" to "children", because the operation applies only to subconfigurations. If the accounts above contained (unprefixed) keys in addition to subconfigurations (prefixed keys), the unprefixed keys would be ignored by the operation. Yet I would call them children of accounts.

Note that . is already well established as the prefix separator in Configrity, and the functionality of Configuration#detachAll is in line with Configuration#detach.

I'm not opposed into more flexible interpretations of "detach", but this pull request is solely based on the needs of an existing application. Until now, the implementation has been part of that application. I believe others, too, would benefit from it, especially as it feels to me to be in line with the spirit of Configrity.

paradigmatic commented 11 years ago

@plalloni Thanks a lot for your feedback Until now I considered that the dot . is a special character for keys to define the hierarchy (perhaps it is not stressed enough in the docs). It worked with all the current formats and I am not receiving requests to implement other formats although I am planning to add INI format soon (I am teaching a lot right now). However, I am very interested in the detachBy method because it can allow fine control and I will try to add it.

@jvirtanen Thanks for your update. I'm merging it.

At Everybody: I am very interested about Configrity usage. Can you explain me a bit in which kind of project do you use it ?