fix: tag filter with no value revisit

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(

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) {

pkg/util/tagutils/filters_test.go

@@ -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))
-		}
-	}
-}