From a59bb8d65f6e9b3242aafc1b8d12ab7b489a4e1e Mon Sep 17 00:00:00 2001 From: Gang Zhang Date: Thu, 20 Jun 2019 18:24:27 -0700 Subject: [PATCH 1/2] Add a 'defunct' tag for Index definitions that define the index is defunct or not --- CHANGELOG.md | 2 +- entity.go | 3 +++ entity_parser.go | 45 +++++++++++++++++++++++++------- entity_parser_index_test.go | 44 +++++++++++++++++++++++++++++++ entity_parser_key_parser_test.go | 40 +++++++++++++++++++++++++++- entity_test.go | 1 + finder.go | 8 +++--- finder_test.go | 1 + 8 files changed, 128 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08ca9490..a4e60031 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Changelog ## v3.4.7 (unreleased) - - Nothing changed yet + - Add optional 'defunct' tag to Index definitions ## v3.4.6 (2019-06-11) - use append but not copy to clone diff --git a/entity.go b/entity.go index beba121b..0f80cb43 100644 --- a/entity.go +++ b/entity.go @@ -168,6 +168,7 @@ func (cd *ColumnDefinition) Clone() *ColumnDefinition { type IndexDefinition struct { Key *PrimaryKey Columns []string + Defunct bool } // Clone returns a deep copy of IndexDefinition @@ -176,9 +177,11 @@ func (id *IndexDefinition) Clone() *IndexDefinition { for _, c := range id.Columns { columns = append(columns, c) } + defunct := id.Defunct return &IndexDefinition{ Key: id.Key.Clone(), Columns: columns, + Defunct: defunct, } } diff --git a/entity_parser.go b/entity_parser.go index 1594e8a2..2f935ab6 100644 --- a/entity_parser.go +++ b/entity_parser.go @@ -24,6 +24,7 @@ import ( "fmt" "reflect" "regexp" + "strconv" "strings" "time" "unicode" @@ -51,6 +52,8 @@ var ( columnsPattern = regexp.MustCompile(`columns\s*=\s*\(([^\(\)]+)\)`) + defunctPattern = regexp.MustCompile(`defunct\s*=\s*(true|false)`) + etlPattern0 = regexp.MustCompile(`etl\s*=\s*(\S*)`) ttlPattern0 = regexp.MustCompile(`ttl\s*=\s*(\S*)`) @@ -208,14 +211,14 @@ func TableFromInstance(object DomainObject) (*Table, error) { } else { // parse index fields if structField.Type == indexType { - indexName, indexKey, indexColumns, err := parseIndexTag(structField.Name, tag) + indexName, indexKey, indexColumns, defunct, err := parseIndexTag(structField.Name, tag) if err != nil { return nil, err } if _, exist := t.Indexes[indexName]; exist { return nil, errors.Errorf("index name is duplicated: %s", indexName) } - t.Indexes[indexName] = &IndexDefinition{Key: indexKey, Columns: indexColumns} + t.Indexes[indexName] = &IndexDefinition{Key: indexKey, Columns: indexColumns, Defunct: defunct} } else { cd, err := parseFieldTag(structField, tag) if err != nil { @@ -278,13 +281,13 @@ func translateKeyName(t *Table) { } // parseIndexTag functions parses DOSA index tag -func parseIndexTag(indexName, dosaAnnotation string) (string, *PrimaryKey, []string, error) { +func parseIndexTag(indexName, dosaAnnotation string) (string, *PrimaryKey, []string, bool, error) { // index name struct must be exported in the entity, // otherwise it will be ignored when upserting the schema. if len(indexName) != 0 && unicode.IsLower([]rune(indexName)[0]) { expected := []rune(indexName) expected[0] = unicode.ToUpper(expected[0]) - return "", nil, nil, fmt.Errorf("index name (%s) must be exported, "+ + return "", nil, nil, false, fmt.Errorf("index name (%s) must be exported, "+ "try (%s) instead", indexName, string(expected)) } tag := dosaAnnotation @@ -292,12 +295,12 @@ func parseIndexTag(indexName, dosaAnnotation string) (string, *PrimaryKey, []str // find the primaryKey matchs := indexKeyPattern0.FindStringSubmatch(tag) if len(matchs) != 4 { - return "", nil, nil, fmt.Errorf("dosa.Index %s with an invalid dosa index tag %q", indexName, tag) + return "", nil, nil, false, fmt.Errorf("dosa.Index %s with an invalid dosa index tag %q", indexName, tag) } pkString := matchs[1] key, err := parsePrimaryKey(indexName, pkString) if err != nil { - return "", nil, nil, errors.Wrapf(err, "struct %s has an invalid index key %q", indexName, pkString) + return "", nil, nil, false, errors.Wrapf(err, "struct %s has an invalid index key %q", indexName, pkString) } toRemove := strings.TrimSuffix(matchs[0], matchs[2]) toRemove = strings.TrimSuffix(matchs[0], matchs[3]) @@ -306,23 +309,30 @@ func parseIndexTag(indexName, dosaAnnotation string) (string, *PrimaryKey, []str //find the name fullNameTag, name, err := parseNameTag(tag, indexName) if err != nil { - return "", nil, nil, errors.Wrapf(err, "invalid name tag: %s", tag) + return "", nil, nil, false, errors.Wrapf(err, "invalid name tag: %s", tag) } tag = strings.Replace(tag, fullNameTag, "", 1) // find the columns fullColumnsTag, indexColumns, err := parseColumnsTag(tag) if err != nil { - return "", nil, nil, errors.Wrapf(err, "invalid columns tag: %s", tag) + return "", nil, nil, false, errors.Wrapf(err, "invalid columns tag: %s", tag) } tag = strings.Replace(tag, fullColumnsTag, "", 1) + // find the defunct + fullDefunctTag, defunct, err := parseDefunctTag(tag) + if err != nil { + return "", nil, nil, false, errors.Wrapf(err, "invalid defunct tag: %s", tag) + } + tag = strings.Replace(tag, fullDefunctTag, "", 1) + tag = strings.TrimSpace(tag) if tag != "" { - return "", nil, nil, fmt.Errorf("index field %s with an invalid dosa index tag: %s", indexName, tag) + return "", nil, nil, false, fmt.Errorf("index field %s with an invalid dosa index tag: %s", indexName, tag) } - return name, key, indexColumns, nil + return name, key, indexColumns, defunct, nil } // parseNameTag functions parses DOSA "name" tag @@ -372,6 +382,21 @@ func parseColumnsTag(tag string) (string, []string, error) { return fullColumnsTag, indexColumns, nil } +// parseDefunctTag parses the "defunct" tag of a dosa.Index in the entity. It returns +// the matched section of the tag string and boolean value of "defunct" +func parseDefunctTag(tag string) (string, bool, error) { + matches := defunctPattern.FindStringSubmatch(tag) + if len(matches) == 2 { + fullDefunctTag := matches[0] + defunct, err := strconv.ParseBool(matches[1]) + if err != nil { + return "", false, err + } + return fullDefunctTag, defunct, nil + } + return "", false, nil +} + // parseETLTag functions parses DOSA "etl" tag func parseETLTag(tag string) (string, ETLState, error) { fullETLTag := "" diff --git a/entity_parser_index_test.go b/entity_parser_index_test.go index d49b656c..6ef8314b 100644 --- a/entity_parser_index_test.go +++ b/entity_parser_index_test.go @@ -155,3 +155,47 @@ func TestIndexesWithColumnsTag(t *testing.T) { }, }, dosaTable.Indexes) } + +type IndexesWithDefunctTag struct { + Entity `dosa:"primaryKey=(ID)"` + SearchByCity Index `dosa:"key=(City, Payload) columns=(ID) defunct=true"` + SearchByID Index `dosa:"key=(City) columns=(ID, Payload) defunct=false"` + SearchByPayload Index `dosa:"key=Payload"` + + ID UUID + City string + Payload []byte +} + +func TestIndexWithDefunctTag(t *testing.T) { + dosaTable, err := TableFromInstance(&IndexesWithDefunctTag{}) + assert.Nil(t, err) + assert.Equal(t, map[string]*IndexDefinition{ + "searchbycity": { + Key: &PrimaryKey{ + PartitionKeys: []string{"city"}, + ClusteringKeys: []*ClusteringKey{ + { + Name: "payload", + Descending: false, + }, + }, + }, + Columns: []string{"id"}, + Defunct: true, + }, + "searchbyid": { + Key: &PrimaryKey{ + PartitionKeys: []string{"city"}, + }, + Columns: []string{"id", "payload"}, + Defunct: false, + }, + "searchbypayload": { + Key: &PrimaryKey{ + PartitionKeys: []string{"payload"}, + }, + Defunct: false, + }, + }, dosaTable.Indexes) +} diff --git a/entity_parser_key_parser_test.go b/entity_parser_key_parser_test.go index fec35cb1..48903aed 100644 --- a/entity_parser_key_parser_test.go +++ b/entity_parser_key_parser_test.go @@ -740,6 +740,7 @@ func TestIndexParse(t *testing.T) { ExpectedIndexName string PrimaryKey *PrimaryKey Columns []string + Defunct bool Error error }{ { @@ -959,10 +960,46 @@ func TestIndexParse(t *testing.T) { Columns: []string{"ok", "test", "hi"}, Error: errors.New("index field SearchByKey with an invalid dosa index tag: columns=(ok, test, (hi),)"), }, + { + Tag: "name=jj key=ok columns=(ok, test, hi,) defunct=true", + ExpectedIndexName: "jj", + PrimaryKey: &PrimaryKey{ + PartitionKeys: []string{"ok"}, + ClusteringKeys: nil, + }, + InputIndexName: "SearchByKey", + Columns: []string{"ok", "test", "hi"}, + Defunct: true, + Error: nil, + }, + { + Tag: "defunct = true name=jj key=ok columns=(ok, test, hi,)", + ExpectedIndexName: "jj", + PrimaryKey: &PrimaryKey{ + PartitionKeys: []string{"ok"}, + ClusteringKeys: nil, + }, + InputIndexName: "SearchByKey", + Columns: []string{"ok", "test", "hi"}, + Defunct: true, + Error: nil, + }, + { + Tag: "name=jj key=ok columns=(ok, test, hi,) defunct=false", + ExpectedIndexName: "jj", + PrimaryKey: &PrimaryKey{ + PartitionKeys: []string{"ok"}, + ClusteringKeys: nil, + }, + InputIndexName: "SearchByKey", + Columns: []string{"ok", "test", "hi"}, + Defunct: false, + Error: nil, + }, } for _, d := range data { - name, primaryKey, columns, err := parseIndexTag(d.InputIndexName, d.Tag) + name, primaryKey, columns, defunct, err := parseIndexTag(d.InputIndexName, d.Tag) if d.Error != nil { assert.Contains(t, err.Error(), d.Error.Error()) } else { @@ -970,6 +1007,7 @@ func TestIndexParse(t *testing.T) { assert.Equal(t, name, d.ExpectedIndexName) assert.Equal(t, primaryKey, d.PrimaryKey) assert.Equal(t, columns, d.Columns) + assert.Equal(t, defunct, d.Defunct) } } } diff --git a/entity_test.go b/entity_test.go index afc6da01..5e3d100f 100644 --- a/entity_test.go +++ b/entity_test.go @@ -365,6 +365,7 @@ func getValidEntityDefinition() *dosa.EntityDefinition { PartitionKeys: []string{"bar"}, }, Columns: []string{"qux", "foo"}, + Defunct: true, }, }, Columns: []*dosa.ColumnDefinition{ diff --git a/finder.go b/finder.go index 77ea8a2d..d804b059 100644 --- a/finder.go +++ b/finder.go @@ -263,14 +263,14 @@ func tableFromStructType(structName string, structType *ast.StructType, packageP for _, fieldName := range field.Names { name := fieldName.Name if kind == packagePrefix+"."+indexName || (packagePrefix == "" && kind == indexName) { - indexName, indexKey, indexColumns, err := parseIndexTag(name, dosaTag) + indexName, indexKey, indexColumns, defunct, err := parseIndexTag(name, dosaTag) if err != nil { return nil, err } if _, exist := t.Indexes[indexName]; exist { return nil, errors.Errorf("index name is duplicated: %s", indexName) } - t.Indexes[indexName] = &IndexDefinition{Key: indexKey, Columns: indexColumns} + t.Indexes[indexName] = &IndexDefinition{Key: indexKey, Columns: indexColumns, Defunct: defunct} } else { firstRune, _ := utf8.DecodeRuneInString(name) if unicode.IsLower(firstRune) { @@ -293,14 +293,14 @@ func tableFromStructType(structName string, structType *ast.StructType, packageP if len(field.Names) == 0 { if kind == packagePrefix+"."+indexName || (packagePrefix == "" && kind == indexName) { - indexName, indexKey, indexColumns, err := parseIndexTag("", dosaTag) + indexName, indexKey, indexColumns, defunct, err := parseIndexTag("", dosaTag) if err != nil { return nil, err } if _, exist := t.Indexes[indexName]; exist { return nil, errors.Errorf("index name is duplicated: %s", indexName) } - t.Indexes[indexName] = &IndexDefinition{Key: indexKey, Columns: indexColumns} + t.Indexes[indexName] = &IndexDefinition{Key: indexKey, Columns: indexColumns, Defunct: defunct} } } } diff --git a/finder_test.go b/finder_test.go index 4eb249f5..959ad6d6 100644 --- a/finder_test.go +++ b/finder_test.go @@ -77,6 +77,7 @@ func TestParser(t *testing.T) { "complexindexes": &ComplexIndexes{}, "scopemetadata": &ScopeMetadata{}, "indexeswithcolumnstag": &IndexesWithColumnsTag{}, + "indexeswithdefuncttag": &IndexesWithDefunctTag{}, } entitiesExcludedForTest := map[string]interface{}{ "clienttestentity1": struct{}{}, // skip, see https://jira.uberinternal.com/browse/DOSA-788 From 77ea1df1ff118228045f663a605066aea9b77f69 Mon Sep 17 00:00:00 2001 From: Gang Zhang Date: Fri, 21 Jun 2019 11:47:43 -0700 Subject: [PATCH 2/2] update idl to support defunct tag in yarpc --- connectors/yarpc/helpers.go | 3 ++- glide.lock | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/connectors/yarpc/helpers.go b/connectors/yarpc/helpers.go index ad4a5fe7..f976fbf2 100644 --- a/connectors/yarpc/helpers.go +++ b/connectors/yarpc/helpers.go @@ -240,7 +240,7 @@ func entityDefToThrift(ed *dosa.EntityDefinition) *dosarpc.EntityDefinition { indexes := make(map[string]*dosarpc.IndexDefinition) for name, index := range ed.Indexes { pkI := PrimaryKeyToThrift(index.Key) - indexes[name] = &dosarpc.IndexDefinition{Key: pkI, Columns: index.Columns} + indexes[name] = &dosarpc.IndexDefinition{Key: pkI, Columns: index.Columns, Defunct: &index.Defunct} } etl := ETLStateToThrift(ed.ETL) @@ -288,6 +288,7 @@ func FromThriftToEntityDefinition(ed *dosarpc.EntityDefinition) *dosa.EntityDefi indexes[name] = &dosa.IndexDefinition{ Key: FromThriftToPrimaryKey(index.Key), Columns: index.Columns, + Defunct: *index.Defunct, } } diff --git a/glide.lock b/glide.lock index bf92cd21..b71c267a 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ hash: 3a161b2246b45ecccd550c70486c37361650084e22a34da58be7d980f0f38c1e -updated: 2019-05-30T11:49:02.779243-07:00 +updated: 2019-06-21T11:22:09.666001-07:00 imports: - name: github.com/anmitsu/go-shlex version: 648efa622239a2f6ff949fed78ee37b48d499ba4 @@ -120,9 +120,9 @@ imports: subpackages: - internal/mapstructure - name: github.com/uber-go/tally - version: 24c699f78afd17db5aac42f83c1c5cad70254294 + version: f266f90e9c4d5894364039a324a05d061f2f34e2 - name: github.com/uber/dosa-idl - version: 097e20c83d25cc733a977da96a6894cc9434fb99 + version: 25b8c1aa8ed54ed3cd177d07761a8b376605dbdc subpackages: - .gen/dosa - .gen/dosa/dosaclient @@ -148,7 +148,7 @@ imports: - push - tallypush - name: go.uber.org/thriftrw - version: f3ff6fef5b56823cf763aebc81b42b8cbd1082c1 + version: 23f23e7fc269002cb7e34f4e2dc079593cd6ad36 subpackages: - ast - compile