Milo: Store console defs as their own entities

milo/common/config.go

Index: milo/common/config.go
diff --git a/milo/common/config.go b/milo/common/config.go
index 42097a94b966bd2e55a9a23c77f7463c74efc7e7..ee5b75738679d9ff25c73433d643afa25ca4af8f 100644
--- a/milo/common/config.go
+++ b/milo/common/config.go
@@ -16,6 +16,7 @@ package common
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/golang/protobuf/proto"
@@ -23,24 +24,99 @@ import (
 
 	"github.com/luci/gae/service/datastore"
 	"github.com/luci/gae/service/info"
+	configInterface "github.com/luci/luci-go/common/config"
 	"github.com/luci/luci-go/common/data/caching/proccache"
 	"github.com/luci/luci-go/common/errors"
 	"github.com/luci/luci-go/common/logging"
 	"github.com/luci/luci-go/luci_config/server/cfgclient"
 	"github.com/luci/luci-go/luci_config/server/cfgclient/backend"
-	"github.com/luci/luci-go/luci_config/server/cfgclient/textproto"
 
 	"github.com/luci/luci-go/milo/api/config"
 )
 
-// Project is a LUCI project.
+// Project is a single luci-config project name.  This is only used as a way
+// to index Console definitions, as it is the parent entity.
 type Project struct {
-	// ID of the project, as per self defined.  This is the luci-config name.
+	// ID is the name of the project
 	ID string `gae:"$id"`
-	// Data is the Project data in protobuf binary format.
-	Data []byte `gae:",noindex"`
-	// Revision is the latest revision we have of this project's config.
+}
+
+// maybePutProject makes the project if it doesn't exist.
+func maybePutProject(c context.Context, id string) error {

[iannucci, 2017/07/19 22:54:10]

The Project only has the ID... why bother putting it all?

[Ryan Tseng, 2017/07/20 00:15:15]

Removed.

+	return datastore.RunInTransaction(c, func(context.Context) error {
+		p := Project{ID: id}
+		if err := datastore.Get(c, &p); err == datastore.ErrNoSuchEntity {
+			return datastore.Put(c, &p)
+		} else {
+			return err
+		}
+	}, nil)
+}
+
+// Console is af datastore entity representing a single console.
+type Console struct {
+	// Parent is a key to the parent project entity where this console was

[iannucci, 2017/07/19 22:54:10]

capital Project (since its a type)

[Ryan Tseng, 2017/07/20 00:15:15]

Done.

+	// defined in.
+	Parent *datastore.Key `gae:"$parent"`
+	// ID is the ID of the console.
+	ID string `gae:"$id"`
+	// RepoURL and Ref combined defines the commits the show up on the left
+	// hand side of a Console.
+	RepoURL string
+	// RepoURL and Ref combined defines the commits the show up on the left
+	// hand side of a Console.
+	Ref string
+	// ManifestName is the name of the manifest to look for when querying for
+	// builds under this console.
+	ManifestName string
+	// URL is the URL to the luci-config definition of this console.
+	URL string
+	// Revision is the luci-config reivision from when this Console was retrieved.
 	Revision string
+	// Builders is a list of universal builder IDs.
+	Builders []string
+}
+
+// GetProjectName retrieves the project name of the console out of the Console's
+// parent key.
+func (con *Console) GetProjectName() string {
+	return con.Parent.StringID()
+}
+
+// NewConsole creates a fully populated console out of the luci-config proto
+// definition of a console.
+func NewConsole(project *datastore.Key, URL, revision string, con *config.Console) *Console {
+	return &Console{
+		Parent:       project,
+		ID:           con.ID,
+		RepoURL:      con.RepoURL,
+		Ref:          con.Ref,
+		ManifestName: con.ManifestName,
+		Revision:     revision,
+		URL:          URL,
+		Builders:     BuilderFromProto(con.Builders),
+	}
+}
+
+// BuilderFromProto tranforms a luci-config proto builder format into the datastore
+// format.
+func BuilderFromProto(cb []*config.Builder) []string {
+	builders := make([]string, len(cb))
+	for i, b := range cb {
+		builders[i] = b.Name
+	}
+	return builders
+}
+
+// LuciConfigURL returns a user friendly URL that specifies where to view
+// this console definition.
+func LuciConfigURL(c context.Context, configSet, path, revision string) string {
+	// TODO(hinoka): This shouldn't be hardcoded, instead we should get the
+	// luci-config instance from the context.  But we only use this instance at
+	// the moment so it is okay for now.
+	// TODO(hinoka): The UI doesn't allow specifying paths and revision yet.  Add
+	// that in when it is supported.
+	return fmt.Sprintf("https://luci-config.appspot.com/newui#/%s", configSet)
 }
 
 // The key for the service config entity in datastore.
