cipd: Extract high level file system operations into the separate interface.

go/src/infra/tools/cipd/local/fs.go

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package local
+
+import (
+        "fmt"
+        "os"
+        "path/filepath"
+        "strings"
+        "sync"
+        "time"
+
+        "github.com/luci/luci-go/common/logging"
+)
+
+// FileSystem is a high level interface for operations that touch single file

[nodir, 2015/06/03 19:30:27]

nit: high-level

[Vadim Sh., 2015/06/03 22:35:08]

Done.

+// system partition. All functions operate in terms of native file paths. It
+// exists mostly to hide difference between file system semantic on Windows and
+// Linux\Mac.
+type FileSystem interface {
+        // Root returns absolute path to a directory FileSystem operates in. All FS
+        // actions are restricted to this directory.
+        Root() string
+
+        // ToAbsPath converts a relative path to an absolute one and verifies it is
+        // under the root path of the FileSystem object.
+        ToAbsPath(path string) (string, error)
+
+        // EnsureDirectory creates a directory at given native path if it doesn't
+        // exist yet. It takes an absolute path or a path relative to the current
+        // working directory and always returns absolute path.
+        EnsureDirectory(path string) (string, error)
+
+        // EnsureSymlink creates a symlink pointing to a target. It will create full
+        // directory path to the symlink if necessary.
+        EnsureSymlink(path string, target string) error
+
+        // EnsureFileGone removes a file, logging the errors (if any). Missing file is
+        // not an error.
+        EnsureFileGone(path string) error
+
+        // EnsureDirectoryGone recursively removes a directory.
+        EnsureDirectoryGone(path string) error
+
+        // Renames oldpath to newpath. If newpath already exists (be it a file or a
+        // directory), removes it first.
+        Replace(oldpath, newpath string) error
+}
+
+// NewFileSystem returns default FileSystem implementation that operates with
+// files under a given path. All methods accept absolute paths or paths relative
+// to current working directory. FileSystem will ensure they are under 'root'
+// directory.
+func NewFileSystem(root string, logger logging.Logger) FileSystem {
+        if logger == nil {
+                logger = logging.Null()
+        }
+        abs, err := filepath.Abs(root)
+        if err != nil {
+                return &fsImplErr{err}
+        }
+        return &fsImpl{abs, logger}
+}
+
+// fsImplErr implements FileSystem by returning given error from all methods.
+type fsImplErr struct {
+        err error
+}
+
+// Root returns absolute path to a directory FileSystem operates in. All FS
+// actions are restricted to this directory.
+func (f *fsImplErr) Root() string                                   { return "" }
+func (f *fsImplErr) ToAbsPath(path string) (string, error)          { return "", f.err }
+func (f *fsImplErr) EnsureDirectory(path string) (string, error)    { return "", f.err }
+func (f *fsImplErr) EnsureSymlink(path string, target string) error { return f.err }
+func (f *fsImplErr) EnsureFileGone(path string) error               { return f.err }
+func (f *fsImplErr) EnsureDirectoryGone(path string) error          { return f.err }
+func (f *fsImplErr) Replace(oldpath, newpath string) error          { return f.err }
+
+/// Implementation.
+
+type fsImpl struct {
+        root   string
+        logger logging.Logger
+}
+
+func (f *fsImpl) Root() string {
+        return f.root
+}
+
+func (f *fsImpl) ToAbsPath(p string) (string, error) {
+        p, err := filepath.Abs(p)
+        if err != nil {
+                return "", err
+        }
+        rel, err := filepath.Rel(f.root, p)
+        if err != nil {
+                return "", err
+        }
+        rel = filepath.ToSlash(rel)
+        if rel == ".." || strings.HasPrefix(rel, "../") {
+                return "", fmt.Errorf("fs: path %s is outside of %s", p, f.root)
+        }
+        return p, nil
+}
+
+func (f *fsImpl) EnsureDirectory(path string) (string, error) {
+        path, err := f.ToAbsPath(path)
+        if err != nil {
+                return "", err
+        }
+        // MkdirAll returns nil if path already exists.
+        err = os.MkdirAll(path, 0777)
+        if err == nil {

[nodir, 2015/06/03 19:30:27]

nit: join lines 115-116?

[Vadim Sh., 2015/06/03 22:35:08]

Done.

+                return path, nil
+        }
+        return "", err
+}
+
+func (f *fsImpl) EnsureSymlink(path string, target string) error {
+        path, err := f.ToAbsPath(path)
+        if err != nil {
+                return err
+        }
+        if existing, _ := os.Readlink(path); existing == target {
+                return nil
+        }
+        if _, err := f.EnsureDirectory(filepath.Dir(path)); err != nil {
+                return err
+        }
+
+        // Create a new symlink file, can't modify existing one in place.
+        temp := fmt.Sprintf("%s_%s", path, pseudoRand())
+        if err := os.Symlink(target, temp); err != nil {
+                return err
+        }
+
+        // Atomically replace a current symlink with a new one.

[nodir, 2015/06/03 19:30:27]

use "the" instead of a

[Vadim Sh., 2015/06/03 22:35:08]

Done.

+        err = os.Rename(temp, path)
+        if err != nil {
+                os.Remove(temp)

[nodir, 2015/06/03 19:30:27]

use defer for this kind of stuff

[Vadim Sh., 2015/06/03 22:35:08]

In this case defer is not good, since cleanup needed only on error and there
isn't much code between where object is created and needs to be cleanup up (i.e.
putting defer will just complicate code).

+                return err
+        }
+
+        return nil
+}
+
+func (f *fsImpl) EnsureFileGone(path string) error {
+        path, err := f.ToAbsPath(path)
+        if err != nil {
+                return err
+        }
+        err = os.Remove(path)
+        if err != nil && !os.IsNotExist(err) {
+                f.logger.Warningf("fs: failed to remove %s - %s", path, err)
+                return err
+        }
+        return nil
+}
+
+func (f *fsImpl) EnsureDirectoryGone(path string) error {
+        path, err := f.ToAbsPath(path)
+        if err != nil {
+                return err
+        }
+        // Make directory "disappear" instantly by renaming it first.
+        temp := fmt.Sprintf("%s_%v", path, pseudoRand())
+        if err = os.Rename(path, temp); err != nil {
+                if os.IsNotExist(err) {
+                        return nil
+                }
+                f.logger.Warningf("fs: failed to rename directory %s - %s", path, err)
+                return err
+        }
+        if err = os.RemoveAll(temp); err != nil {
+                f.logger.Warningf("fs: failed to remove directory %s - %s", temp, err)
+                return err
+        }
+        return nil
+}
+
+func (f *fsImpl) Replace(oldpath, newpath string) error {
+        oldpath, err := f.ToAbsPath(oldpath)
+        if err != nil {
+                return err
+        }
+        newpath, err = f.ToAbsPath(newpath)
+        if err != nil {
+                return err
+        }
+        if oldpath == newpath {
+                return nil
+        }
+
+        // Make sure oldpath exists before doing heavy stuff.
+        if _, err = os.Stat(oldpath); err != nil {
+                return err
+        }
+
+        // Make parent directory of newpath.
+        if _, err = f.EnsureDirectory(filepath.Dir(newpath)); err != nil {
+                return err
+        }
+
+        // Try a regular move first. Replaces files and empty directories.
+        if err = os.Rename(oldpath, newpath); err == nil {
+                return nil
+        }
+
+        // Move existing path away, if it is there.
+        temp := fmt.Sprintf("%s_%s", newpath, pseudoRand())
+        if err = os.Rename(newpath, temp); err != nil {
+                if !os.IsNotExist(err) {
+                        f.logger.Warningf("fs: failed to rename(%v, %v) - %s", newpath, temp, err)
+                        return err
+                }
+                temp = ""
+        }
+
+        // 'newpath' now should be available.
+        if err := os.Rename(oldpath, newpath); err != nil {
+                f.logger.Warningf("fs: failed to rename(%v, %v) - %s", oldpath, newpath, err)
+                // Try to return the path back... May be too late already.
+                if temp != "" {
+                        if err := os.Rename(temp, newpath); err != nil {
+                                f.logger.Errorf("fs: failed to rename(%v, %v) after unsuccessful move - %s", temp, newpath, err)
+                        }
+                }
+                return err
+        }
+
+        // Cleanup the garbage left. Not a error if fails.
+        if temp != "" {
+                if err := f.EnsureDirectoryGone(temp); err != nil {

[nodir, 2015/06/03 19:30:27]

use defer for cleanup

[nodir, 2015/06/03 19:30:27]

no need for err variable here

[Vadim Sh., 2015/06/03 22:35:08]

Why?.. It is logged.

[Vadim Sh., 2015/06/03 22:35:08]

again, cleanup path is different depending on circumstances, I think it's
cleaner to cleanup before return rather then in defer.

[nodir, 2015/06/04 00:57:36]

Acknowledged.

+                        f.logger.Warningf("fs: failed to cleanup garbage after file replace - %s", err)
+                }
+        }
+        return nil
+}
+
+/// Internal stuff.
+
+var lastUsedTime int64
+var lastUsedTimeLock sync.Mutex
+
+// pseudoRand returns "random enough" string that can be used in file system
+// paths of temp files.
+func pseudoRand() string {
+        lastUsedTimeLock.Lock()
+        ts := time.Now().UnixNano()
+        if ts == lastUsedTime {

[nodir, 2015/06/03 19:30:27]

if ts <= lastUsedTime {
 ts = lastUsedTime + 1
}

otherwise 3 consequent calls in the same nanosecond will return (X, X + 1, X)
add a test

[Vadim Sh., 2015/06/03 22:35:08]

Done.

+                ts++
+        }
+        lastUsedTime = ts
+        lastUsedTimeLock.Unlock()

[nodir, 2015/06/03 19:30:27]

You are not doing defer because Getpid is slow?
If yes, maybe explain this in a comment

[Vadim Sh., 2015/06/03 22:35:08]

I'm not using a defer because 4 lines of code is not that huge chunk of code to
forgot to unlock the lock after it. I like how Lock()/Unlock() pair visually
identifies critical section.

[nodir, 2015/06/04 00:57:36]

in general, defers are not syntax sugar. If something panics between Lock and
Unlock (here it won't), then defer is needed.

+        return fmt.Sprintf("%v_%v", os.Getpid(), ts)
+}