Skip to content

Commit 6fefd0f

Browse files
moshaad7moshaad7
committed
MB-60207 fix facets merge (#1946)
1 parent d5b78da commit 6fefd0f

File tree

2 files changed

+194
-100
lines changed

2 files changed

+194
-100
lines changed

‎search/facets_builder.go

+15-3
Original file line numberDiff line numberDiff line change
@@ -321,17 +321,29 @@ func (fr *FacetResult) Merge(other *FacetResult) {
321321
fr.Total += other.Total
322322
fr.Missing += other.Missing
323323
fr.Other += other.Other
324-
if fr.Terms != nil && other.Terms != nil {
324+
if other.Terms != nil {
325+
if fr.Terms == nil {
326+
fr.Terms = other.Terms
327+
return
328+
}
325329
for _, term := range other.Terms.termFacets {
326330
fr.Terms.Add(term)
327331
}
328332
}
329-
if fr.NumericRanges != nil && other.NumericRanges != nil {
333+
if other.NumericRanges != nil {
334+
if fr.NumericRanges == nil {
335+
fr.NumericRanges = other.NumericRanges
336+
return
337+
}
330338
for _, nr := range other.NumericRanges {
331339
fr.NumericRanges = fr.NumericRanges.Add(nr)
332340
}
333341
}
334-
if fr.DateRanges != nil && other.DateRanges != nil {
342+
if other.DateRanges != nil {
343+
if fr.DateRanges == nil {
344+
fr.DateRanges = other.DateRanges
345+
return
346+
}
335347
for _, dr := range other.DateRanges {
336348
fr.DateRanges = fr.DateRanges.Add(dr)
337349
}

‎search/facets_builder_test.go

+179-97
Original file line numberDiff line numberDiff line change
@@ -15,122 +15,204 @@
1515
package search
1616

1717
import (
18+
"fmt"
1819
"reflect"
1920
"testing"
2021
)
2122

2223
func TestTermFacetResultsMerge(t *testing.T) {
24+
type testCase struct {
25+
// Input
26+
frs1 FacetResults // first facet results
27+
frs2 FacetResults // second facet results (to be merged into first)
28+
fixups map[string]int // {facetName:size} (to be applied after merge)
2329

24-
fr1TypeTerms := &TermFacets{}
25-
fr1TypeTerms.Add(
26-
&TermFacet{
27-
Term: "blog",
28-
Count: 25,
29-
},
30-
&TermFacet{
31-
Term: "comment",
32-
Count: 24,
33-
},
34-
&TermFacet{
35-
Term: "feedback",
36-
Count: 1,
37-
},
38-
)
39-
fr1 := &FacetResult{
40-
Field: "type",
41-
Total: 100,
42-
Missing: 25,
43-
Other: 25,
44-
Terms: fr1TypeTerms,
30+
// Expected output
31+
expFrs FacetResults // facet results after merge and fixup
4532
}
4633

47-
fr1CategoryTerms := &TermFacets{}
48-
fr1CategoryTerms.Add(
49-
&TermFacet{
50-
Term: "clothing",
51-
Count: 35,
52-
},
53-
&TermFacet{
54-
Term: "electronics",
55-
Count: 25,
56-
},
57-
)
34+
tests := []*testCase{
35+
func() *testCase {
36+
rv := &testCase{}
5837

59-
fr1Only := &FacetResult{
60-
Field: "category",
61-
Total: 97,
62-
Missing: 22,
63-
Other: 15,
64-
Terms: fr1CategoryTerms,
65-
}
66-
frs1 := FacetResults{
67-
"types": fr1,
68-
"categories": fr1Only,
69-
}
38+
rv.frs1 = FacetResults{
39+
"types": &FacetResult{
40+
Field: "type",
41+
Total: 100,
42+
Missing: 25,
43+
Other: 25,
44+
Terms: func() *TermFacets {
45+
tfs := &TermFacets{}
46+
tfs.Add(
47+
&TermFacet{
48+
Term: "blog",
49+
Count: 25,
50+
},
51+
&TermFacet{
52+
Term: "comment",
53+
Count: 24,
54+
},
55+
&TermFacet{
56+
Term: "feedback",
57+
Count: 1,
58+
},
59+
)
60+
return tfs
61+
}(),
62+
},
63+
"categories": &FacetResult{
64+
Field: "category",
65+
Total: 97,
66+
Missing: 22,
67+
Other: 15,
68+
Terms: func() *TermFacets {
69+
tfs := &TermFacets{}
70+
tfs.Add(
71+
&TermFacet{
72+
Term: "clothing",
73+
Count: 35,
74+
},
75+
&TermFacet{
76+
Term: "electronics",
77+
Count: 25,
78+
},
79+
)
80+
return tfs
81+
}(),
82+
},
83+
}
84+
rv.frs2 = FacetResults{
85+
"types": &FacetResult{
86+
Field: "type",
87+
Total: 100,
88+
Missing: 25,
89+
Other: 25,
90+
Terms: func() *TermFacets {
91+
tfs := &TermFacets{}
92+
tfs.Add(
93+
&TermFacet{
94+
Term: "blog",
95+
Count: 25,
96+
},
97+
&TermFacet{
98+
Term: "comment",
99+
Count: 22,
100+
},
101+
&TermFacet{
102+
Term: "flag",
103+
Count: 3,
104+
},
105+
)
106+
return tfs
107+
}(),
108+
},
109+
}
110+
rv.fixups = map[string]int{
111+
"types": 3, // we want top 3 terms based on count
112+
}
70113

71-
fr2TypeTerms := &TermFacets{}
72-
fr2TypeTerms.Add(
73-
&TermFacet{
74-
Term: "blog",
75-
Count: 25,
76-
},
77-
&TermFacet{
78-
Term: "comment",
79-
Count: 22,
80-
},
81-
&TermFacet{
82-
Term: "flag",
83-
Count: 3,
84-
},
85-
)
114+
rv.expFrs = FacetResults{
115+
"types": &FacetResult{
116+
Field: "type",
117+
Total: 200,
118+
Missing: 50,
119+
Other: 51,
120+
Terms: &TermFacets{
121+
termFacets: []*TermFacet{
122+
{
123+
Term: "blog",
124+
Count: 50,
125+
},
126+
{
127+
Term: "comment",
128+
Count: 46,
129+
},
130+
{
131+
Term: "flag",
132+
Count: 3,
133+
},
134+
},
135+
},
136+
},
137+
"categories": rv.frs1["categories"],
138+
}
86139

87-
fr2 := &FacetResult{
88-
Field: "type",
89-
Total: 100,
90-
Missing: 25,
91-
Other: 25,
92-
Terms: fr2TypeTerms,
93-
}
94-
frs2 := FacetResults{
95-
"types": fr2,
96-
}
140+
return rv
141+
}(),
142+
func() *testCase {
143+
rv := &testCase{}
97144

98-
expectedFr := &FacetResult{
99-
Field: "type",
100-
Total: 200,
101-
Missing: 50,
102-
Other: 51,
103-
Terms: &TermFacets{
104-
termFacets: []*TermFacet{
105-
{
106-
Term: "blog",
107-
Count: 50,
145+
rv.frs1 = FacetResults{
146+
"facetName": &FacetResult{
147+
Field: "docField",
148+
Total: 0,
149+
Missing: 0,
150+
Other: 0,
151+
Terms: nil,
108152
},
109-
{
110-
Term: "comment",
111-
Count: 46,
153+
}
154+
rv.frs2 = FacetResults{
155+
"facetName": &FacetResult{
156+
Field: "docField",
157+
Total: 3,
158+
Missing: 0,
159+
Other: 0,
160+
Terms: &TermFacets{
161+
termFacets: []*TermFacet{
162+
{
163+
Term: "firstTerm",
164+
Count: 1,
165+
},
166+
{
167+
Term: "secondTerm",
168+
Count: 2,
169+
},
170+
},
171+
},
112172
},
113-
{
114-
Term: "flag",
115-
Count: 3,
173+
}
174+
rv.fixups = map[string]int{
175+
"facetName": 1,
176+
}
177+
178+
rv.expFrs = FacetResults{
179+
"facetName": &FacetResult{
180+
Field: "docField",
181+
Total: 3,
182+
Missing: 0,
183+
Other: 1,
184+
Terms: &TermFacets{
185+
termFacets: []*TermFacet{
186+
{
187+
Term: "secondTerm",
188+
Count: 2,
189+
},
190+
},
191+
},
116192
},
117-
},
118-
},
119-
}
120-
expectedFrs := FacetResults{
121-
"types": expectedFr,
122-
"categories": fr1Only,
193+
}
194+
return rv
195+
}(),
123196
}
124197

125-
frs1.Merge(frs2)
126-
frs1.Fixup("types", 3)
198+
for tcIdx, tc := range tests {
199+
t.Run(fmt.Sprintf("T#%d", tcIdx), func(t *testing.T) {
200+
tc.frs1.Merge(tc.frs2)
201+
for facetName, size := range tc.fixups {
202+
tc.frs1.Fixup(facetName, size)
203+
}
127204

128-
for _, v := range frs1 {
129-
v.Terms.termLookup = nil
130-
}
205+
// clear termLookup, so we can compare the facet results
206+
for _, fr := range tc.frs1 {
207+
if fr.Terms != nil {
208+
fr.Terms.termLookup = nil
209+
}
210+
}
131211

132-
if !reflect.DeepEqual(frs1, expectedFrs) {
133-
t.Errorf("expected %v, got %v", expectedFrs, frs1)
212+
if !reflect.DeepEqual(tc.frs1, tc.expFrs) {
213+
t.Errorf("expected %v, got %v", tc.expFrs, tc.frs1)
214+
}
215+
})
134216
}
135217
}
136218

0 commit comments

Comments
 (0)