From 78f87c6bb08d74e228bd0ba6015cd1372384ade7 Mon Sep 17 00:00:00 2001 From: Nia Weiss Date: Thu, 22 Oct 2020 09:49:26 -0400 Subject: [PATCH] cleanup of key translation fix --- cluster.go | 8 +- executor.go | 287 +------------------------------------- executor_internal_test.go | 116 +-------------- executor_test.go | 9 +- http/client.go | 8 +- http/handler.go | 8 +- translator_test.go | 94 ++++--------- 7 files changed, 52 insertions(+), 478 deletions(-) diff --git a/cluster.go b/cluster.go index 246f71e74..1c7d14968 100644 --- a/cluster.go +++ b/cluster.go @@ -2659,7 +2659,7 @@ func (c *cluster) findIndexKeys(ctx context.Context, indexName string, keys ...s // On child calls, there are no remote results since we were only sent the keys that we own. remoteResults := make(chan map[string]uint64, len(keysByNode)) var g errgroup.Group - defer g.Wait() + defer g.Wait() //nolint:errcheck for node, keys := range keysByNode { node, keys := node, keys @@ -2702,6 +2702,8 @@ func (c *cluster) findIndexKeys(ctx context.Context, indexName string, keys ...s } // Merge the translations. + // All data should have been written to here while we waited. + // Closing the channel prevents the range from blocking. close(remoteResults) for t := range remoteResults { for key, id := range t { @@ -2756,7 +2758,7 @@ func (c *cluster) createIndexKeys(ctx context.Context, indexName string, keys .. translateResults := make(chan map[string]uint64, len(keysByNode)+len(keysByPartition)) var g errgroup.Group - defer g.Wait() + defer g.Wait() //nolint:errcheck // Start translating keys remotely. // On child calls, there are no remote results since we were only sent the keys that we own. @@ -2805,6 +2807,8 @@ func (c *cluster) createIndexKeys(ctx context.Context, indexName string, keys .. } // Merge the translations. + // All data should have been written to here while we waited. + // Closing the channel prevents the range from blocking. translations := make(map[string]uint64, len(keys)) close(translateResults) for t := range translateResults { diff --git a/executor.go b/executor.go index 044563132..575820de0 100644 --- a/executor.go +++ b/executor.go @@ -456,8 +456,8 @@ func (e *executor) execute(ctx context.Context, index string, q *pql.Query, shar defer span.Finish() // Apply translations if necessary. - var colTranslations map[string]map[string]uint64 - var rowTranslations map[string]map[string]map[string]uint64 + var colTranslations map[string]map[string]uint64 // colID := colTranslations[index][key] + var rowTranslations map[string]map[string]map[string]uint64 // rowID := rowTranslations[index][field][key] if !opt.Remote { cols, rows, err := e.preTranslate(ctx, index, q.Calls...) if err != nil { @@ -3948,103 +3948,6 @@ func (e *executor) mapperLocal(ctx context.Context, shards []uint64, mapFn mapFu } } -func (e *executor) translateCalls(ctx context.Context, defaultIndexName string, calls []*pql.Call) (err error) { - span, _ := tracing.StartSpanFromContext(ctx, "Executor.translateCalls") - defer span.Finish() - - // Generate a list of all used - keySets := make(map[string]map[string]struct{}) - for _, c := range calls { - if err := e.collectCallKeySets(ctx, defaultIndexName, c, keySets); err != nil { - return err - } - } - - // Perform a separate batch translation for each separate index used. - keyMaps := make(map[string]map[string]uint64) - for indexName, keySet := range keySets { - idx := e.Holder.indexes[indexName] - if idx == nil { - return fmt.Errorf("cannot find index %q", indexName) - } - - if !idx.Keys() || len(keySets) == 0 { - continue - } - if keyMaps[indexName], err = e.Cluster.translateIndexKeySet(ctx, indexName, keySet, true); err != nil { - return err - } - } - - // Translate calls. - for _, c := range calls { - if err := e.translateCall(ctx, defaultIndexName, c, keyMaps, true); err != nil { - return err - } - } - - return nil -} - -func (e *executor) collectCallKeySets(ctx context.Context, indexName string, c *pql.Call, m map[string]map[string]struct{}) error { - // Specifying an 'index' call overrides indexes on subsequent calls. - if s := c.CallIndex(); s != "" { - indexName = s - } - - if m[indexName] == nil { - m[indexName] = make(map[string]struct{}) - } - - // Collect key for this call. - colKey, rowKey, fieldName := c.TranslateInfo(columnLabel, rowLabel) - if c.Args[colKey] != nil && isString(c.Args[colKey]) { - if value := callArgString(c, colKey); value != "" { - m[indexName][value] = struct{}{} - } - } - - // Collect foreign index keys. - if fieldName != "" { - idx, exists := e.Holder.indexes[indexName] - if !exists { - return errors.Wrapf(ErrIndexNotFound, "%s", indexName) - } - if field := idx.Field(fieldName); field != nil && field.ForeignIndex() != "" { - foreignIndexName := field.ForeignIndex() - if m[foreignIndexName] == nil { - m[foreignIndexName] = make(map[string]struct{}) - } - - if c.Args[rowKey] != nil && isCondition(c.Args[rowKey]) { - cond := c.Args[rowKey].(*pql.Condition) - if isString(cond.Value) { - m[foreignIndexName][cond.Value.(string)] = struct{}{} - } - } else if value := callArgString(c, rowKey); value != "" { - m[foreignIndexName][value] = struct{}{} - } - } - } - - // Recursively collect argument calls. - for _, arg := range c.Args { - if arg, ok := arg.(*pql.Call); ok { - if err := e.collectCallKeySets(ctx, indexName, arg, m); err != nil { - return errors.Wrap(err, "collecting group by call index name") - } - } - } - - // Recursively collect child calls. - for _, child := range c.Children { - if err := e.collectCallKeySets(ctx, indexName, child, m); err != nil { - return err - } - } - return nil -} - func (e *executor) preTranslate(ctx context.Context, index string, calls ...*pql.Call) (cols map[string]map[string]uint64, rows map[string]map[string]map[string]uint64, err error) { // Collect all of the required keys. collector := keyCollector{ @@ -4697,192 +4600,6 @@ func (e *executor) callZero(c *pql.Call) *pql.Call { } } -func (e *executor) translateCall(ctx context.Context, indexName string, c *pql.Call, keyMaps map[string]map[string]uint64, writable bool) (err error) { - // Specifying an 'index' arg applies to all nested calls. - if s := c.CallIndex(); s != "" { - indexName = s - } - keyMap := keyMaps[indexName] - - // Translate column key. - colKey, rowKey, fieldName := c.TranslateInfo(columnLabel, rowLabel) - idx, exists := e.Holder.indexes[indexName] - if !exists { - return errors.Wrapf(ErrIndexNotFound, "%s", indexName) - } - if idx.Keys() { - if c.Args[colKey] != nil && !isString(c.Args[colKey]) { - if !isValidID(c.Args[colKey]) { - return errors.Errorf("column value must be a string or non-negative integer, but got: %v of %[1]T", c.Args[colKey]) - } - } else if value := callArgString(c, colKey); value != "" { - c.Args[colKey] = keyMap[value] - } - } else { - if isString(c.Args[colKey]) { - return errors.New("string 'col' value not allowed unless index 'keys' option enabled") - } - } - - // Translate row key, if field is specified & key exists. - var field *Field - if fieldName != "" { - field = idx.Field(fieldName) - if field == nil { - // Instead of returning ErrFieldNotFound here, - // we just return, and don't attempt the translation. - // The assumption is that the non-existent field - // will raise an error downstream when it's used. - return nil - } - - // Bool field keys do not use the translator because there - // are only two possible values. Instead, they are handled - // directly. - if field.Type() == FieldTypeBool { - if c.Name == "Rows" { - // TranslateInfo for Rows returns "previous" as rowKey, - // so for bool fields we would get "missing bool argument" error - return nil - } - boolVal, err := callArgBool(c, rowKey) - if err != nil { - return errors.Wrapf(err, "getting bool key (%+v)", rowKey) - } - rowID := falseRowID - if boolVal { - rowID = trueRowID - } - c.Args[rowKey] = rowID - } else if field.Keys() { - foreignIndexName := field.ForeignIndex() - if c.Args[rowKey] != nil && isCondition(c.Args[rowKey]) { - // In the case where a field has a foreign index with keys, - // allow `== "key"` or `!= "key"` to be used against the BSI - // field. - cond := c.Args[rowKey].(*pql.Condition) - if isString(cond.Value) { - switch cond.Op { - case pql.EQ, pql.NEQ: - var id uint64 - if foreignIndexName != "" { - id = keyMaps[foreignIndexName][cond.Value.(string)] - } else { - if id, err = e.Cluster.translateFieldKey(ctx, field, cond.Value.(string), writable); err != nil { - return errors.Wrapf(err, "translating field key: %s", cond.Value) - } - } - - c.Args[rowKey] = &pql.Condition{ - Op: cond.Op, - Value: id, - } - default: - return errors.Errorf("conditional is not supported with string predicates: %s", cond.Op) - } - } - } else if c.Args[rowKey] != nil && !isString(c.Args[rowKey]) { - // allow passing row id directly (this can come in handy, but make sure it is a valid row id) - if !isValidID(c.Args[rowKey]) { - return errors.Errorf("row value must be a string or non-negative integer, but got: %v of %[1]T", c.Args[rowKey]) - } - } else if value := callArgString(c, rowKey); value != "" { - var id uint64 - if foreignIndexName != "" { - id = keyMaps[foreignIndexName][value] - } else { - if id, err = e.Cluster.translateFieldKey(ctx, field, value, writable); err != nil { - return errors.Wrapf(err, "translating field key: %s", value) - } - } - c.Args[rowKey] = id - } - } else { - if isString(c.Args[rowKey]) { - return errors.New("string 'row' value not allowed unless field 'keys' option enabled") - } - } - } - - // Translate child calls. - for _, child := range c.Children { - if err := e.translateCall(ctx, indexName, child, keyMaps, writable); err != nil { - return err - } - } - - // Translate call args. - for _, arg := range c.Args { - if arg, ok := arg.(*pql.Call); ok { - if err := e.translateCall(ctx, indexName, arg, keyMaps, writable); err != nil { - return errors.Wrap(err, "translating arg") - } - } - } - - // GroupBy-specific call translation. - if c.Name == "GroupBy" { - prev, ok := c.Args["previous"] - if !ok { - return nil // nothing else to be translated - } - previous, ok := prev.([]interface{}) - if !ok { - return errors.Errorf("'previous' argument must be list, but got %T", prev) - } - if len(c.Children) != len(previous) { - return errors.Errorf("mismatched lengths for previous: %d and children: %d in %s", len(previous), len(c.Children), c) - } - - fields := make([]*Field, len(c.Children)) - for i, child := range c.Children { - fieldname := callArgString(child, "_field") - field := idx.Field(fieldname) - if field == nil { - return errors.Wrapf(ErrFieldNotFound, "getting field '%s' from '%s'", fieldname, child) - } - fields[i] = field - } - - for i, field := range fields { - prev := previous[i] - if field.Keys() { - prevStr, ok := prev.(string) - if !ok { - return errors.New("prev value must be a string when field 'keys' option enabled") - } - // TODO: does this need to take field.ForeignIndex() into consideration? - id, err := e.Cluster.translateFieldKey(ctx, field, prevStr, writable) - if err != nil { - return errors.Wrapf(err, "translating field key: %s", prevStr) - } - previous[i] = id - } else { - if prevStr, ok := prev.(string); ok { - return errors.Errorf("got string row val '%s' in 'previous' for field %s which doesn't use string keys", prevStr, field.Name()) - } - } - } - } - - // This is workaround to support pql.ASSIGN ('=') as condition ('==') for int and decimal fields - if c.Name == "Row" && field != nil && - (field.Type() == FieldTypeInt || field.Type() == FieldTypeDecimal) { - // re-write args as conditions for fieldName - for k, v := range c.Args { - if _, ok := v.(*pql.Condition); k == fieldName && !ok { - c.Args[k] = &pql.Condition{ - Op: pql.EQ, - Value: v, - } - break - } - } - } - - return nil -} - func (e *executor) translateResults(ctx context.Context, index string, idx *Index, calls []*pql.Call, results []interface{}) (err error) { span, _ := tracing.StartSpanFromContext(ctx, "Executor.translateResults") defer span.Finish() diff --git a/executor_internal_test.go b/executor_internal_test.go index 534469864..82a905439 100644 --- a/executor_internal_test.go +++ b/executor_internal_test.go @@ -20,121 +20,11 @@ import ( "fmt" "io/ioutil" "strconv" - "strings" "testing" "github.com/pilosa/pilosa/v2/pql" ) -func TestExecutor_TranslateGroupByCall(t *testing.T) { - holder := NewHolder(DefaultPartitionN) - - cluster := NewTestCluster(1) - - e := &executor{ - Holder: holder, - Cluster: cluster, - } - e.Holder.Path, _ = ioutil.TempDir(*TempDir, "") - err := e.Holder.Open() - if err != nil { - t.Fatalf("opening holder: %v", err) - } - - idx, err := e.Holder.CreateIndex("i", IndexOptions{}) - if err != nil { - t.Fatalf("creating index: %v", err) - } - - _, erra := idx.CreateField("ak", OptFieldKeys()) - _, errb := idx.CreateField("b") - _, errc := idx.CreateField("ck", OptFieldKeys()) - if erra != nil || errb != nil || errc != nil { - t.Fatalf("creating fields %v, %v, %v", erra, errb, errc) - } - - query, err := pql.ParseString(`GroupBy(Rows(ak), Rows(b), Rows(ck), previous=["la", 0, "ha"], having=Condition(count > 10))`) - if err != nil { - t.Fatalf("parsing query: %v", err) - } - c := query.Calls[0] - // this is writable call just for testing purpose - to test previous argument - // generally GroupBy calls are not writable and keys should already exist - err = e.translateCall(context.Background(), "i", c, make(map[string]map[string]uint64), true) - if err != nil { - t.Fatalf("translating call: %v", err) - } - if len(c.Args["previous"].([]interface{})) != 3 { - t.Fatalf("unexpected length for 'previous' arg %v", c.Args["previous"]) - } - for i, v := range c.Args["previous"].([]interface{}) { - if !isInt(v) { - t.Fatalf("expected all items in previous to be ints, but '%v' at index %d is %[1]T", v, i) - } - } - - if having, hok := c.Args["having"].(*pql.Call); !hok { - t.Fatal("expected having to be a call") - } else if cond, cok := having.Args["count"].(*pql.Condition); !cok { - t.Fatal("expected condition to be a count") - } else if cond.Op != pql.GT { - t.Fatal("expected condition op to be >") - } else { - val, ok := cond.Uint64Value() - if !ok || val != uint64(10) { - t.Fatal("expected condition val to be uint64(10)") - } - } - - errTests := []struct { - pql string - err string - }{ - { - pql: `GroupBy(Rows(notfound), previous=1)`, - err: "'previous' argument must be list", - }, - { - pql: `GroupBy(Rows(ak), previous=["la", 0])`, - err: "mismatched lengths", - }, - { - pql: `GroupBy(Rows(ak), previous=[1])`, - err: "prev value must be a string", - }, - { - pql: `GroupBy(Rows(notfound), previous=[1])`, - err: ErrFieldNotFound.Error(), - }, - // TODO: an unknown key will actually allocate an id. this is probably bad. - // { - // pql: `GroupBy(Rows(ak), previous=["zoop"])`, - // err: "translating row key '", - // }, - { - pql: `GroupBy(Rows(b), previous=["la"])`, - err: "which doesn't use string keys", - }, - } - - for i, test := range errTests { - t.Run(fmt.Sprintf("#%d_%s", i, test.err), func(t *testing.T) { - query, err := pql.ParseString(test.pql) - if err != nil { - t.Fatalf("parsing query: %v", err) - } - c := query.Calls[0] - err = e.translateCall(context.Background(), "i", c, make(map[string]map[string]uint64), false) - if err == nil { - t.Fatalf("expected error, but translated call is '%s", c) - } - if !strings.Contains(err.Error(), test.err) { - t.Fatalf("expected '%s', got '%v'", test.err, err) - } - }) - } -} - func TestExecutor_TranslateRowsOnBool(t *testing.T) { holder := NewHolder(DefaultPartitionN) defer holder.Close() @@ -183,7 +73,11 @@ func TestExecutor_TranslateRowsOnBool(t *testing.T) { } c := query.Calls[0] - err = e.translateCall(context.Background(), "i", c, make(map[string]map[string]uint64), true) + colTranslations, rowTranslations, err := e.preTranslate(context.Background(), "i", c) + if err != nil { + t.Fatalf("pre-translating call: %v", err) + } + _, err = e.translateCallNew(c, "i", colTranslations, rowTranslations) if err != nil { t.Fatalf("translating call: %v", err) } diff --git a/executor_test.go b/executor_test.go index 636ac5369..c313286dc 100644 --- a/executor_test.go +++ b/executor_test.go @@ -932,7 +932,7 @@ func TestExecutor_Execute_SetValue(t *testing.T) { } t.Run("ColumnBSIGroupRequired", func(t *testing.T) { - if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(invalid_column_name=10, f=100)`}); err == nil || !hasCause(err, pilosa.ErrFieldNotFound) { + if _, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Set(f=100)`}); err == nil || errors.Cause(err).Error() != `Set() column argument 'col' required` { t.Fatalf("unexpected error: %s", err) } }) @@ -953,11 +953,12 @@ func TestExecutor_Execute_SetValue(t *testing.T) { func hasCause(err, cause error) bool { for err != cause { - eCause := errors.Cause(err) - if eCause == err { + innerErr := errors.Cause(err) + if innerErr == err { + // This is the innermost accessible error, and it does not have that cause. return false } - err = eCause + err = innerErr } return true diff --git a/http/client.go b/http/client.go index 1fa62f273..6ab718f81 100644 --- a/http/client.go +++ b/http/client.go @@ -1252,7 +1252,7 @@ func (c *InternalClient) FindIndexKeysNode(ctx context.Context, uri *pilosa.URI, defer span.Finish() // Create HTTP request. - u := uriPathToURL(uri, fmt.Sprintf("/internal/translate/index/%s/keys", index)) + u := uriPathToURL(uri, fmt.Sprintf("/internal/translate/index/%s/keys/find", index)) reqData, err := json.Marshal(keys) if err != nil { return nil, errors.Wrap(err, "marshalling request") @@ -1301,7 +1301,7 @@ func (c *InternalClient) FindFieldKeysNode(ctx context.Context, uri *pilosa.URI, defer span.Finish() // Create HTTP request. - u := uriPathToURL(uri, fmt.Sprintf("/internal/translate/field/%s/%s/keys", index, field)) + u := uriPathToURL(uri, fmt.Sprintf("/internal/translate/field/%s/%s/keys/find", index, field)) q := u.Query() q.Add("remote", "true") u.RawQuery = q.Encode() @@ -1353,7 +1353,7 @@ func (c *InternalClient) CreateIndexKeysNode(ctx context.Context, uri *pilosa.UR defer span.Finish() // Create HTTP request. - u := uriPathToURL(uri, fmt.Sprintf("/internal/translate/index/%s/keys", index)) + u := uriPathToURL(uri, fmt.Sprintf("/internal/translate/index/%s/keys/create", index)) reqData, err := json.Marshal(keys) if err != nil { return nil, errors.Wrap(err, "marshalling request") @@ -1402,7 +1402,7 @@ func (c *InternalClient) CreateFieldKeysNode(ctx context.Context, uri *pilosa.UR defer span.Finish() // Create HTTP request. - u := uriPathToURL(uri, fmt.Sprintf("/internal/translate/field/%s/%s/keys", index, field)) + u := uriPathToURL(uri, fmt.Sprintf("/internal/translate/field/%s/%s/keys/create", index, field)) q := u.Query() q.Add("remote", "true") u.RawQuery = q.Encode() diff --git a/http/handler.go b/http/handler.go index 5792d0e2b..8b47f8edd 100644 --- a/http/handler.go +++ b/http/handler.go @@ -385,10 +385,10 @@ func newRouter(handler *Handler) *mux.Router { router.HandleFunc("/internal/nodes", handler.handleGetNodes).Methods("GET").Name("GetNodes") router.HandleFunc("/internal/shards/max", handler.handleGetShardsMax).Methods("GET").Name("GetShardsMax") // TODO: deprecate, but it's being used by the client - router.HandleFunc("/internal/translate/index/{index}/keys", handler.handleFindIndexKeys).Methods("POST").Name("FindIndexKeys") - router.HandleFunc("/internal/translate/index/{index}/keys", handler.handleCreateIndexKeys).Methods("PUT").Name("CreateIndexKeys") - router.HandleFunc("/internal/translate/field/{index}/{field}/keys", handler.handleFindFieldKeys).Methods("POST").Name("FindFieldKeys") - router.HandleFunc("/internal/translate/field/{index}/{field}/keys", handler.handleCreateFieldKeys).Methods("PUT").Name("CreateFieldKeys") + router.HandleFunc("/internal/translate/index/{index}/keys/find", handler.handleFindIndexKeys).Methods("POST").Name("FindIndexKeys") + router.HandleFunc("/internal/translate/index/{index}/keys/create", handler.handleCreateIndexKeys).Methods("PUT").Name("CreateIndexKeys") + router.HandleFunc("/internal/translate/field/{index}/{field}/keys/find", handler.handleFindFieldKeys).Methods("POST").Name("FindFieldKeys") + router.HandleFunc("/internal/translate/field/{index}/{field}/keys/create", handler.handleCreateFieldKeys).Methods("PUT").Name("CreateFieldKeys") router.Use(handler.queryArgValidator) router.Use(handler.addQueryContext) diff --git a/translator_test.go b/translator_test.go index cb48ec7bf..334c301a0 100644 --- a/translator_test.go +++ b/translator_test.go @@ -504,7 +504,7 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { // Create some keys on each node. var g errgroup.Group - defer g.Wait() + defer g.Wait() //nolint:errcheck for i, keys := range parts { i, keys := i, keys g.Go(func() error { @@ -540,7 +540,7 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { // Check that all nodes agree on these translations. var g errgroup.Group - defer g.Wait() + defer g.Wait() //nolint:errcheck for i, n := range c.Nodes { i, api := i, n.API g.Go(func() (err error) { @@ -549,22 +549,7 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { if err != nil { return errors.Wrap(err, "finding translations") } - for key, id := range localTranslations { - if realID, ok := translations[key]; !ok { - return errors.Errorf("unexpected key %q mapped to ID %d", key, id) - } else if id != realID { - return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) - } - } - for key, realID := range translations { - if id, ok := localTranslations[key]; !ok { - return errors.Errorf("missing translation of key %q", key) - } else if id != realID { - // This should not be necessary, but do it just to be safe. - return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) - } - } - return nil + return compareTranslations(translations, localTranslations) }) } if err := g.Wait(); err != nil { @@ -581,22 +566,7 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { if err != nil { return errors.Wrap(err, "finding translations") } - for key, id := range localTranslations { - if realID, ok := translations[key]; !ok { - return errors.Errorf("unexpected key %q mapped to ID %d", key, id) - } else if id != realID { - return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) - } - } - for key, realID := range translations { - if id, ok := localTranslations[key]; !ok { - return errors.Errorf("missing translation of key %q", key) - } else if id != realID { - // This should not be necessary, but do it just to be safe. - return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) - } - } - return nil + return compareTranslations(translations, localTranslations) }) } if err := g.Wait(); err != nil { @@ -620,7 +590,7 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { // Create some keys on each node. var g errgroup.Group - defer g.Wait() + defer g.Wait() //nolint:errcheck for i, keys := range parts { i, keys := i, keys g.Go(func() error { @@ -656,7 +626,7 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { // Check that all nodes agree on these translations. var g errgroup.Group - defer g.Wait() + defer g.Wait() //nolint:errcheck for i, n := range c.Nodes { i, api := i, n.API g.Go(func() (err error) { @@ -665,22 +635,7 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { if err != nil { return errors.Wrap(err, "finding translations") } - for key, id := range localTranslations { - if realID, ok := translations[key]; !ok { - return errors.Errorf("unexpected key %q mapped to ID %d", key, id) - } else if id != realID { - return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) - } - } - for key, realID := range translations { - if id, ok := localTranslations[key]; !ok { - return errors.Errorf("missing translation of key %q", key) - } else if id != realID { - // This should not be necessary, but do it just to be safe. - return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) - } - } - return nil + return compareTranslations(translations, localTranslations) }) } if err := g.Wait(); err != nil { @@ -697,22 +652,7 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { if err != nil { return errors.Wrap(err, "finding translations") } - for key, id := range localTranslations { - if realID, ok := translations[key]; !ok { - return errors.Errorf("unexpected key %q mapped to ID %d", key, id) - } else if id != realID { - return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) - } - } - for key, realID := range translations { - if id, ok := localTranslations[key]; !ok { - return errors.Errorf("missing translation of key %q", key) - } else if id != realID { - // This should not be necessary, but do it just to be safe. - return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) - } - } - return nil + return compareTranslations(translations, localTranslations) }) } if err := g.Wait(); err != nil { @@ -722,4 +662,22 @@ func TestTranslation_Cluster_CreateFind(t *testing.T) { } }) } + +func compareTranslations(expected, got map[string]uint64) error { + for key, id := range got { + if realID, ok := expected[key]; !ok { + return errors.Errorf("unexpected key %q mapped to ID %d", key, id) + } else if id != realID { + return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) + } + } + for key, realID := range expected { + if id, ok := got[key]; !ok { + return errors.Errorf("missing translation of key %q", key) + } else if id != realID { + return errors.Errorf("mismatched translation: expected %q:%d but got %q:%d", key, realID, key, id) + } + } + return nil +} */