charm: allow empty string options

state/apiserver/client/client.go

 // Copyright 2013 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
 package client
 
 import (
         "errors"
         "fmt"
 
         "launchpad.net/juju-core/charm"
@@ skipping 129 matching lines @@
                 return err
         }
         ch, err := conn.PutCharm(curl, CharmStore, false)
         if err != nil {
                 return err
         }
         var settings charm.Settings
         if len(args.ConfigYAML) > 0 {
                 settings, err = ch.Config().ParseSettingsYAML([]byte(args.ConfigYAML), args.ServiceName)
         } else if len(args.Config) > 0 {
-                settings, err = ch.Config().ParseSettingsStrings(args.Config)
+                // Parse config in a compatile way (see function comment).
+                settings, err = parseSettingsCompatible(ch, args.Config)
         }
         if err != nil {
                 return err
         }
         _, err = conn.DeployService(juju.DeployServiceParams{
                 ServiceName:    args.ServiceName,
                 Charm:          ch,
                 NumUnits:       args.NumUnits,
                 ConfigSettings: settings,
                 Constraints:    args.Constraints,
@@ skipping 76 matching lines @@
         return service.UpdateConfigSettings(changes)
 }
 
 // serviceSetSettingsStrings updates the settings for the given service,
 // taking the configuration from a map of strings.
 func serviceSetSettingsStrings(service *state.Service, settings map[string]string) error {
         ch, _, err := service.Charm()
         if err != nil {
                 return err
         }
-        changes, err := ch.Config().ParseSettingsStrings(settings)
+        // Parse config in a compatile way (see function comment).

[axw1, 2013/09/11 08:37:42]

typo - s/compatile/compatible/

[mue, 2013/09/11 16:29:09]

Done.

+        changes, err := parseSettingsCompatible(ch, settings)
         if err != nil {
                 return err
         }
         return service.UpdateConfigSettings(changes)
 }
 
 // ServiceSetCharm sets the charm for a given service.
 func (c *Client) ServiceSetCharm(args params.ServiceSetCharm) error {
         service, err := c.api.state.Service(args.ServiceName)
         if err != nil {
@@ skipping 136 matching lines @@
 }
 
 // SetAnnotations stores annotations about a given entity.
 func (c *Client) SetAnnotations(args params.SetAnnotations) error {
         entity, err := c.findEntity(args.Tag)
         if err != nil {
                 return err
         }
         return entity.SetAnnotations(args.Pairs)
 }
+
+// parseSettingsCompatible parses setting strings in a way that is
+// campatible with the behavior before this CL based on the issue

[axw1, 2013/09/11 08:37:42]

s/campatible/compatible/

[mue, 2013/09/11 16:29:09]

Done.

+// http://pad.lv/1194945. Until then setting an option to an empty
+// string caused it to reset to the default value. We now allow
+// empty strings as actual values, but we want to preserve the API
+// behavior.
+func parseSettingsCompatible(ch *state.Charm, settings map[string]string) (charm.Settings, error) {
+        setSettings := map[string]string{}
+        unsetSettings := charm.Settings{}
+        // Split settings into those which set and those which unset a value.
+        for name, value := range settings {
+                if value == "" {
+                        unsetSettings[name] = nil
+                        continue
+                }
+                setSettings[name] = value
+        }
+        // Validate the settings.
+        changes, err := ch.Config().ParseSettingsStrings(setSettings)
+        if err != nil {
+                return nil, err
+        }
+        // Validate the unsettings and merge them into the changes.
+        unsetSettings, err = ch.Config().ValidateSettings(unsetSettings)
+        if err != nil {
+                return nil, err
+        }
+        for name := range unsetSettings {
+                changes[name] = nil
+        }
+        return changes, nil
+}