cmd/relui: add persistence to development database

This change renames memoryStore to fileStore, and persists data created
in the UI. This will help make local development less painful when
testing changes. Data stored is intended to be loaded at start-up, which
will be implemented in a future change.

For golang/go#40279

Co-authored-by: Carlos Amedee <carlos@golang.org>
Change-Id: Id2390c35b8e1d1d368fbf7ac13b3cdef0776ad87
Reviewed-on: https://go-review.googlesource.com/c/build/+/246298
Run-TryBot: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>

cmd/relui/main.go

@@ -6,6 +6,7 @@
 package main
 
 import (
+	"flag"
 	"io/ioutil"
 	"log"
 	"net/http"
@@ -16,8 +17,13 @@ import (
 	reluipb "golang.org/x/build/cmd/relui/protos"
 )
 
+var (
+	devDataDir = flag.String("dev-data-directory", defaultDevDataDir(), "Development-only directory to use for storage of application state.")
+)
+
 func main() {
-	s := &server{store: &memoryStore{}, configs: loadWorkflowConfig("./workflows")}
+	flag.Parse()
+	s := &server{store: newFileStore(*devDataDir), configs: loadWorkflowConfig("./workflows")}
 	http.Handle("/workflows/create", http.HandlerFunc(s.createWorkflowHandler))
 	http.Handle("/workflows/new", http.HandlerFunc(s.newWorkflowHandler))
 	http.Handle("/", fileServerHandler(relativeFile("./static"), http.HandlerFunc(s.homeHandler)))
@@ -54,3 +60,12 @@ func loadWorkflowConfig(dir string) []*reluipb.Workflow {
 	}
 	return ws
 }
+
+// defaultDevDataDir returns a directory suitable for storage of data when developing relui on most platforms.
+func defaultDevDataDir() string {
+	c, err := os.UserConfigDir()
+	if err != nil {
+		c = os.TempDir()
+	}
+	return filepath.Join(c, "go-build", "relui")
+}

cmd/relui/protos/relui.proto

@@ -41,6 +41,12 @@ message BuildableTask {
   string task_type = 6;
 }
 
+// LocalStorage is the persisted data of relui. It is used in development mode for saving application state.
+message LocalStorage {
+  // workflows are a list of user-created workflows.
+  repeated Workflow workflows = 1;
+}
+
 message GitSource {
   string url = 1;
   string ref = 2;

cmd/relui/store.go

@@ -5,38 +5,81 @@
 package main
 
 import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 	"sync"
 
+	"github.com/golang/protobuf/proto"
 	reluipb "golang.org/x/build/cmd/relui/protos"
 )
 
 // store is a persistence adapter for saving data.
 type store interface {
-	GetWorkflows() []*reluipb.Workflow
+	Workflows() []*reluipb.Workflow
 	AddWorkflow(workflow *reluipb.Workflow) error
 }
 
-var _ store = (*memoryStore)(nil)
+var _ store = (*fileStore)(nil)
 
-// memoryStore is a non-durable implementation of store that keeps everything in memory.
-type memoryStore struct {
-	mut       sync.Mutex
-	workflows []*reluipb.Workflow
+// newFileStore initializes a fileStore ready for use.
+//
+// If dir is set to an empty string (""), no data will be saved to disk.
+func newFileStore(dir string) *fileStore {
+	return &fileStore{
+		persistDir: dir,
+		ls:         new(reluipb.LocalStorage),
+	}
+}
+
+// fileStoreName is the name of the data file used by fileStore for persistence.
+const fileStoreName = "local_storage.textpb"
+
+// fileStore is a non-durable implementation of store that keeps everything in memory.
+type fileStore struct {
+	mu sync.Mutex
+	ls *reluipb.LocalStorage
+
+	// persistDir is a path to a directory for saving application data in textproto format.
+	persistDir string
 }
 
-// AddWorkflow adds a workflow to the store.
-func (m *memoryStore) AddWorkflow(w *reluipb.Workflow) error {
-	m.mut.Lock()
-	defer m.mut.Unlock()
-	m.workflows = append(m.workflows, w)
+// AddWorkflow adds a workflow to the store, persisting changes to disk.
+func (f *fileStore) AddWorkflow(w *reluipb.Workflow) error {
+	f.mu.Lock()
+	f.ls.Workflows = append(f.ls.Workflows, w)
+	f.mu.Unlock()
+	if err := f.persist(); err != nil {
+		return err
+	}
 	return nil
 }
 
-// GetWorkflows returns all workflows stored.
-//
-// TODO(golang.org/issue/40279) - clone workflows if they're ever mutated.
-func (m *memoryStore) GetWorkflows() []*reluipb.Workflow {
-	m.mut.Lock()
-	defer m.mut.Unlock()
-	return m.workflows
+// Workflows returns all workflows stored.
+func (f *fileStore) Workflows() []*reluipb.Workflow {
+	return f.localStorage().GetWorkflows()
+}
+
+// localStorage returns a deep copy of data stored in fileStore.
+func (f *fileStore) localStorage() *reluipb.LocalStorage {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return proto.Clone(f.ls).(*reluipb.LocalStorage)
+}
+
+// persist saves fileStore state to persistDir/fileStoreName.
+func (f *fileStore) persist() error {
+	if f.persistDir == "" {
+		return nil
+	}
+	if err := os.MkdirAll(f.persistDir, 0755); err != nil {
+		return fmt.Errorf("os.MkDirAll(%q, %v) = %w", f.persistDir, 0755, err)
+	}
+	dst := filepath.Join(f.persistDir, fileStoreName)
+	data := []byte(proto.MarshalTextString(f.localStorage()))
+	if err := ioutil.WriteFile(dst, data, 0644); err != nil {
+		return fmt.Errorf("ioutil.WriteFile(%q, _, %v) = %w", dst, 0644, err)
+	}
+	return nil
 }

cmd/relui/store_test.go

@@ -0,0 +1,52 @@
+// Copyright 2020 The Go 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 main
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/golang/protobuf/proto"
+	"github.com/google/go-cmp/cmp"
+	reluipb "golang.org/x/build/cmd/relui/protos"
+)
+
+func TestFileStorePersist(t *testing.T) {
+	dir, err := ioutil.TempDir("", "memory-store-test")
+	if err != nil {
+		t.Fatalf("ioutil.TempDir(%q, %q) = _, %v", "", "memory-store-test", err)
+	}
+	defer os.RemoveAll(dir)
+	want := &reluipb.LocalStorage{
+		Workflows: []*reluipb.Workflow{
+			{
+				Name:           "Persist Test",
+				BuildableTasks: []*reluipb.BuildableTask{{Name: "Persist Test Task"}},
+			},
+		},
+	}
+	fs := newFileStore(filepath.Join(dir, "relui"))
+	fs.ls = want
+
+	err = fs.persist()
+	if err != nil {
+		t.Fatalf("fs.Persist() = %v, wanted no error", err)
+	}
+
+	b, err := ioutil.ReadFile(filepath.Join(dir, "relui", fileStoreName))
+	if err != nil {
+		t.Fatalf("ioutil.ReadFile(%q) = _, %v, wanted no error", filepath.Join(dir, "relui", fileStoreName), err)
+	}
+	got := new(reluipb.LocalStorage)
+	err = proto.UnmarshalText(string(b), got)
+	if err != nil {
+		t.Fatalf("proto.UnmarshalText(_) = %v, wanted no error", err)
+	}
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("reluipb.LocalStorage mismatch (-want, +got):\n%s", diff)
+	}
+}

cmd/relui/web.go

@@ -64,7 +64,7 @@ type homeResponse struct {
 // homeHandler renders the homepage.
 func (s *server) homeHandler(w http.ResponseWriter, _ *http.Request) {
 	out := bytes.Buffer{}
-	if err := homeTmpl.Execute(&out, homeResponse{Workflows: s.store.GetWorkflows()}); err != nil {
+	if err := homeTmpl.Execute(&out, homeResponse{Workflows: s.store.Workflows()}); err != nil {
 		log.Printf("homeHandler: %v", err)
 		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return
@@ -106,6 +106,7 @@ func (s *server) createWorkflowHandler(w http.ResponseWriter, r *http.Request) {
 	}
 	wf.Params["GitObject"] = ref
 	if err := s.store.AddWorkflow(wf); err != nil {
+		log.Printf("Error adding workflow: s.store.AddWorkflow(%v) = %v", wf, err)
 		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		return
 	}

cmd/relui/web_test.go

@@ -83,7 +83,7 @@ func TestServerHomeHandler(t *testing.T) {
 	req := httptest.NewRequest(http.MethodGet, "/", nil)
 	w := httptest.NewRecorder()
 
-	s := &server{store: &memoryStore{}}
+	s := &server{store: newFileStore("")}
 	s.homeHandler(w, req)
 	resp := w.Result()
 
@@ -96,7 +96,7 @@ func TestServerNewWorkflowHandler(t *testing.T) {
 	req := httptest.NewRequest(http.MethodGet, "/workflows/new", nil)
 	w := httptest.NewRecorder()
 
-	s := &server{store: &memoryStore{}}
+	s := &server{store: newFileStore("")}
 	s.newWorkflowHandler(w, req)
 	resp := w.Result()
 
@@ -134,7 +134,7 @@ func TestServerCreateWorkflowHandler(t *testing.T) {
 			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 			w := httptest.NewRecorder()
 
-			s := &server{store: &memoryStore{}, configs: config}
+			s := &server{store: newFileStore(""), configs: config}
 			s.createWorkflowHandler(w, req)
 			resp := w.Result()
 
@@ -146,16 +146,16 @@ func TestServerCreateWorkflowHandler(t *testing.T) {
 					t.Errorf("resp.Header.Get(%q) = %q, wanted %q", k, resp.Header.Get(k), v)
 				}
 			}
-			if len(s.store.GetWorkflows()) != 1 && c.wantParams != nil {
-				t.Fatalf("len(s.store.GetWorkflows()) = %d, wanted %d", len(s.store.GetWorkflows()), 1)
-			} else if len(s.store.GetWorkflows()) != 0 && c.wantParams == nil {
-				t.Fatalf("len(s.store.GetWorkflows()) = %d, wanted %d", len(s.store.GetWorkflows()), 0)
+			if len(s.store.Workflows()) != 1 && c.wantParams != nil {
+				t.Fatalf("len(s.store.Workflows()) = %d, wanted %d", len(s.store.Workflows()), 1)
+			} else if len(s.store.Workflows()) != 0 && c.wantParams == nil {
+				t.Fatalf("len(s.store.Workflows()) = %d, wanted %d", len(s.store.Workflows()), 0)
 			}
 			if c.wantParams == nil {
 				return
 			}
-			if diff := cmp.Diff(c.wantParams, s.store.GetWorkflows()[0].GetParams()); diff != "" {
-				t.Errorf("s.Store.GetWorkflows()[0].Params() mismatch (-want, +got):\n%s", diff)
+			if diff := cmp.Diff(c.wantParams, s.store.Workflows()[0].GetParams()); diff != "" {
+				t.Errorf("s.Store.Workflows()[0].Params() mismatch (-want, +got):\n%s", diff)
 			}
 		})
 	}