From 440ab3e0ae4961c669efdaf84bb6e2e7d09e826e Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@hashicorp.com>
Date: Mon, 19 Apr 2021 18:47:07 -0400
Subject: [PATCH] submatview: move error return to NewMaterializer

So that we don't have to create views ahead of time, when we will never use that view.
---
 agent/rpcclient/health/health.go | 31 ++++++++++++-------------------
 agent/submatview/store.go        | 24 +++++++++++++++++-------
 agent/submatview/store_test.go   |  4 ++--
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/agent/rpcclient/health/health.go b/agent/rpcclient/health/health.go
index 8c42b5e85..547777337 100644
--- a/agent/rpcclient/health/health.go
+++ b/agent/rpcclient/health/health.go
@@ -59,15 +59,17 @@ func (c *Client) getServiceNodes(
 ) (structs.IndexedCheckServiceNodes, cache.ResultMeta, error) {
 	var out structs.IndexedCheckServiceNodes
 
+	// TODO: if UseStreaming, elif !UseCache, else cache
+
 	if !req.QueryOptions.UseCache {
 		err := c.NetRPC.RPC("Health.ServiceNodes", &req, &out)
 		return out, cache.ResultMeta{}, err
 	}
 
 	if req.Source.Node == "" {
-		sr, err := newServiceRequest(req, c.MaterializerDeps)
-		if err != nil {
-			return out, cache.ResultMeta{}, err
+		sr := serviceRequest{
+			ServiceSpecificRequest: req,
+			deps:                   c.MaterializerDeps,
 		}
 
 		result, err := c.ViewStore.Get(ctx, sr)
@@ -109,21 +111,8 @@ func (c *Client) Notify(
 	return c.Cache.Notify(ctx, cacheName, &req, correlationID, ch)
 }
 
-func newServiceRequest(req structs.ServiceSpecificRequest, deps MaterializerDeps) (serviceRequest, error) {
-	view, err := newHealthView(req)
-	if err != nil {
-		return serviceRequest{}, err
-	}
-	return serviceRequest{
-		ServiceSpecificRequest: req,
-		view:                   view,
-		deps:                   deps,
-	}, nil
-}
-
 type serviceRequest struct {
 	structs.ServiceSpecificRequest
-	view *healthView
 	deps MaterializerDeps
 }
 
@@ -135,11 +124,15 @@ func (r serviceRequest) Type() string {
 	return "service-health"
 }
 
-func (r serviceRequest) NewMaterializer() *submatview.Materializer {
+func (r serviceRequest) NewMaterializer() (*submatview.Materializer, error) {
+	view, err := newHealthView(r.ServiceSpecificRequest)
+	if err != nil {
+		return nil, err
+	}
 	return submatview.NewMaterializer(submatview.Deps{
-		View:    r.view,
+		View:    view,
 		Client:  r.deps.Client,
 		Logger:  r.deps.Logger,
 		Request: newMaterializerRequest(r.ServiceSpecificRequest),
-	})
+	}), nil
 }
diff --git a/agent/submatview/store.go b/agent/submatview/store.go
index f7dd61fb6..18fa7b2bb 100644
--- a/agent/submatview/store.go
+++ b/agent/submatview/store.go
@@ -73,7 +73,7 @@ func (s *Store) Run(ctx context.Context) {
 // TODO: godoc
 type Request interface {
 	cache.Request
-	NewMaterializer() *Materializer
+	NewMaterializer() (*Materializer, error)
 	Type() string
 }
 
@@ -82,7 +82,10 @@ type Request interface {
 // See agent/cache.Cache.Get for complete documentation.
 func (s *Store) Get(ctx context.Context, req Request) (Result, error) {
 	info := req.CacheInfo()
-	key, e := s.getEntry(req)
+	key, e, err := s.getEntry(req)
+	if err != nil {
+		return Result{}, err
+	}
 	defer s.releaseEntry(key)
 
 	ctx, cancel := context.WithTimeout(ctx, info.Timeout)
@@ -108,7 +111,10 @@ func (s *Store) Notify(
 	updateCh chan<- cache.UpdateEvent,
 ) error {
 	info := req.CacheInfo()
-	key, e := s.getEntry(req)
+	key, e, err := s.getEntry(req)
+	if err != nil {
+		return err
+	}
 
 	go func() {
 		defer s.releaseEntry(key)
@@ -150,7 +156,7 @@ func (s *Store) Notify(
 
 // getEntry from the store, and increment the requests counter. releaseEntry
 // must be called when the request is finished to decrement the counter.
-func (s *Store) getEntry(req Request) (string, entry) {
+func (s *Store) getEntry(req Request) (string, entry, error) {
 	info := req.CacheInfo()
 	key := makeEntryKey(req.Type(), info)
 
@@ -160,11 +166,15 @@ func (s *Store) getEntry(req Request) (string, entry) {
 	if ok {
 		e.requests++
 		s.byKey[key] = e
-		return key, e
+		return key, e, nil
+	}
+
+	mat, err := req.NewMaterializer()
+	if err != nil {
+		return "", e, err
 	}
 
 	ctx, cancel := context.WithCancel(context.Background())
-	mat := req.NewMaterializer()
 	go mat.Run(ctx)
 
 	e = entry{
@@ -173,7 +183,7 @@ func (s *Store) getEntry(req Request) (string, entry) {
 		requests:     1,
 	}
 	s.byKey[key] = e
-	return key, e
+	return key, e, nil
 }
 
 // idleTTL is the duration of time an entry should remain in the Store after the
diff --git a/agent/submatview/store_test.go b/agent/submatview/store_test.go
index 2ac61912a..8d6105036 100644
--- a/agent/submatview/store_test.go
+++ b/agent/submatview/store_test.go
@@ -146,7 +146,7 @@ func (r *fakeRequest) CacheInfo() cache.RequestInfo {
 	}
 }
 
-func (r *fakeRequest) NewMaterializer() *Materializer {
+func (r *fakeRequest) NewMaterializer() (*Materializer, error) {
 	return NewMaterializer(Deps{
 		View:   &fakeView{srvs: make(map[string]*pbservice.CheckServiceNode)},
 		Client: r.client,
@@ -162,7 +162,7 @@ func (r *fakeRequest) NewMaterializer() *Materializer {
 			}
 			return req
 		},
-	})
+	}), nil
 }
 
 func (r *fakeRequest) Type() string {