Split WriteStorage.Set to Create and Update, add typed errors

Signed-off-by: Dennis Marttinen <dennis@weave.works>

pkg/storage/mappedrawstorage.go

@@ -13,6 +13,11 @@ import (
 	"github.com/weaveworks/libgitops/pkg/util"
 )
 
+var (
+	// ErrNotTracked is returned when the requested resource wasn't found.
+	ErrNotTracked = fmt.Errorf("untracked object: %w", ErrNotFound)
+)
+
 // MappedRawStorage is an interface for RawStorages which store their
 // data in a flat/unordered directory format like manifest directories.
 type MappedRawStorage interface {
@@ -44,15 +49,15 @@ type GenericMappedRawStorage struct {
 	mux          *sync.Mutex
 }
 
-func (r *GenericMappedRawStorage) realPath(key ObjectKey) (path string, err error) {
+func (r *GenericMappedRawStorage) realPath(key ObjectKey) (string, error) {
 	r.mux.Lock()
 	path, ok := r.fileMappings[key]
 	r.mux.Unlock()
 	if !ok {
-		err = fmt.Errorf("GenericMappedRawStorage: %q not tracked", key)
+		return "", fmt.Errorf("GenericMappedRawStorage: cannot resolve %q: %w", key, ErrNotTracked)
 	}
 
-	return
+	return path, nil
 }
 
 func (r *GenericMappedRawStorage) Read(key ObjectKey) ([]byte, error) {
@@ -78,7 +83,7 @@ func (r *GenericMappedRawStorage) Write(key ObjectKey, content []byte) error {
 	// only write if the file is already known
 	file, err := r.realPath(key)
 	if err != nil {
-		return nil
+		return err
 	}
 
 	return ioutil.WriteFile(file, content, 0644)
@@ -117,21 +122,19 @@ func (r *GenericMappedRawStorage) List(kind KindKey) ([]ObjectKey, error) {
 }
 
 // This returns the modification time as a UnixNano string
-// If the file doesn't exist, return blank
+// If the file doesn't exist, return ErrNotFound
 func (r *GenericMappedRawStorage) Checksum(key ObjectKey) (s string, err error) {
 	file, err := r.realPath(key)
 	if err != nil {
 		return
 	}
 
-	var fi os.FileInfo
-	if r.Exists(key) {
-		if fi, err = os.Stat(file); err == nil {
-			s = strconv.FormatInt(fi.ModTime().UnixNano(), 10)
-		}
+	fi, err := os.Stat(file)
+	if err != nil {
+		return "", err
 	}
 
-	return
+	return strconv.FormatInt(fi.ModTime().UnixNano(), 10), nil
 }
 
 func (r *GenericMappedRawStorage) ContentType(key ObjectKey) (ct serializer.ContentType) {

pkg/storage/rawstorage.go

@@ -19,25 +19,29 @@ import (
 // store byte-encoded Objects (resources) in non-volatile
 // memory.
 type RawStorage interface {
-	// Read returns a resource's content based on key
+	// Read returns a resource's content based on key.
+	// If the resource does not exist, it returns ErrNotFound.
 	Read(key ObjectKey) ([]byte, error)
-	// Exists checks if the resource indicated by key exists
+	// Exists checks if the resource indicated by key exists.
 	Exists(key ObjectKey) bool
-	// Write writes the given content to the resource indicated by key
+	// Write writes the given content to the resource indicated by key.
+	// Error returns are implementation-specific.
 	Write(key ObjectKey, content []byte) error
-	// Delete deletes the resource indicated by key
+	// Delete deletes the resource indicated by key.
+	// If the resource does not exist, it returns ErrNotFound.
 	Delete(key ObjectKey) error
-	// List returns all matching object keys based on the given KindKey
+	// List returns all matching object keys based on the given KindKey.
 	List(key KindKey) ([]ObjectKey, error)
-	// Checksum returns a string checksum for the resource indicated by key
+	// Checksum returns a string checksum for the resource indicated by key.
+	// If the resource does not exist, it returns ErrNotFound.
 	Checksum(key ObjectKey) (string, error)
-	// ContentType returns the content type of the contents of the resource indicated by key
+	// ContentType returns the content type of the contents of the resource indicated by key.
 	ContentType(key ObjectKey) serializer.ContentType
 
-	// WatchDir returns the path for Watchers to watch changes in
+	// WatchDir returns the path for Watchers to watch changes in.
 	WatchDir() string
 	// GetKey retrieves the Key containing the virtual path based
-	// on the given physical file path returned by a Watcher
+	// on the given physical file path returned by a Watcher.
 	GetKey(path string) (ObjectKey, error)
 }
 
@@ -87,6 +91,11 @@ func (r *GenericRawStorage) Read(key ObjectKey) ([]byte, error) {
 		return nil, err
 	}
 
+	// Check if the resource indicated by key exists
+	if !r.Exists(key) {
+		return nil, ErrNotFound
+	}
+
 	return ioutil.ReadFile(r.keyPath(key))
 }
 
@@ -123,6 +132,11 @@ func (r *GenericRawStorage) Delete(key ObjectKey) error {
 		return err
 	}
 
+	// Check if the resource indicated by key exists
+	if !r.Exists(key) {
+		return ErrNotFound
+	}
+
 	return os.RemoveAll(path.Dir(r.keyPath(key)))
 }
 
@@ -146,22 +160,24 @@ func (r *GenericRawStorage) List(kind KindKey) ([]ObjectKey, error) {
 }
 
 // This returns the modification time as a UnixNano string
-// If the file doesn't exist, return blank
+// If the file doesn't exist, return ErrNotFound
 func (r *GenericRawStorage) Checksum(key ObjectKey) (s string, err error) {
 	// Validate GroupVersion first
 	if err := r.validateGroupVersion(key); err != nil {
 		return "", err
 	}
 
-	var fi os.FileInfo
+	// Check if the resource indicated by key exists
+	if !r.Exists(key) {
+		return "", ErrNotFound
+	}
 
-	if r.Exists(key) {
-		if fi, err = os.Stat(r.keyPath(key)); err == nil {
-			s = strconv.FormatInt(fi.ModTime().UnixNano(), 10)
-		}
+	fi, err := os.Stat(r.keyPath(key))
+	if err != nil {
+		return "", err
 	}
 
-	return
+	return strconv.FormatInt(fi.ModTime().UnixNano(), 10), nil
 }
 
 func (r *GenericRawStorage) ContentType(_ ObjectKey) serializer.ContentType {

pkg/storage/storage.go

@@ -21,10 +21,13 @@ var (
 	ErrAmbiguousFind = errors.New("two or more results were aquired when one was expected")
 	// ErrNotFound is returned when the requested resource wasn't found.
 	ErrNotFound = errors.New("resource not found")
+	// ErrAlreadyExists is returned when when WriteStorage.Create is called for an already stored object.
+	ErrAlreadyExists = errors.New("resource already exists")
 )
 
 type ReadStorage interface {
-	// Get returns a new Object for the resource at the specified kind/uid path, based on the file content
+	// Get returns a new Object for the resource at the specified kind/uid path, based on the file content.
+	// If the resource referred to by the given ObjectKey does not exist, Get returns ErrNotFound.
 	Get(key ObjectKey) (runtime.Object, error)
 
 	// List lists Objects for the specific kind. Optionally, filters can be applied (see the filter package
@@ -41,7 +44,8 @@ type ReadStorage interface {
 	// TODO: Figure out what we should do with these, do we need them and if so where?
 	//
 
-	// GetMeta returns a new Object's APIType representation for the resource at the specified kind/uid path
+	// GetMeta returns a new Object's APIType representation for the resource at the specified kind/uid path.
+	// If the resource referred to by the given ObjectKey does not exist, GetMeta returns ErrNotFound.
 	GetMeta(key ObjectKey) (runtime.PartialObject, error)
 	// ListMeta lists all Objects' APIType representation. In other words,
 	// only metadata about each Object is unmarshalled (uid/name/kind/apiVersion).
@@ -56,7 +60,7 @@ type ReadStorage interface {
 	// Checksum returns a string representing the state of an Object on disk
 	// The checksum should change if any modifications have been made to the
 	// Object on disk, it can be e.g. the Object's modification timestamp or
-	// calculated checksum
+	// calculated checksum. If the Object is not found, ErrNotFound is returned.
 	Checksum(key ObjectKey) (string, error)
 	// Count returns the amount of available Objects of a specific kind
 	// This is used by Caches to check if all Objects are cached to perform a List
@@ -82,9 +86,13 @@ type ReadStorage interface {
 }
 
 type WriteStorage interface {
-	// Set saves the Object to disk. If the Object does not exist, the
-	// ObjectMeta.Created field is set automatically
-	Set(obj runtime.Object) error
+	// Create creates an entry for and stores the given Object in the storage. The Object must be new to the storage.
+	// The ObjectMeta.Created field is set automatically to the current time if it is unset.
+	Create(obj runtime.Object) error
+	// Update updates the state of the given Object in the storage. The Object must exist in the storage.
+	// The ObjectMeta.Created field is set automatically to the current time if it is unset.
+	Update(obj runtime.Object) error
+
 	// Patch performs a strategic merge patch on the Object with the given UID, using the byte-encoded patch given
 	Patch(key ObjectKey, patch []byte) error
 	// Delete removes an Object from the storage
@@ -138,15 +146,8 @@ func (s *GenericStorage) GetMeta(key ObjectKey) (runtime.PartialObject, error) {
 	return s.decodeMeta(key, content)
 }
 
-// Set saves the Object to disk
-func (s *GenericStorage) Set(obj runtime.Object) error {
-	// TODO: Make sure we don't save a partial object
-
-	key, err := s.ObjectKeyFor(obj)
-	if err != nil {
-		return err
-	}
-
+// TODO: Make sure we don't save a partial object
+func (s *GenericStorage) write(key ObjectKey, obj runtime.Object) error {
 	// Set the content type based on the format given by the RawStorage, but default to JSON
 	contentType := serializer.ContentTypeJSON
 	if ct := s.raw.ContentType(key); len(ct) != 0 {
@@ -160,14 +161,42 @@ func (s *GenericStorage) Set(obj runtime.Object) error {
 	}
 
 	var objBytes bytes.Buffer
-	err = s.serializer.Encoder().Encode(serializer.NewFrameWriter(contentType, &objBytes), obj)
+	err := s.serializer.Encoder().Encode(serializer.NewFrameWriter(contentType, &objBytes), obj)
 	if err != nil {
 		return err
 	}
 
 	return s.raw.Write(key, objBytes.Bytes())
 }
 
+func (s *GenericStorage) Create(obj runtime.Object) error {
+	key, err := s.ObjectKeyFor(obj)
+	if err != nil {
+		return err
+	}
+
+	if s.raw.Exists(key) {
+		return ErrAlreadyExists
+	}
+
+	// The object was not found so we can safely create it
+	return s.write(key, obj)
+}
+
+func (s *GenericStorage) Update(obj runtime.Object) error {
+	key, err := s.ObjectKeyFor(obj)
+	if err != nil {
+		return err
+	}
+
+	if !s.raw.Exists(key) {
+		return ErrNotFound
+	}
+
+	// The object was found so we can safely update it
+	return s.write(key, obj)
+}
+
 // Patch performs a strategic merge patch on the object with the given UID, using the byte-encoded patch given
 func (s *GenericStorage) Patch(key ObjectKey, patch []byte) error {
 	oldContent, err := s.raw.Read(key)

pkg/storage/watch/storage.go

@@ -59,10 +59,16 @@ type GenericWatchStorage struct {
 
 var _ WatchStorage = &GenericWatchStorage{}
 
-// Suspend modify events during Set
-func (s *GenericWatchStorage) Set(obj runtime.Object) error {
+// Suspend modify events during Create
+func (s *GenericWatchStorage) Create(obj runtime.Object) error {
 	s.watcher.Suspend(watcher.FileEventModify)
-	return s.Storage.Set(obj)
+	return s.Storage.Create(obj)
+}
+
+// Suspend modify events during Update
+func (s *GenericWatchStorage) Update(obj runtime.Object) error {
+	s.watcher.Suspend(watcher.FileEventModify)
+	return s.Storage.Update(obj)
 }
 
 // Suspend modify events during Patch