Fix a bug where filter wasn't being serialized properly

backend/src/apiserver/filter/BUILD.bazel

@@ -8,6 +8,7 @@ go_library(
     deps = [
         "//backend/api:go_default_library",
         "//backend/src/common/util:go_default_library",
+        "@com_github_golang_protobuf//jsonpb:go_default_library_gen",
         "@com_github_golang_protobuf//ptypes:go_default_library_gen",
         "@com_github_masterminds_squirrel//:go_default_library",
     ],

backend/src/apiserver/filter/filter.go

@@ -17,9 +17,11 @@
 package filter
 
 import (
+	"encoding/json"
 	"fmt"
 
 	"github.com/Masterminds/squirrel"
+	"github.com/golang/protobuf/jsonpb"
 	"github.com/golang/protobuf/ptypes"
 	"github.com/kubeflow/pipelines/backend/src/common/util"
 
@@ -43,6 +45,69 @@ type Filter struct {
 	substring map[string]interface{}
 }
 
+// filterForMarshaling is a helper struct for marshaling Filter into JSON. This
+// is needed as we don't want to export the fields in Filter.
+type filterForMarshaling struct {
+	FilterProto string
+
+	EQ  map[string]interface{}
+	NEQ map[string]interface{}
+	GT  map[string]interface{}
+	GTE map[string]interface{}
+	LT  map[string]interface{}
+	LTE map[string]interface{}
+
+	IN map[string]interface{}
+
+	SUBSTRING map[string]interface{}
+}
+
+// MarshalJSON implements JSON Marshaler for Filter.
+func (f *Filter) MarshalJSON() ([]byte, error) {
+	m := &jsonpb.Marshaler{}
+	s, err := m.MarshalToString(f.filterProto)
+	if err != nil {
+		return nil, err
+	}
+	return json.Marshal(&filterForMarshaling{
+		FilterProto: s,
+		EQ:          f.eq,
+		NEQ:         f.neq,
+		GT:          f.gt,
+		GTE:         f.gte,
+		LT:          f.lt,
+		LTE:         f.lte,
+		IN:          f.in,
+		SUBSTRING:   f.substring,
+	})
+}
+
+// UnmarshalJSON implements JSON Unmarshaler for Filter.
+func (f *Filter) UnmarshalJSON(b []byte) error {
+	ffm := filterForMarshaling{}
+	err := json.Unmarshal(b, &ffm)
+	if err != nil {
+		return err
+	}
+
+	f.filterProto = &api.Filter{}
+	err = jsonpb.UnmarshalString(ffm.FilterProto, f.filterProto)
+	if err != nil {
+		return err
+	}
+
+	f.eq = ffm.EQ
+	f.neq = ffm.NEQ
+	f.gt = ffm.GT
+	f.gte = ffm.GTE
+	f.lt = ffm.LT
+	f.lte = ffm.LTE
+	f.in = ffm.IN
+	f.substring = ffm.SUBSTRING
+
+	return nil
+}
+
 // New creates a new Filter from parsing the API filter protocol buffer.
 func New(filterProto *api.Filter) (*Filter, error) {
 	f := &Filter{

backend/src/apiserver/filter/filter_test.go

@@ -1,6 +1,7 @@
 package filter
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/Masterminds/squirrel"
@@ -235,3 +236,46 @@ func TestAddToSelect(t *testing.T) {
 		}
 	}
 }
+
+func TestMarshalJSON(t *testing.T) {
+	f := &Filter{
+		filterProto: &api.Filter{
+			Predicates: []*api.Predicate{
+				&api.Predicate{
+					Key: "Name", Op: api.Predicate_EQUALS,
+					Value: &api.Predicate_StringValue{StringValue: "SomeName"},
+				},
+			},
+		},
+		eq: map[string]interface{}{"name": "SomeName"},
+	}
+
+	want := `{"FilterProto":"{\"predicates\":[{\"op\":\"EQUALS\",\"key\":\"Name\",\"stringValue\":\"SomeName\"}]}","EQ":{"name":"SomeName"},"NEQ":null,"GT":null,"GTE":null,"LT":null,"LTE":null,"IN":null,"SUBSTRING":null}`
+
+	got, err := json.Marshal(f)
+	if err != nil || string(got) != want {
+		t.Errorf("json.Marshal(%+v):\n%s, %v\nWant:%s, nil error\n", f, got, err, want)
+	}
+}
+
+func TestUnmarshalJSON(t *testing.T) {
+	in := `{"FilterProto":"{\"predicates\":[{\"op\":\"EQUALS\",\"key\":\"Name\",\"stringValue\":\"SomeName\"}]}","EQ":{"name":"SomeName"},"NEQ":null,"GT":null,"GTE":null,"LT":null,"LTE":null,"IN":null,"SUBSTRING":null}`
+
+	want := &Filter{
+		filterProto: &api.Filter{
+			Predicates: []*api.Predicate{
+				&api.Predicate{
+					Key: "Name", Op: api.Predicate_EQUALS,
+					Value: &api.Predicate_StringValue{StringValue: "SomeName"},
+				},
+			},
+		},
+		eq: map[string]interface{}{"name": "SomeName"},
+	}
+
+	got := &Filter{}
+	err := json.Unmarshal([]byte(in), got)
+	if err != nil || !cmp.Equal(got, want, cmp.AllowUnexported(Filter{})) {
+		t.Errorf("json.Unmarshal(%+v):\nGot: %v, Error: %v\nWant:\n%+v, Error: nil\nDiff:%s\n", in, got, err, want, cmp.Diff(want, got, cmp.AllowUnexported(Filter{})))
+	}
+}

backend/src/apiserver/list/list.go

@@ -76,6 +76,7 @@ func (t *token) marshal() (string, error) {
 	if err != nil {
 		return "", util.NewInternalServerError(err, "Failed to serialize page token.")
 	}
+	// return string(b), nil
 	return base64.StdEncoding.EncodeToString(b), nil
 }
 

backend/src/apiserver/list/list_test.go

@@ -512,6 +512,17 @@ func TestAddPaginationAndFilterToSelect(t *testing.T) {
 }
 
 func TestTokenSerialization(t *testing.T) {
+	protoFilter := &api.Filter{Predicates: []*api.Predicate{
+		&api.Predicate{
+			Key:   "name",
+			Op:    api.Predicate_EQUALS,
+			Value: &api.Predicate_StringValue{StringValue: "SomeName"},
+		}}}
+	testFilter, err := filter.New(protoFilter)
+	if err != nil {
+		t.Fatalf("failed to parse filter proto %+v: %v", protoFilter, err)
+	}
+
 	tests := []struct {
 		in   *token
 		want *token
@@ -546,6 +557,25 @@ func TestTokenSerialization(t *testing.T) {
 				KeyFieldValue:    float64(200),
 				IsDesc:           true},
 		},
+		// has a filter.
+		{
+			in: &token{
+				SortByFieldName:  "SortField",
+				SortByFieldValue: 100,
+				KeyFieldName:     "KeyField",
+				KeyFieldValue:    200,
+				IsDesc:           true,
+				Filter:           testFilter,
+			},
+			want: &token{
+				SortByFieldName:  "SortField",
+				SortByFieldValue: float64(100),
+				KeyFieldName:     "KeyField",
+				KeyFieldValue:    float64(200),
+				IsDesc:           true,
+				Filter:           testFilter,
+			},
+		},
 	}
 
 	for _, test := range tests {
@@ -558,9 +588,9 @@ func TestTokenSerialization(t *testing.T) {
 
 		got := &token{}
 		got.unmarshal(s)
-		if !cmp.Equal(got, test.want) {
+		if !cmp.Equal(got, test.want, cmp.AllowUnexported(filter.Filter{})) {
 			t.Errorf("token.unmarshal(%q) =\nGot: %+v\nWant: %+v\nDiff:\n%s",
-				s, got, test.want, cmp.Diff(test.want, got))
+				s, got, test.want, cmp.Diff(test.want, got, cmp.AllowUnexported(filter.Filter{})))
 		}
 	}
 }

backend/src/apiserver/storage/experiment_store_test.go

@@ -369,5 +369,5 @@ func TestListExperiments_Filtering(t *testing.T) {
 	// No more pages.
 	assert.Equal(t, "", nextPageToken)
 	assert.Equal(t, expected, experiments)
-	assert.Equal(t, 4, total_size)
+	assert.Equal(t, 3, total_size)
 }