From 799339acb536db50fadffe52877ff0b69c9c794f Mon Sep 17 00:00:00 2001 From: James Phillips Date: Fri, 26 Feb 2016 12:07:43 -0800 Subject: [PATCH] Factors rendering down into the resolve function. --- consul/prepared_query/template.go | 5 +++ consul/prepared_query_endpoint.go | 15 +-------- consul/state/prepared_query.go | 49 +++++++++++++++++++---------- consul/state/prepared_query_test.go | 12 +++---- consul/state/state_store.go | 2 +- 5 files changed, 46 insertions(+), 37 deletions(-) diff --git a/consul/prepared_query/template.go b/consul/prepared_query/template.go index 2da712b9cb..0854576a18 100644 --- a/consul/prepared_query/template.go +++ b/consul/prepared_query/template.go @@ -86,6 +86,11 @@ func Compile(query *structs.PreparedQuery) (*CompiledTemplate, error) { // example, if the user looks up foobar.query.consul via DNS then we will call // this function with "foobar" on the compiled template. func (ct *CompiledTemplate) Render(name string) (*structs.PreparedQuery, error) { + // Make it "safe" to render a default structure. + if ct == nil { + return nil, fmt.Errorf("Cannot render an uncompiled template") + } + // Start with a fresh, detached copy of the original so we don't disturb // the prototype. dup, err := copystructure.Copy(ct.query) diff --git a/consul/prepared_query_endpoint.go b/consul/prepared_query_endpoint.go index a9b9f1b7b0..6cd6d62ba4 100644 --- a/consul/prepared_query_endpoint.go +++ b/consul/prepared_query_endpoint.go @@ -8,7 +8,6 @@ import ( "time" "github.com/armon/go-metrics" - "github.com/hashicorp/consul/consul/prepared_query" "github.com/hashicorp/consul/consul/structs" "github.com/hashicorp/go-uuid" ) @@ -290,7 +289,7 @@ func (p *PreparedQuery) Execute(args *structs.PreparedQueryExecuteRequest, // Try to locate the query. state := p.srv.fsm.State() - _, query, ct, err := state.PreparedQueryLookup(args.QueryIDOrName) + _, query, err := state.PreparedQueryResolve(args.QueryIDOrName) if err != nil { return err } @@ -298,18 +297,6 @@ func (p *PreparedQuery) Execute(args *structs.PreparedQueryExecuteRequest, return ErrQueryNotFound } - // If this is a template then render the query first. - if prepared_query.IsTemplate(query) { - if ct == nil { - return fmt.Errorf("Missing compiled template") - } - render, err := ct.Render(args.QueryIDOrName) - if err != nil { - return err - } - query = render - } - // Execute the query for the local DC. if err := p.execute(query, reply); err != nil { return err diff --git a/consul/state/prepared_query.go b/consul/state/prepared_query.go index d71f764ee2..66476ecb36 100644 --- a/consul/state/prepared_query.go +++ b/consul/state/prepared_query.go @@ -261,21 +261,22 @@ func (s *StateStore) PreparedQueryGet(queryID string) (uint64, *structs.Prepared return idx, toPreparedQuery(wrapped), nil } -// PreparedQueryLookup returns the given prepared query by looking up an ID or -// Name. -func (s *StateStore) PreparedQueryLookup(queryIDOrName string) (uint64, *structs.PreparedQuery, *prepared_query.CompiledTemplate, error) { +// PreparedQueryResolve returns the given prepared query by looking up an ID or +// Name. If the query was looked up by name and it's a template, then the +// template will be rendered before it is returned. +func (s *StateStore) PreparedQueryResolve(queryIDOrName string) (uint64, *structs.PreparedQuery, error) { tx := s.db.Txn(false) defer tx.Abort() // Get the table index. - idx := maxIndexTxn(tx, s.getWatchTables("PreparedQueryLookup")...) + idx := maxIndexTxn(tx, s.getWatchTables("PreparedQueryResolve")...) // Explicitly ban an empty query. This will never match an ID and the // schema is set up so it will never match a query with an empty name, // but we check it here to be explicit about it (we'd never want to // return the results from the first query w/o a name). if queryIDOrName == "" { - return 0, nil, nil, ErrMissingQueryID + return 0, nil, ErrMissingQueryID } // Try first by ID if it looks like they gave us an ID. We check the @@ -284,11 +285,29 @@ func (s *StateStore) PreparedQueryLookup(queryIDOrName string) (uint64, *structs if isUUID(queryIDOrName) { wrapped, err := tx.First("prepared-queries", "id", queryIDOrName) if err != nil { - return 0, nil, nil, fmt.Errorf("failed prepared query lookup: %s", err) + return 0, nil, fmt.Errorf("failed prepared query lookup: %s", err) } if wrapped != nil { - wrapper := wrapped.(*queryWrapper) - return idx, wrapper.PreparedQuery, wrapper.ct, nil + query := toPreparedQuery(wrapped) + if prepared_query.IsTemplate(query) { + return idx, nil, fmt.Errorf("prepared query templates can only be resolved up by name, not by ID") + } + return idx, query, nil + } + } + + // prep will check to see if the query is a template and render it + // first, otherwise it will just return a regular query. + prep := func(wrapped interface{}) (uint64, *structs.PreparedQuery, error) { + wrapper := wrapped.(*queryWrapper) + if prepared_query.IsTemplate(wrapper.PreparedQuery) { + render, err := wrapper.ct.Render(queryIDOrName) + if err != nil { + return idx, nil, err + } + return idx, render, nil + } else { + return idx, wrapper.PreparedQuery, nil } } @@ -300,13 +319,12 @@ func (s *StateStore) PreparedQueryLookup(queryIDOrName string) (uint64, *structs { wrapped, err := tx.First("prepared-queries", "name_prefix", queryIDOrName) if err != nil { - return 0, nil, nil, fmt.Errorf("failed prepared query lookup: %s", err) + return 0, nil, fmt.Errorf("failed prepared query lookup: %s", err) } if wrapped != nil { - wrapper := wrapped.(*queryWrapper) - query, ct := wrapper.PreparedQuery, wrapper.ct + query := toPreparedQuery(wrapped) if query.Name == queryIDOrName || prepared_query.IsTemplate(query) { - return idx, query, ct, nil + return prep(wrapped) } } } @@ -315,15 +333,14 @@ func (s *StateStore) PreparedQueryLookup(queryIDOrName string) (uint64, *structs { wrapped, err := tx.First("prepared-queries", "wild", true) if err != nil { - return 0, nil, nil, fmt.Errorf("failed prepared query lookup: %s", err) + return 0, nil, fmt.Errorf("failed prepared query lookup: %s", err) } if wrapped != nil { - wrapper := wrapped.(*queryWrapper) - return idx, wrapper.PreparedQuery, wrapper.ct, nil + return prep(wrapped) } } - return idx, nil, nil, nil + return idx, nil, nil } // PreparedQueryList returns all the prepared queries. diff --git a/consul/state/prepared_query_test.go b/consul/state/prepared_query_test.go index 04412da8f4..de7196815b 100644 --- a/consul/state/prepared_query_test.go +++ b/consul/state/prepared_query_test.go @@ -318,7 +318,7 @@ func TestStateStore_PreparedQueryDelete(t *testing.T) { } } -func TestStateStore_PreparedQueryLookup(t *testing.T) { +func TestStateStore_PreparedQueryResolve(t *testing.T) { s := testStateStore(t) // Set up our test environment. @@ -336,7 +336,7 @@ func TestStateStore_PreparedQueryLookup(t *testing.T) { // Try to lookup a query that's not there using something that looks // like a real ID. - idx, actual, _, err := s.PreparedQueryLookup(query.ID) + idx, actual, err := s.PreparedQueryResolve(query.ID) if err != nil { t.Fatalf("err: %s", err) } @@ -349,7 +349,7 @@ func TestStateStore_PreparedQueryLookup(t *testing.T) { // Try to lookup a query that's not there using something that looks // like a name - idx, actual, _, err = s.PreparedQueryLookup(query.Name) + idx, actual, err = s.PreparedQueryResolve(query.Name) if err != nil { t.Fatalf("err: %s", err) } @@ -382,7 +382,7 @@ func TestStateStore_PreparedQueryLookup(t *testing.T) { ModifyIndex: 3, }, } - idx, actual, _, err = s.PreparedQueryLookup(query.ID) + idx, actual, err = s.PreparedQueryResolve(query.ID) if err != nil { t.Fatalf("err: %s", err) } @@ -394,7 +394,7 @@ func TestStateStore_PreparedQueryLookup(t *testing.T) { } // Read it back using the name and verify it again. - idx, actual, _, err = s.PreparedQueryLookup(query.Name) + idx, actual, err = s.PreparedQueryResolve(query.Name) if err != nil { t.Fatalf("err: %s", err) } @@ -407,7 +407,7 @@ func TestStateStore_PreparedQueryLookup(t *testing.T) { // Make sure an empty lookup is well-behaved if there are actual queries // in the state store. - idx, actual, _, err = s.PreparedQueryLookup("") + idx, actual, err = s.PreparedQueryResolve("") if err != ErrMissingQueryID { t.Fatalf("bad: %v ", err) } diff --git a/consul/state/state_store.go b/consul/state/state_store.go index 9f50fb2631..7ba3ae3355 100644 --- a/consul/state/state_store.go +++ b/consul/state/state_store.go @@ -413,7 +413,7 @@ func (s *StateStore) getWatchTables(method string) []string { return []string{"acls"} case "Coordinates": return []string{"coordinates"} - case "PreparedQueryGet", "PreparedQueryLookup", "PreparedQueryList": + case "PreparedQueryGet", "PreparedQueryResolve", "PreparedQueryList": return []string{"prepared-queries"} }