storage/filesystem: implement missing functionality. (#110)

* storage/filesystem: added ObjectStorage Set.
* storage/filesystem: now passes all tests, except those specific to transactions.
* formats/config: Encoder now encodes subsections with no options.
* formats/config: add HasSubsection on Section.
* dotgit: add Ref method to get specific reference.

Author: smola

formats/config/common_test.go

@@ -13,7 +13,7 @@ type CommonSuite struct{}
 var _ = Suite(&CommonSuite{})
 
 func (s *CommonSuite) TestConfig_SetOption(c *C) {
-	obtained := New().SetOption("section", "", "key1", "value1")
+	obtained := New().SetOption("section", NoSubsection, "key1", "value1")
 	expected := &Config{
 		Sections: []*Section{
 			{
@@ -25,7 +25,7 @@ func (s *CommonSuite) TestConfig_SetOption(c *C) {
 		},
 	}
 	c.Assert(obtained, DeepEquals, expected)
-	obtained = obtained.SetOption("section", "", "key1", "value1")
+	obtained = obtained.SetOption("section", NoSubsection, "key1", "value1")
 	c.Assert(obtained, DeepEquals, expected)
 
 	obtained = New().SetOption("section", "subsection", "key1", "value1")
@@ -50,7 +50,7 @@ func (s *CommonSuite) TestConfig_SetOption(c *C) {
 }
 
 func (s *CommonSuite) TestConfig_AddOption(c *C) {
-	obtained := New().AddOption("section", "", "key1", "value1")
+	obtained := New().AddOption("section", NoSubsection, "key1", "value1")
 	expected := &Config{
 		Sections: []*Section{
 			{
@@ -66,10 +66,10 @@ func (s *CommonSuite) TestConfig_AddOption(c *C) {
 
 func (s *CommonSuite) TestConfig_RemoveSection(c *C) {
 	sect := New().
-		AddOption("section1", "", "key1", "value1").
-		AddOption("section2", "", "key1", "value1")
+		AddOption("section1", NoSubsection, "key1", "value1").
+		AddOption("section2", NoSubsection, "key1", "value1")
 	expected := New().
-		AddOption("section1", "", "key1", "value1")
+		AddOption("section1", NoSubsection, "key1", "value1")
 	c.Assert(sect.RemoveSection("other"), DeepEquals, sect)
 	c.Assert(sect.RemoveSection("section2"), DeepEquals, expected)
 }

formats/config/encoder.go

@@ -7,7 +7,7 @@ import (
 
 // An Encoder writes config files to an output stream.
 type Encoder struct {
-	io.Writer
+	w io.Writer
 }
 
 // NewEncoder returns a new encoder that writes to w.
@@ -18,21 +18,58 @@ func NewEncoder(w io.Writer) *Encoder {
 // Encode writes the config in git config format to the stream of the encoder.
 func (e *Encoder) Encode(cfg *Config) error {
 	for _, s := range cfg.Sections {
-		if len(s.Options) > 0 {
-			fmt.Fprintf(e, "[%s]\n", s.Name)
-			for _, o := range s.Options {
-				fmt.Fprintf(e, "\t%s = %s\n", o.Key, o.Value)
-			}
+		if err := e.encodeSection(s); err != nil {
+			return err
 		}
-		for _, ss := range s.Subsections {
-			if len(ss.Options) > 0 {
-				//TODO: escape
-				fmt.Fprintf(e, "[%s \"%s\"]\n", s.Name, ss.Name)
-				for _, o := range ss.Options {
-					fmt.Fprintf(e, "\t%s = %s\n", o.Key, o.Value)
-				}
-			}
+	}
+
+	return nil
+}
+
+func (e *Encoder) encodeSection(s *Section) error {
+	if len(s.Options) > 0 {
+		if err := e.printf("[%s]\n", s.Name); err != nil {
+			return err
+		}
+
+		if err := e.encodeOptions(s.Options); err != nil {
+			return err
+		}
+	}
+
+	for _, ss := range s.Subsections {
+		if err := e.encodeSubsection(s.Name, ss); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (e *Encoder) encodeSubsection(sectionName string, s *Subsection) error {
+	//TODO: escape
+	if err := e.printf("[%s \"%s\"]\n", sectionName, s.Name); err != nil {
+		return err
+	}
+
+	if err := e.encodeOptions(s.Options); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (e *Encoder) encodeOptions(opts Options) error {
+	for _, o := range opts {
+		if err := e.printf("\t%s = %s\n", o.Key, o.Value); err != nil {
+			return err
 		}
 	}
+
 	return nil
 }
+
+func (e *Encoder) printf(msg string, args ...interface{}) error {
+	_, err := fmt.Fprintf(e.w, msg, args...)
+	return err
+}

formats/config/section.go

@@ -74,3 +74,13 @@ func (s *Section) Subsection(name string) *Subsection {
 	s.Subsections = append(s.Subsections, ss)
 	return ss
 }
+
+func (s *Section) HasSubsection(name string) bool {
+	for _, ss := range s.Subsections {
+		if ss.IsName(name) {
+			return true
+		}
+	}
+
+	return false
+}

storage/filesystem/config.go

@@ -24,12 +24,12 @@ func (c *ConfigStorage) Remote(name string) (*config.RemoteConfig, error) {
 		return nil, err
 	}
 
-	s := cfg.Section(remoteSection).Subsection(name)
-	if s == nil {
+	s := cfg.Section(remoteSection)
+	if !s.HasSubsection(name) {
 		return nil, config.ErrRemoteConfigNotFound
 	}
 
-	return parseRemote(s), nil
+	return parseRemote(s.Subsection(name)), nil
 }
 
 func (c *ConfigStorage) Remotes() ([]*config.RemoteConfig, error) {
@@ -48,14 +48,20 @@ func (c *ConfigStorage) Remotes() ([]*config.RemoteConfig, error) {
 }
 
 func (c *ConfigStorage) SetRemote(r *config.RemoteConfig) error {
+	if err := r.Validate(); err != nil {
+		return err
+	}
+
 	cfg, err := c.read()
 	if err != nil {
 		return err
 	}
 
 	s := cfg.Section(remoteSection).Subsection(r.Name)
 	s.Name = r.Name
-	s.SetOption(urlKey, r.URL)
+	if r.URL != "" {
+		s.SetOption(urlKey, r.URL)
+	}
 	s.RemoveOption(fetchKey)
 	for _, rs := range r.Fetch {
 		s.AddOption(fetchKey, rs.String())
@@ -103,15 +109,14 @@ func (c *ConfigStorage) write(cfg *gitconfig.Config) error {
 		return err
 	}
 
-	defer f.Close()
-
 	e := gitconfig.NewEncoder(f)
 	err = e.Encode(cfg)
 	if err != nil {
+		f.Close()
 		return err
 	}
 
-	return nil
+	return f.Close()
 }
 
 func parseRemote(s *gitconfig.Subsection) *config.RemoteConfig {

storage/filesystem/config_test.go

@@ -31,7 +31,7 @@ func (s *ConfigSuite) SetUpTest(c *C) {
 
 func (s *ConfigSuite) TestSetRemote(c *C) {
 	cfg := &ConfigStorage{s.dir}
-	err := cfg.SetRemote(&config.RemoteConfig{Name: "foo"})
+	err := cfg.SetRemote(&config.RemoteConfig{Name: "foo", URL: "foo"})
 	c.Assert(err, IsNil)
 
 	remote, err := cfg.Remote("foo")

storage/filesystem/internal/dotgit/dotgit.go

@@ -215,6 +215,27 @@ func (d *DotGit) Refs() ([]*core.Reference, error) {
 	return refs, nil
 }
 
+// Ref returns the reference for a given reference name.
+func (d *DotGit) Ref(name core.ReferenceName) (*core.Reference, error) {
+	ref, err := d.readReferenceFile(".", name.String())
+	if err == nil {
+		return ref, nil
+	}
+
+	refs, err := d.Refs()
+	if err != nil {
+		return nil, err
+	}
+
+	for _, ref := range refs {
+		if ref.Name() == name {
+			return ref, nil
+		}
+	}
+
+	return nil, core.ErrReferenceNotFound
+}
+
 func (d *DotGit) addRefsFromPackedRefs(refs *[]*core.Reference) (err error) {
 	f, err := d.fs.Open(packedRefsPath)
 	if err != nil {

storage/filesystem/internal/dotgit/dotgit_test.go

@@ -44,6 +44,12 @@ func (s *SuiteDotGit) TestSetRefs(c *C) {
 
 	c.Assert(err, IsNil)
 
+	err = dir.SetRef(core.NewReferenceFromStrings(
+		"bar",
+		"e8d3ffab552895c19b9fcf7aa264d277cde33881",
+	))
+	c.Assert(err, IsNil)
+
 	refs, err := dir.Refs()
 	c.Assert(err, IsNil)
 	c.Assert(refs, HasLen, 2)
@@ -55,6 +61,25 @@ func (s *SuiteDotGit) TestSetRefs(c *C) {
 	ref = findReference(refs, "refs/heads/symbolic")
 	c.Assert(ref, NotNil)
 	c.Assert(ref.Target().String(), Equals, "refs/heads/foo")
+
+	ref = findReference(refs, "bar")
+	c.Assert(ref, IsNil)
+
+	ref, err = dir.Ref("refs/heads/foo")
+	c.Assert(err, IsNil)
+	c.Assert(ref, NotNil)
+	c.Assert(ref.Hash().String(), Equals, "e8d3ffab552895c19b9fcf7aa264d277cde33881")
+
+	ref, err = dir.Ref("refs/heads/symbolic")
+	c.Assert(err, IsNil)
+	c.Assert(ref, NotNil)
+	c.Assert(ref.Target().String(), Equals, "refs/heads/foo")
+
+	ref, err = dir.Ref("bar")
+	c.Assert(err, IsNil)
+	c.Assert(ref, NotNil)
+	c.Assert(ref.Hash().String(), Equals, "e8d3ffab552895c19b9fcf7aa264d277cde33881")
+
 }
 
 func (s *SuiteDotGit) TestRefsFromPackedRefs(c *C) {

storage/filesystem/object.go

@@ -1,7 +1,6 @@
 package filesystem
 
 import (
-	"fmt"
 	"io"
 	"os"
 
@@ -82,11 +81,35 @@ func (s *ObjectStorage) Writer() (io.WriteCloser, error) {
 	return w, nil
 }
 
-// Set adds a new object to the storage. As this functionality is not
-// yet supported, this method always returns a "not implemented yet"
-// error an zero hash.
-func (s *ObjectStorage) Set(core.Object) (core.Hash, error) {
-	return core.ZeroHash, fmt.Errorf("set - not implemented yet")
+// Set adds a new object to the storage.
+func (s *ObjectStorage) Set(o core.Object) (core.Hash, error) {
+	if o.Type() == core.OFSDeltaObject || o.Type() == core.REFDeltaObject {
+		return core.ZeroHash, core.ErrInvalidType
+	}
+
+	ow, err := s.dir.NewObject()
+	if err != nil {
+		return core.ZeroHash, err
+	}
+
+	defer ow.Close()
+
+	or, err := o.Reader()
+	if err != nil {
+		return core.ZeroHash, err
+	}
+
+	defer or.Close()
+
+	if err := ow.WriteHeader(o.Type(), o.Size()); err != nil {
+		return core.ZeroHash, err
+	}
+
+	if _, err := io.Copy(ow, or); err != nil {
+		return core.ZeroHash, err
+	}
+
+	return o.Hash(), nil
 }
 
 // Get returns the object with the given hash, by searching for it in
@@ -228,26 +251,18 @@ func (s *ObjectStorage) buildPackfileIters(
 	return iters, nil
 }
 
+// Begin opens a new transaction. However, this implementation is not
+// transactional, so Commit and Rollback have no effect.
 func (o *ObjectStorage) Begin() core.TxObjectStorage {
-	return &TxObjectStorage{}
-}
-
-type TxObjectStorage struct{}
-
-func (tx *TxObjectStorage) Set(obj core.Object) (core.Hash, error) {
-	return core.ZeroHash, fmt.Errorf("tx.Set - not implemented yet")
-}
-
-func (tx *TxObjectStorage) Get(core.ObjectType, core.Hash) (core.Object, error) {
-	return nil, fmt.Errorf("tx.Get - not implemented yet")
+	return o
 }
 
-func (tx *TxObjectStorage) Commit() error {
-	return fmt.Errorf("tx.Commit - not implemented yet")
+func (tx *ObjectStorage) Commit() error {
+	return nil
 }
 
-func (tx *TxObjectStorage) Rollback() error {
-	return fmt.Errorf("tx.Rollback - not implemented yet")
+func (tx *ObjectStorage) Rollback() error {
+	return nil
 }
 
 type index map[core.Hash]int64