diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go index 682d3700..c159e004 100644 --- a/cmd/syncthing/gui.go +++ b/cmd/syncthing/gui.go @@ -361,8 +361,9 @@ func restPostConfig(m *model.Model, w http.ResponseWriter, r *http.Request) { // Activate and save + newCfg.Location = cfg.Location + newCfg.Save() cfg = newCfg - saveConfig() } } diff --git a/cmd/syncthing/main.go b/cmd/syncthing/main.go index 61f3d678..70ec15d6 100644 --- a/cmd/syncthing/main.go +++ b/cmd/syncthing/main.go @@ -33,7 +33,6 @@ import ( "github.com/syncthing/syncthing/files" "github.com/syncthing/syncthing/logger" "github.com/syncthing/syncthing/model" - "github.com/syncthing/syncthing/osutil" "github.com/syncthing/syncthing/protocol" "github.com/syncthing/syncthing/upgrade" "github.com/syncthing/syncthing/upnp" @@ -310,21 +309,14 @@ func syncthingMain() { // Prepare to be able to save configuration cfgFile := filepath.Join(confDir, "config.xml") - go saveConfigLoop(cfgFile) var myName string // Load the configuration file, if it exists. // If it does not, create a template. - cf, err := os.Open(cfgFile) + cfg, err = config.Load(cfgFile, myID) if err == nil { - // Read config.xml - cfg, err = config.Load(cf, myID) - if err != nil { - l.Fatalln(err) - } - cf.Close() myCfg := cfg.GetNodeConfiguration(myID) if myCfg == nil || myCfg.Name == "" { myName, _ = os.Hostname() @@ -336,7 +328,7 @@ func syncthingMain() { myName, _ = os.Hostname() defaultRepo := filepath.Join(getHomeDir(), "Sync") - cfg, err = config.Load(nil, myID) + cfg = config.New(cfgFile, myID) cfg.Repositories = []config.RepositoryConfiguration{ { ID: "default", @@ -361,7 +353,7 @@ func syncthingMain() { l.FatalErr(err) cfg.Options.ListenAddress = []string{fmt.Sprintf("0.0.0.0:%d", port)} - saveConfig() + cfg.Save() l.Infof("Edit %s to taste or use the GUI\n", cfgFile) } @@ -745,40 +737,6 @@ func shutdown() { stop <- exitSuccess } -var saveConfigCh = make(chan struct{}) - -func saveConfigLoop(cfgFile string) { - for _ = range saveConfigCh { - fd, err := os.Create(cfgFile + ".tmp") - if err != nil { - l.Warnln("Saving config:", err) - continue - } - - err = config.Save(fd, cfg) - if err != nil { - l.Warnln("Saving config:", err) - fd.Close() - continue - } - - err = fd.Close() - if err != nil { - l.Warnln("Saving config:", err) - continue - } - - err = osutil.Rename(cfgFile+".tmp", cfgFile) - if err != nil { - l.Warnln("Saving config:", err) - } - } -} - -func saveConfig() { - saveConfigCh <- struct{}{} -} - func listenConnect(myID protocol.NodeID, m *model.Model, tlsCfg *tls.Config) { var conns = make(chan *tls.Conn) diff --git a/config/config.go b/config/config.go index ec61c266..12684538 100644 --- a/config/config.go +++ b/config/config.go @@ -8,7 +8,6 @@ package config import ( "encoding/xml" "fmt" - "io" "os" "reflect" "sort" @@ -16,12 +15,14 @@ import ( "code.google.com/p/go.crypto/bcrypt" "github.com/syncthing/syncthing/logger" + "github.com/syncthing/syncthing/osutil" "github.com/syncthing/syncthing/protocol" ) var l = logger.DefaultLogger type Configuration struct { + Location string `xml:"-" json:"-"` Version int `xml:"version,attr" default:"3"` Repositories []RepositoryConfiguration `xml:"repository"` Nodes []NodeConfiguration `xml:"node"` @@ -227,14 +228,38 @@ func fillNilSlices(data interface{}) error { return nil } -func Save(wr io.Writer, cfg Configuration) error { - e := xml.NewEncoder(wr) - e.Indent("", " ") - err := e.Encode(cfg) +func (cfg *Configuration) Save() error { + fd, err := os.Create(cfg.Location + ".tmp") if err != nil { + l.Warnln("Saving config:", err) return err } - _, err = wr.Write([]byte("\n")) + + e := xml.NewEncoder(fd) + e.Indent("", " ") + err = e.Encode(cfg) + if err != nil { + fd.Close() + return err + } + _, err = fd.Write([]byte("\n")) + + if err != nil { + l.Warnln("Saving config:", err) + fd.Close() + return err + } + + err = fd.Close() + if err != nil { + l.Warnln("Saving config:", err) + return err + } + + err = osutil.Rename(cfg.Location+".tmp", cfg.Location) + if err != nil { + l.Warnln("Saving config:", err) + } return err } @@ -252,18 +277,7 @@ func uniqueStrings(ss []string) []string { return us } -func Load(rd io.Reader, myID protocol.NodeID) (Configuration, error) { - var cfg Configuration - - setDefaults(&cfg) - setDefaults(&cfg.Options) - setDefaults(&cfg.GUI) - - var err error - if rd != nil { - err = xml.NewDecoder(rd).Decode(&cfg) - } - +func (cfg *Configuration) prepare(myID protocol.NodeID) { fillNilSlices(&cfg.Options) cfg.Options.ListenAddress = uniqueStrings(cfg.Options.ListenAddress) @@ -312,17 +326,17 @@ func Load(rd io.Reader, myID protocol.NodeID) (Configuration, error) { // Upgrade to v2 configuration if appropriate if cfg.Version == 1 { - convertV1V2(&cfg) + convertV1V2(cfg) } // Upgrade to v3 configuration if appropriate if cfg.Version == 2 { - convertV2V3(&cfg) + convertV2V3(cfg) } // Upgrade to v4 configuration if appropriate if cfg.Version == 3 { - convertV3V4(&cfg) + convertV3V4(cfg) } // Hash old cleartext passwords @@ -368,6 +382,39 @@ func Load(rd io.Reader, myID protocol.NodeID) (Configuration, error) { n.Addresses = []string{"dynamic"} } } +} + +func New(location string, myID protocol.NodeID) Configuration { + var cfg Configuration + + cfg.Location = location + + setDefaults(&cfg) + setDefaults(&cfg.Options) + setDefaults(&cfg.GUI) + + cfg.prepare(myID) + + return cfg +} + +func Load(location string, myID protocol.NodeID) (Configuration, error) { + var cfg Configuration + + cfg.Location = location + + setDefaults(&cfg) + setDefaults(&cfg.Options) + setDefaults(&cfg.GUI) + + fd, err := os.Open(location) + if err != nil { + return Configuration{}, err + } + err = xml.NewDecoder(fd).Decode(&cfg) + fd.Close() + + cfg.prepare(myID) return cfg, err } diff --git a/config/config_test.go b/config/config_test.go index 696baecb..6a6f8956 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -5,8 +5,6 @@ package config import ( - "bytes" - "io" "os" "reflect" "testing" @@ -40,10 +38,7 @@ func TestDefaultValues(t *testing.T) { UPnPRenewal: 30, } - cfg, err := Load(bytes.NewReader(nil), node1) - if err != io.EOF { - t.Error(err) - } + cfg := New("test", node1) if !reflect.DeepEqual(cfg.Options, expected) { t.Errorf("Default config differs;\n E: %#v\n A: %#v", expected, cfg.Options) @@ -51,84 +46,8 @@ func TestDefaultValues(t *testing.T) { } func TestNodeConfig(t *testing.T) { - v1data := []byte(` - - - -
a
- - -
b
- - -
a
- - -
b
- - - - true - 600 - - -`) - - v2data := []byte(` - - - - - - - - - - -
a
- - -
b
- - - 600 - - -`) - - v3data := []byte(` - - - - - - -
a
- - -
b
- - - 600 - -`) - - v4data := []byte(` - - - - - - -
a
- - -
b
- -`) - - for i, data := range [][]byte{v1data, v2data, v3data, v4data} { - cfg, err := Load(bytes.NewReader(data), node1) + for i, ver := range []string{"v1", "v2", "v3", "v4"} { + cfg, err := Load("testdata/"+ver+".xml", node1) if err != nil { t.Error(err) } @@ -181,14 +100,7 @@ func TestNodeConfig(t *testing.T) { } func TestNoListenAddress(t *testing.T) { - data := []byte(` - - - - -`) - - cfg, err := Load(bytes.NewReader(data), node1) + cfg, err := Load("testdata/nolistenaddress.xml", node1) if err != nil { t.Error(err) } @@ -200,26 +112,6 @@ func TestNoListenAddress(t *testing.T) { } func TestOverriddenValues(t *testing.T) { - data := []byte(` - - :23000 - false - syncthing.nym.se:22026 - false - false - 42123 - quux:3232 - 32 - 1234 - 6000 - false - false - 60 - 15 - - -`) - expected := OptionsConfiguration{ ListenAddress: []string{":23000"}, GlobalAnnServer: "syncthing.nym.se:22026", @@ -236,7 +128,7 @@ func TestOverriddenValues(t *testing.T) { UPnPRenewal: 15, } - cfg, err := Load(bytes.NewReader(data), node1) + cfg, err := Load("testdata/overridenvalues.xml", node1) if err != nil { t.Error(err) } @@ -247,19 +139,6 @@ func TestOverriddenValues(t *testing.T) { } func TestNodeAddressesDynamic(t *testing.T) { - data := []byte(` - - -
- - - - -
dynamic
- - -`) - name, _ := os.Hostname() expected := []NodeConfiguration{ { @@ -284,7 +163,7 @@ func TestNodeAddressesDynamic(t *testing.T) { }, } - cfg, err := Load(bytes.NewReader(data), node4) + cfg, err := Load("testdata/nodeaddressesdynamic.xml", node4) if err != nil { t.Error(err) } @@ -295,23 +174,6 @@ func TestNodeAddressesDynamic(t *testing.T) { } func TestNodeAddressesStatic(t *testing.T) { - data := []byte(` - - -
192.0.2.1
-
192.0.2.2
- - -
192.0.2.3:6070
-
[2001:db8::42]:4242
- - -
[2001:db8::44]:4444
-
192.0.2.4:6090
- - -`) - name, _ := os.Hostname() expected := []NodeConfiguration{ { @@ -333,7 +195,7 @@ func TestNodeAddressesStatic(t *testing.T) { }, } - cfg, err := Load(bytes.NewReader(data), node4) + cfg, err := Load("testdata/nodeaddressesstatic.xml", node4) if err != nil { t.Error(err) } @@ -344,18 +206,7 @@ func TestNodeAddressesStatic(t *testing.T) { } func TestVersioningConfig(t *testing.T) { - data := []byte(` - - - - - - - - - `) - - cfg, err := Load(bytes.NewReader(data), node4) + cfg, err := Load("testdata/versioningconfig.xml", node4) if err != nil { t.Error(err) } @@ -376,3 +227,67 @@ func TestVersioningConfig(t *testing.T) { t.Errorf("vc.Params differ;\n E: %#v\n A: %#v", expected, vc.Params) } } + +func TestNewSaveLoad(t *testing.T) { + path := "testdata/temp.xml" + os.Remove(path) + + exists := func(path string) bool { + _, err := os.Stat(path) + return err == nil + } + + cfg := New(path, node1) + + // To make the equality pass later + cfg.XMLName.Local = "configuration" + + if exists(path) { + t.Error(path, "exists") + } + + err := cfg.Save() + if err != nil { + t.Error(err) + } + if !exists(path) { + t.Error(path, "does not exist") + } + + cfg2, err := Load(path, node1) + if err != nil { + t.Error(err) + } + + if !reflect.DeepEqual(cfg, cfg2) { + t.Errorf("Configs are not equal;\n E: %#v\n A: %#v", cfg, cfg2) + } + + cfg.GUI.User = "test" + cfg.Save() + + cfg2, err = Load(path, node1) + if err != nil { + t.Error(err) + } + + if cfg2.GUI.User != "test" || !reflect.DeepEqual(cfg, cfg2) { + t.Errorf("Configs are not equal;\n E: %#v\n A: %#v", cfg, cfg2) + } + + os.Remove(path) +} + +func TestPrepare(t *testing.T) { + var cfg Configuration + + if cfg.Repositories != nil || cfg.Nodes != nil || cfg.Options.ListenAddress != nil { + t.Error("Expected nil") + } + + cfg.prepare(node1) + + if cfg.Repositories == nil || cfg.Nodes == nil || cfg.Options.ListenAddress == nil { + t.Error("Unexpected nil") + } +} diff --git a/config/testdata/nodeaddressesdynamic.xml b/config/testdata/nodeaddressesdynamic.xml new file mode 100755 index 00000000..fe506c83 --- /dev/null +++ b/config/testdata/nodeaddressesdynamic.xml @@ -0,0 +1,10 @@ + + +
+ + + + +
dynamic
+ + diff --git a/config/testdata/nodeaddressesstatic.xml b/config/testdata/nodeaddressesstatic.xml new file mode 100755 index 00000000..23c15928 --- /dev/null +++ b/config/testdata/nodeaddressesstatic.xml @@ -0,0 +1,14 @@ + + +
192.0.2.1
+
192.0.2.2
+ + +
192.0.2.3:6070
+
[2001:db8::42]:4242
+ + +
[2001:db8::44]:4444
+
192.0.2.4:6090
+ + diff --git a/config/testdata/nolistenaddress.xml b/config/testdata/nolistenaddress.xml new file mode 100755 index 00000000..ab9f87b0 --- /dev/null +++ b/config/testdata/nolistenaddress.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/config/testdata/overridenvalues.xml b/config/testdata/overridenvalues.xml new file mode 100755 index 00000000..034a662b --- /dev/null +++ b/config/testdata/overridenvalues.xml @@ -0,0 +1,18 @@ + + + :23000 + false + syncthing.nym.se:22026 + false + false + 42123 + quux:3232 + 32 + 1234 + 6000 + false + false + 60 + 15 + + diff --git a/config/testdata/v1.xml b/config/testdata/v1.xml new file mode 100755 index 00000000..2365effb --- /dev/null +++ b/config/testdata/v1.xml @@ -0,0 +1,20 @@ + + + +
a
+ + +
b
+ + +
a
+ + +
b
+ + + + true + 600 + + diff --git a/config/testdata/v2.xml b/config/testdata/v2.xml new file mode 100755 index 00000000..bfddd25e --- /dev/null +++ b/config/testdata/v2.xml @@ -0,0 +1,19 @@ + + + + + + + + + + +
a
+ + +
b
+ + + 600 + + diff --git a/config/testdata/v3.xml b/config/testdata/v3.xml new file mode 100755 index 00000000..0f527fae --- /dev/null +++ b/config/testdata/v3.xml @@ -0,0 +1,15 @@ + + + + + + +
a
+ + +
b
+ + + 600 + + diff --git a/config/testdata/v4.xml b/config/testdata/v4.xml new file mode 100755 index 00000000..73d6649d --- /dev/null +++ b/config/testdata/v4.xml @@ -0,0 +1,12 @@ + + + + + + +
a
+ + +
b
+ + diff --git a/config/testdata/versioningconfig.xml b/config/testdata/versioningconfig.xml new file mode 100755 index 00000000..c98be30b --- /dev/null +++ b/config/testdata/versioningconfig.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/model/model_test.go b/model/model_test.go index d7238cea..0a417fcf 100644 --- a/model/model_test.go +++ b/model/model_test.go @@ -266,7 +266,7 @@ func TestNodeRename(t *testing.T) { ClientVersion: "v0.9.4", } - cfg, _ := config.Load(nil, node1) + cfg := config.New("test", node1) cfg.Nodes = []config.NodeConfiguration{ { NodeID: node1,