From 006174bd2b6173da78b079bfd6e6ec401a9e00ea Mon Sep 17 00:00:00 2001 From: Qiu Jian Date: Tue, 12 Mar 2024 20:46:04 +0800 Subject: [PATCH] fix: tag filter with no value revisit --- pkg/cloudcommon/db/metadataresource.go | 58 +++++++++++------ pkg/util/tagutils/filters.go | 40 +----------- pkg/util/tagutils/filters_test.go | 89 -------------------------- 3 files changed, 39 insertions(+), 148 deletions(-) delete mode 100644 pkg/util/tagutils/filters_test.go diff --git a/pkg/cloudcommon/db/metadataresource.go b/pkg/cloudcommon/db/metadataresource.go index a783ccb14a4..e8133a84d45 100644 --- a/pkg/cloudcommon/db/metadataresource.go +++ b/pkg/cloudcommon/db/metadataresource.go @@ -21,6 +21,7 @@ import ( "yunion.io/x/jsonutils" "yunion.io/x/log" "yunion.io/x/pkg/util/rbacscope" + "yunion.io/x/pkg/utils" "yunion.io/x/sqlchemy" "yunion.io/x/onecloud/pkg/apis" @@ -58,23 +59,30 @@ func ObjectIdQueryWithPolicyResult(ctx context.Context, q *sqlchemy.SQuery, mana } func ObjectIdQueryWithTagFilters(ctx context.Context, q *sqlchemy.SQuery, idField string, modelName string, filters tagutils.STagFilters) *sqlchemy.SQuery { - if len(filters.Filters) > 0 { - sq := objIdQueryWithTags(ctx, modelName, filters.Filters) - if sq != nil { - sqq := sq.SubQuery() - q = q.Join(sqq, sqlchemy.Equals(q.Field(idField), sqq.Field("obj_id"))) + if len(filters.Filters) > 0 || len(filters.NoFilters) > 0 { + idSubQ := q.Copy().SubQuery().Query() + idSubQ.AppendField(sqlchemy.DISTINCT(idField, idSubQ.Field(idField))) + subQ := idSubQ.SubQuery() + if len(filters.Filters) > 0 { + sq := objIdQueryWithTags(ctx, subQ, idField, modelName, filters.Filters) + if sq != nil { + sqq := sq.SubQuery() + q = q.Join(sqq, sqlchemy.Equals(q.Field(idField), sqq.Field(idField))) + } } - } - if len(filters.NoFilters) > 0 { - sq := objIdQueryWithTags(ctx, modelName, filters.NoFilters) - if sq != nil { - q = q.Filter(sqlchemy.NotIn(q.Field(idField), sq.SubQuery())) + if len(filters.NoFilters) > 0 { + sq := objIdQueryWithTags(ctx, subQ, idField, modelName, filters.NoFilters) + if sq != nil { + sqq := sq.SubQuery() + q = q.LeftJoin(sqq, sqlchemy.Equals(q.Field(idField), sqq.Field(idField))) + q = q.Filter(sqlchemy.IsNull(sqq.Field(idField))) + } } } return q } -func objIdQueryWithTags(ctx context.Context, modelName string, tagsList []map[string][]string) *sqlchemy.SQuery { +func objIdQueryWithTags(ctx context.Context, objIdSubQ *sqlchemy.SSubQuery, idField string, modelName string, tagsList []map[string][]string) *sqlchemy.SQuery { manager := GetMetadaManagerInContext(ctx) metadataResQ := manager.Query().Equals("obj_type", modelName).SubQuery() @@ -83,29 +91,37 @@ func objIdQueryWithTags(ctx context.Context, modelName string, tagsList []map[st if len(tags) == 0 { continue } - metadataView := metadataResQ.Query(metadataResQ.Field("obj_id").Label("obj_id")) + objIdQ := objIdSubQ.Query() + objIdQ = objIdQ.AppendField(objIdQ.Field(idField)) for key, val := range tags { - q := metadataResQ.Query(metadataResQ.Field("id")) - q = q.Equals("key", key) + sq := metadataResQ.Query().Equals("key", key).SubQuery() + objIdQ = objIdQ.LeftJoin(sq, sqlchemy.Equals(objIdQ.Field(idField), sq.Field("obj_id"))) if len(val) > 0 { - q = q.In("value", val) + if utils.IsInArray(tagutils.NoValue, val) { + objIdQ = objIdQ.Filter(sqlchemy.OR( + sqlchemy.In(sq.Field("value"), val), + sqlchemy.IsNull(sq.Field("value")), + )) + } else { + objIdQ = objIdQ.Filter(sqlchemy.In(sq.Field("value"), val)) + } + } else { + objIdQ = objIdQ.Filter(sqlchemy.IsNotNull(sq.Field("obj_id"))) } - sq := q.SubQuery() - metadataView = metadataView.Join(sq, sqlchemy.Equals(metadataView.Field("id"), sq.Field("id"))) } - queries = append(queries, metadataView.Distinct()) + queries = append(queries, objIdQ.Distinct()) } if len(queries) == 0 { return nil } - var query sqlchemy.IQuery + var query *sqlchemy.SQuery if len(queries) == 1 { - query = queries[0] + query = queries[0].(*sqlchemy.SQuery) } else { uq, _ := sqlchemy.UnionWithError(queries...) query = uq.Query() } - return query.SubQuery().Query() + return query } func (meta *SMetadataResourceBaseModelManager) ListItemFilter( diff --git a/pkg/util/tagutils/filters.go b/pkg/util/tagutils/filters.go index 457a43c7014..bbd7fd3145c 100644 --- a/pkg/util/tagutils/filters.go +++ b/pkg/util/tagutils/filters.go @@ -30,48 +30,12 @@ func splitValues(values []string) (bool, []string) { } } -func (ts TTagSet) toFilters() (map[string][]string, map[string][]string) { - filter := tagset2Map(ts) - pos := make(map[string][]string) - neg := make(map[string][]string) - negExist := false - for k, v := range filter { - noval, values := splitValues(v) - if noval { - negExist = true - if len(values) > 0 { - pos[k] = values - } - neg[k] = []string{} - } else { - pos[k] = values - neg[k] = values - } - } - if !negExist { - neg = nil - } - return pos, neg -} - func (tf *STagFilters) AddFilter(ts TTagSet) { - pos, neg := ts.toFilters() - if pos != nil { - tf.Filters = append(tf.Filters, pos) - } - if neg != nil { - tf.NoFilters = append(tf.NoFilters, neg) - } + tf.Filters = append(tf.Filters, tagset2Map(ts)) } func (tf *STagFilters) AddNoFilter(ts TTagSet) { - pos, neg := ts.toFilters() - if pos != nil { - tf.NoFilters = append(tf.NoFilters, pos) - } - if neg != nil { - tf.Filters = append(tf.Filters, neg) - } + tf.NoFilters = append(tf.NoFilters, tagset2Map(ts)) } func (tf *STagFilters) AddFilters(tsl TTagSetList) { diff --git a/pkg/util/tagutils/filters_test.go b/pkg/util/tagutils/filters_test.go deleted file mode 100644 index 95cfa01ac8e..00000000000 --- a/pkg/util/tagutils/filters_test.go +++ /dev/null @@ -1,89 +0,0 @@ -// Copyright 2019 Yunion -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package tagutils - -import ( - "reflect" - "testing" - - "yunion.io/x/jsonutils" -) - -func TestTagSetToFilters(t *testing.T) { - cases := []struct { - ts TTagSet - pos map[string][]string - neg map[string][]string - }{ - { - ts: TTagSet{ - STag{ - Key: "project", - Value: "a", - }, - }, - pos: map[string][]string{ - "project": { - "a", - }, - }, - neg: nil, - }, - { - ts: TTagSet{ - STag{ - Key: "project", - Value: NoValue, - }, - }, - pos: map[string][]string{}, - neg: map[string][]string{ - "project": {}, - }, - }, - { - ts: TTagSet{ - STag{ - Key: "env", - Value: "c", - }, - STag{ - Key: "project", - Value: NoValue, - }, - }, - pos: map[string][]string{ - "env": { - "c", - }, - }, - neg: map[string][]string{ - "env": { - "c", - }, - "project": {}, - }, - }, - } - for i, c := range cases { - gotPos, gotNeg := c.ts.toFilters() - if !reflect.DeepEqual(c.pos, gotPos) { - t.Errorf("[%d] filters want: %s got: %s", i, jsonutils.Marshal(c.pos), jsonutils.Marshal(gotPos)) - } - if !reflect.DeepEqual(c.neg, gotNeg) { - t.Errorf("[%d] nofilters want %s got %s", i, jsonutils.Marshal(c.neg), jsonutils.Marshal(gotNeg)) - } - } -}