@@ -178,125 +254,145 @@ func UpdateServiceConfig(c context.Context) (*config.Settings, error) {
 	return settings, nil
 }
 
-// UpdateProjectConfigs internal project configuration based off luci-config.
-// update updates Milo's configuration based off luci config.  This includes
-// scanning through all project and extract all console configs.
-func UpdateProjectConfigs(c context.Context) error {
-	cfgName := info.AppID(c) + ".cfg"
-
-	var (
-		configs []*config.Project
-		metas   []*cfgclient.Meta
-		merr    errors.MultiError
-	)
-
-	logging.Debugf(c, "fetching configs for %s", cfgName)
-	if err := cfgclient.Projects(c, cfgclient.AsService, cfgName, textproto.Slice(&configs), &metas); err != nil {
-		merr = err.(errors.MultiError)
-		// Some configs errored out, but we can continue with the ones that work still.
+func updateProjectConsoles(c context.Context, projectName string, cfg *configInterface.Config) error {
+	// Ensure that the project exists as an entity.

[iannucci, 2017/07/19 22:54:10]

but y?

[Ryan Tseng, 2017/07/20 00:15:16]

Removed

+	if err := maybePutProject(c, projectName); err != nil {
+		return errors.Annotate(err, "adding project entity").Err()
+	}
+	proj := config.Project{}
+	if err := proto.UnmarshalText(cfg.Content, &proj); err != nil {
+		return errors.Annotate(err, "unmarshalling proto").Err()
 	}
 
-	// A map of project ID to project.
-	projects := map[string]*Project{}
-	for i, proj := range configs {
-		meta := metas[i]
-		projectName, _, _ := meta.ConfigSet.SplitProject()
-		name := string(projectName)
-		projects[name] = nil
-		if merr != nil && merr[i] != nil {
-			logging.WithError(merr[i]).Warningf(c, "skipping %s due to error", name)
-			continue
-		}
-
-		p := &Project{
-			ID:       name,
-			Revision: meta.Revision,
+	// Keep a list of known consoles so we can prune unknown ones later.
+	knownConsoles := make(map[string]bool, len(proj.Consoles))

[iannucci, 2017/07/19 22:54:09]

stringset.New is handy (in luci-go/common/data/stringset)

[Ryan Tseng, 2017/07/20 00:15:15]

Done.

+	// Iterate through all the proto consoles, adding and replacing the
+	// known ones if needed.  If any consoles errors out, then the whole project fails.

[iannucci, 2017/07/19 22:54:09]

'whole project fails' but not really; this isn't in a transaction

[Ryan Tseng, 2017/07/20 00:15:15]

Done.

+	parentKey := datastore.MakeKey(c, "Project", projectName)
+	for _, pc := range proj.Consoles {
+		knownConsoles[pc.ID] = true
+		con, err := GetConsole(c, projectName, pc.ID)
+		switch err {
+		case datastore.ErrNoSuchEntity:
+			// continue
+		case nil:
+			// Check if revisions match, if so just skip it.
+			if con.Revision == cfg.Revision {
+				continue
+			}
+		default:
+			return errors.Annotate(err, "checking %s", pc.ID).Err()
 		}
-
-		logging.Infof(c, "prossing %s", name)
-
-		var err error
-		p.Data, err = proto.Marshal(proj)
-		if err != nil {
-			logging.WithError(err).Errorf(c, "Encountered error while processing project %s", name)
-			// Set this to nil to signal this project exists but we don't want to update it.
-			projects[name] = nil
-			continue
+		URL := LuciConfigURL(c, cfg.ConfigSet, cfg.Path, cfg.Revision)
+		con = NewConsole(parentKey, URL, cfg.Revision, pc)
+		if err = datastore.Put(c, con); err != nil {
+			return errors.Annotate(err, "saving %s", pc.ID).Err()
+		} else {
+			logging.Infof(c, "saved a new %s / %s (revision %s)", projectName, con.ID, cfg.Revision)
 		}
-		projects[name] = p
 	}
 
-	// Now load all the data into the datastore.
-	projs := make([]*Project, 0, len(projects))
-	for _, proj := range projects {
-		if proj != nil {
-			projs = append(projs, proj)
-		}
-	}
-	if err := datastore.Put(c, projs); err != nil {
+	// Now delete all of the extra consoles.
+	q := datastore.NewQuery("Console")
+	q = q.Ancestor(parentKey)
+	q = q.KeysOnly(true)

[iannucci, 2017/07/19 22:54:10]

you can string these together btw

q := datastore.NewQuery("Console").Ancestor(parentKey).KeysOnly(true)

[Ryan Tseng, 2017/07/20 00:15:16]

Yeah, I like doing them line by line tho, i thought it looked nicer this way.

+	existingConsoles := []*Console{}

[iannucci, 2017/07/19 22:54:10]

make this []*datastore.Key{}

[Ryan Tseng, 2017/07/20 00:15:15]

Done.

+	if err := datastore.GetAll(c, q, &existingConsoles); err != nil {
 		return err
 	}
-
-	// Delete entries that no longer exist.
-	q := datastore.NewQuery("Project").KeysOnly(true)
-	allProjs := []Project{}
-	datastore.GetAll(c, q, &allProjs)
-	toDelete := []Project{}
-	for _, proj := range allProjs {
-		if _, ok := projects[proj.ID]; !ok {
-			toDelete = append(toDelete, proj)
+	for _, ec := range existingConsoles {
+		if _, ok := knownConsoles[ec.ID]; !ok {

[iannucci, 2017/07/19 22:54:10]

make a new []*datastore.Key{}, and append to-be-deleted keys to it

[Ryan Tseng, 2017/07/20 00:15:15]

Done.

+			// This console no longer exists, delete it.
+			logging.Infof(c, "deleting console %s / %s", projectName, ec.ID)
+			if err := datastore.Delete(c, ec); err != nil {
+				return err
+			}
 		}
 	}
-	return datastore.Delete(c, toDelete)
+	return nil

[iannucci, 2017/07/19 22:54:10]

do a single `datastore.Delete(c, keysToDelete)`

[Ryan Tseng, 2017/07/20 00:15:15]

Done.

 }
 
-// GetAllProjects returns all registered projects.
-func GetAllProjects(c context.Context) ([]*config.Project, error) {
-	q := datastore.NewQuery("Project")
-	q.Order("ID")
+// UpdateConsoles updates internal console definitions entities based off luci-config.
+func UpdateConsoles(c context.Context) error {
+	cfgName := info.AppID(c) + ".cfg"
 
-	ps := []*Project{}
-	err := datastore.GetAll(c, q, &ps)
+	logging.Debugf(c, "fetching configs for %s", cfgName)
+	// Acquire the raw config client.
+	lucicfg := backend.Get(c).GetConfigInterface(c, backend.AsService)
+	// Project configs for Milo contains console definitions.
+	configs, err := lucicfg.GetProjectConfigs(c, cfgName, false)
 	if err != nil {
-		return nil, err
+		return errors.Annotate(err, "while fetching project configs").Err()
 	}
-	results := make([]*config.Project, len(ps))
-	for i, p := range ps {
-		results[i] = &config.Project{}
-		if err := proto.Unmarshal(p.Data, results[i]); err != nil {
-			return nil, err
+	logging.Infof(c, "got %d project configs", len(configs))
+
+	merr := errors.MultiError{}
+	knownProjects := map[string]bool{}
+	// Iterate through each project config, extracting the console definition.
+	for _, cfg := range configs {
+		// This looks like "projects/<project name>"
+		splitPath := strings.SplitN(cfg.ConfigSet, "/", 2)
+		if len(splitPath) != 2 {
+			return fmt.Errorf("Invalid config set path %s", cfg.ConfigSet)
+		}
+		projectName := splitPath[1]
+		knownProjects[projectName] = true
+		if err := updateProjectConsoles(c, projectName, &cfg); err != nil {
+			err = errors.Annotate(err, "processing project %s", cfg.ConfigSet).Err()
+			merr = append(merr, err)
 		}
 	}
-	return results, nil
-}
 
-// GetProject returns the requested project.
-func GetProject(c context.Context, projName string) (*config.Project, error) {
-	// Next, Try datastore
-	p := Project{ID: projName}
-	if err := datastore.Get(c, &p); err != nil {
-		return nil, err
+	// Delete all the projects that no longer exist.
+	projects := []*Project{}
+	q := datastore.NewQuery("Project")

[iannucci, 2017/07/19 22:54:09]

projectKey := datastore.MakeKey("Project", projName)
keys := []*datastore.Key{}
datastore.GetAll(c, datastore.NewQuery("").Ancestor(projectKey), &keys)
datastore.Delete(c, keys)

[Ryan Tseng, 2017/07/20 00:15:16]

Project removed.

+	if err := datastore.GetAll(c, q, &projects); err != nil {
+		return append(merr, err)
 	}
-	mp := config.Project{}
-	if err := proto.Unmarshal(p.Data, &mp); err != nil {
-		return nil, err
+	for _, proj := range projects {
+		if _, ok := knownProjects[proj.ID]; !ok {
+			// Delete all of the project's consoles, then the project itself.
+			parentKey := datastore.KeyForObj(c, &proj)
+			q := datastore.NewQuery("Console")
+			q = q.Ancestor(parentKey)
+			q = q.KeysOnly(true)
+			projectConsoles := []*Console{}
+			if err := datastore.GetAll(c, q, &projectConsoles); err != nil {
+				return append(merr, err)
+			}
+			logging.Infof(c, "deleting project %s and %s", proj.ID, &projectConsoles)
+			if err := datastore.Delete(c, &projectConsoles); err != nil {
+				return append(merr, err)
+			}
+			if err := datastore.Delete(c, &proj); err != nil {
+				return append(merr, err)
+			}
+		}
 	}
+	if len(merr) == 0 {
+		return nil
+	}
+	return merr
+}
 
-	return &mp, nil
+// GetAllConsoles returns all registered projects.
+func GetAllConsoles(c context.Context) ([]*Console, error) {
+	q := datastore.NewQuery("Console")
+	q.Order("ID")
+	con := []*Console{}
+	err := datastore.GetAll(c, q, &con)
+	return con, err
 }
 
-// GetConsole returns the requested console instance.
-func GetConsole(c context.Context, projName, consoleName string) (*config.Console, error) {
-	p, err := GetProject(c, projName)
-	if err != nil {
-		return nil, err
-	}
-	for _, cs := range p.Consoles {
-		if cs.Name == consoleName {
-			return cs, nil
-		}
+// GetConsole returns the requested console.
+func GetConsole(c context.Context, proj, id string) (*Console, error) {
+	// TODO(hinoka): Memcache this.
+	con := Console{
+		Parent: datastore.MakeKey(c, "Project", proj),
+		ID:     id,
 	}
-	return nil, fmt.Errorf("Console %s not found in project %s", consoleName, projName)
+	err := datastore.Get(c, &con)
+	return &con, err
 }
 
 // ProjectConsole is a simple tuple type for GetConsolesForBuilder.
@@ -307,18 +403,16 @@ type ProjectConsole struct {
 
 // GetConsolesForBuilder retrieves all the console definitions that this builder
 // belongs to.
-func GetConsolesForBuilder(c context.Context, builderName string) ([]*ProjectConsole, error) {
-	projs, err := GetAllProjects(c)
+func GetConsolesForBuilder(c context.Context, builderName string) ([]*Console, error) {
+	cons, err := GetAllConsoles(c)

[Ryan Tseng, 2017/07/20 00:15:15]

do the thing

Done

 	if err != nil {
 		return nil, err
 	}
-	ret := []*ProjectConsole{}
-	for _, p := range projs {
-		for _, con := range p.Consoles {
-			for _, b := range con.Builders {
-				if b.Name == builderName {
-					ret = append(ret, &ProjectConsole{p.ID, con})
-				}
+	ret := []*Console{}
+	for _, con := range cons {
+		for _, b := range con.Builders {
+			if b == builderName {
+				ret = append(ret, con)
 			}
 		}
 	}