Skip to content

Commit 277d36d

Browse files
author
Kevin Wiesmüller
committed
make schema.Equals pointer receivers for Map and Schema
This fixes data races caused by copied mutexes change schema types to pointers Revert "change schema types to pointers" This reverts commit 92e30db. change equals methods to pointer receivers
1 parent c1d55aa commit 277d36d

File tree

5 files changed

+69
-41
lines changed

5 files changed

+69
-41
lines changed

schema/elements_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestFindNamedType(t *testing.T) {
4040
Types: tt.defs,
4141
}
4242
td, exist := s.FindNamedType(tt.namedType)
43-
if !td.Equals(tt.expectTypeDef) {
43+
if !td.Equals(&tt.expectTypeDef) {
4444
t.Errorf("expected TypeDef %v, got %v", tt.expectTypeDef, td)
4545
}
4646
if exist != tt.expectExist {

schema/equals.go

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,16 @@ limitations under the License.
1717
package schema
1818

1919
// Equals returns true iff the two Schemas are equal.
20-
func (a Schema) Equals(b Schema) bool {
20+
func (a *Schema) Equals(b *Schema) bool {
21+
if a == nil || b == nil {
22+
return a == nil && b == nil
23+
}
24+
2125
if len(a.Types) != len(b.Types) {
2226
return false
2327
}
2428
for i := range a.Types {
25-
if !a.Types[i].Equals(b.Types[i]) {
29+
if !a.Types[i].Equals(&b.Types[i]) {
2630
return false
2731
}
2832
}
@@ -33,7 +37,10 @@ func (a Schema) Equals(b Schema) bool {
3337
//
3438
// Note that two typerefs that have an equivalent type but where one is
3539
// inlined and the other is named, are not considered equal.
36-
func (a TypeRef) Equals(b TypeRef) bool {
40+
func (a *TypeRef) Equals(b *TypeRef) bool {
41+
if a == nil || b == nil {
42+
return a == nil && b == nil
43+
}
3744
if (a.NamedType == nil) != (b.NamedType == nil) {
3845
return false
3946
}
@@ -43,19 +50,25 @@ func (a TypeRef) Equals(b TypeRef) bool {
4350
}
4451
//return true
4552
}
46-
return a.Inlined.Equals(b.Inlined)
53+
return a.Inlined.Equals(&b.Inlined)
4754
}
4855

4956
// Equals returns true iff the two TypeDefs are equal.
50-
func (a TypeDef) Equals(b TypeDef) bool {
57+
func (a *TypeDef) Equals(b *TypeDef) bool {
58+
if a == nil || b == nil {
59+
return a == nil && b == nil
60+
}
5161
if a.Name != b.Name {
5262
return false
5363
}
54-
return a.Atom.Equals(b.Atom)
64+
return a.Atom.Equals(&b.Atom)
5565
}
5666

5767
// Equals returns true iff the two Atoms are equal.
58-
func (a Atom) Equals(b Atom) bool {
68+
func (a *Atom) Equals(b *Atom) bool {
69+
if a == nil || b == nil {
70+
return a == nil && b == nil
71+
}
5972
if (a.Scalar == nil) != (b.Scalar == nil) {
6073
return false
6174
}
@@ -69,16 +82,19 @@ func (a Atom) Equals(b Atom) bool {
6982
case a.Scalar != nil:
7083
return *a.Scalar == *b.Scalar
7184
case a.List != nil:
72-
return a.List.Equals(*b.List)
85+
return a.List.Equals(b.List)
7386
case a.Map != nil:
74-
return a.Map.Equals(*b.Map)
87+
return a.Map.Equals(b.Map)
7588
}
7689
return true
7790
}
7891

7992
// Equals returns true iff the two Maps are equal.
80-
func (a Map) Equals(b Map) bool {
81-
if !a.ElementType.Equals(b.ElementType) {
93+
func (a *Map) Equals(b *Map) bool {
94+
if a == nil || b == nil {
95+
return a == nil && b == nil
96+
}
97+
if !a.ElementType.Equals(&b.ElementType) {
8298
return false
8399
}
84100
if a.ElementRelationship != b.ElementRelationship {
@@ -88,23 +104,26 @@ func (a Map) Equals(b Map) bool {
88104
return false
89105
}
90106
for i := range a.Fields {
91-
if !a.Fields[i].Equals(b.Fields[i]) {
107+
if !a.Fields[i].Equals(&b.Fields[i]) {
92108
return false
93109
}
94110
}
95111
if len(a.Unions) != len(b.Unions) {
96112
return false
97113
}
98114
for i := range a.Unions {
99-
if !a.Unions[i].Equals(b.Unions[i]) {
115+
if !a.Unions[i].Equals(&b.Unions[i]) {
100116
return false
101117
}
102118
}
103119
return true
104120
}
105121

106122
// Equals returns true iff the two Unions are equal.
107-
func (a Union) Equals(b Union) bool {
123+
func (a *Union) Equals(b *Union) bool {
124+
if a == nil || b == nil {
125+
return a == nil && b == nil
126+
}
108127
if (a.Discriminator == nil) != (b.Discriminator == nil) {
109128
return false
110129
}
@@ -120,15 +139,18 @@ func (a Union) Equals(b Union) bool {
120139
return false
121140
}
122141
for i := range a.Fields {
123-
if !a.Fields[i].Equals(b.Fields[i]) {
142+
if !a.Fields[i].Equals(&b.Fields[i]) {
124143
return false
125144
}
126145
}
127146
return true
128147
}
129148

130149
// Equals returns true iff the two UnionFields are equal.
131-
func (a UnionField) Equals(b UnionField) bool {
150+
func (a *UnionField) Equals(b *UnionField) bool {
151+
if a == nil || b == nil {
152+
return a == nil && b == nil
153+
}
132154
if a.FieldName != b.FieldName {
133155
return false
134156
}
@@ -139,16 +161,22 @@ func (a UnionField) Equals(b UnionField) bool {
139161
}
140162

141163
// Equals returns true iff the two StructFields are equal.
142-
func (a StructField) Equals(b StructField) bool {
164+
func (a *StructField) Equals(b *StructField) bool {
165+
if a == nil || b == nil {
166+
return a == nil && b == nil
167+
}
143168
if a.Name != b.Name {
144169
return false
145170
}
146-
return a.Type.Equals(b.Type)
171+
return a.Type.Equals(&b.Type)
147172
}
148173

149174
// Equals returns true iff the two Lists are equal.
150-
func (a List) Equals(b List) bool {
151-
if !a.ElementType.Equals(b.ElementType) {
175+
func (a *List) Equals(b *List) bool {
176+
if a == nil || b == nil {
177+
return a == nil && b == nil
178+
}
179+
if !a.ElementType.Equals(&b.ElementType) {
152180
return false
153181
}
154182
if a.ElementRelationship != b.ElementRelationship {

schema/equals_test.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -61,89 +61,89 @@ func TestEquals(t *testing.T) {
6161
// add new fields without fixing the Equals function and this test.
6262
funcs := []interface{}{
6363
func(x Schema) bool {
64-
if !x.Equals(x) {
64+
if !x.Equals(&x) {
6565
return false
6666
}
6767
var y Schema
6868
y.Types = x.Types
69-
return x.Equals(y) == reflect.DeepEqual(x, y)
69+
return x.Equals(&y) == reflect.DeepEqual(&x, &y)
7070
},
7171
func(x TypeDef) bool {
72-
if !x.Equals(x) {
72+
if !x.Equals(&x) {
7373
return false
7474
}
7575
var y TypeDef
7676
y.Name = x.Name
7777
y.Atom = x.Atom
78-
return x.Equals(y) == reflect.DeepEqual(x, y)
78+
return x.Equals(&y) == reflect.DeepEqual(x, y)
7979
},
8080
func(x TypeRef) bool {
81-
if !x.Equals(x) {
81+
if !x.Equals(&x) {
8282
return false
8383
}
8484
var y TypeRef
8585
y.NamedType = x.NamedType
8686
y.Inlined = x.Inlined
87-
return x.Equals(y) == reflect.DeepEqual(x, y)
87+
return x.Equals(&y) == reflect.DeepEqual(x, y)
8888
},
8989
func(x Atom) bool {
90-
if !x.Equals(x) {
90+
if !x.Equals(&x) {
9191
return false
9292
}
9393
var y Atom
9494
y.Scalar = x.Scalar
9595
y.List = x.List
9696
y.Map = x.Map
97-
return x.Equals(y) == reflect.DeepEqual(x, y)
97+
return x.Equals(&y) == reflect.DeepEqual(x, y)
9898
},
9999
func(x Map) bool {
100-
if !x.Equals(x) {
100+
if !x.Equals(&x) {
101101
return false
102102
}
103103
var y Map
104104
y.ElementType = x.ElementType
105105
y.ElementRelationship = x.ElementRelationship
106106
y.Fields = x.Fields
107107
y.Unions = x.Unions
108-
return x.Equals(y) == reflect.DeepEqual(x, y)
108+
return x.Equals(&y) == reflect.DeepEqual(&x, &y)
109109
},
110110
func(x Union) bool {
111-
if !x.Equals(x) {
111+
if !x.Equals(&x) {
112112
return false
113113
}
114114
var y Union
115115
y.Discriminator = x.Discriminator
116116
y.DeduceInvalidDiscriminator = x.DeduceInvalidDiscriminator
117117
y.Fields = x.Fields
118-
return x.Equals(y) == reflect.DeepEqual(x, y)
118+
return x.Equals(&y) == reflect.DeepEqual(x, y)
119119
},
120120
func(x UnionField) bool {
121-
if !x.Equals(x) {
121+
if !x.Equals(&x) {
122122
return false
123123
}
124124
var y UnionField
125125
y.DiscriminatorValue = x.DiscriminatorValue
126126
y.FieldName = x.FieldName
127-
return x.Equals(y) == reflect.DeepEqual(x, y)
127+
return x.Equals(&y) == reflect.DeepEqual(x, y)
128128
},
129129
func(x StructField) bool {
130-
if !x.Equals(x) {
130+
if !x.Equals(&x) {
131131
return false
132132
}
133133
var y StructField
134134
y.Name = x.Name
135135
y.Type = x.Type
136-
return x.Equals(y) == reflect.DeepEqual(x, y)
136+
return x.Equals(&y) == reflect.DeepEqual(x, y)
137137
},
138138
func(x List) bool {
139-
if !x.Equals(x) {
139+
if !x.Equals(&x) {
140140
return false
141141
}
142142
var y List
143143
y.ElementType = x.ElementType
144144
y.ElementRelationship = x.ElementRelationship
145145
y.Keys = x.Keys
146-
return x.Equals(y) == reflect.DeepEqual(x, y)
146+
return x.Equals(&y) == reflect.DeepEqual(x, y)
147147
},
148148
}
149149
for i, f := range funcs {

typed/merge.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (w *mergingWalker) merge(prefixFn func() string) (errs ValidationErrors) {
8080

8181
alhs := deduceAtom(a, w.lhs)
8282
arhs := deduceAtom(a, w.rhs)
83-
if alhs.Equals(arhs) {
83+
if alhs.Equals(&arhs) {
8484
errs = append(errs, handleAtom(arhs, w.typeRef, w)...)
8585
} else {
8686
w2 := *w

typed/typed.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func merge(lhs, rhs *TypedValue, rule, postRule mergeRule) (*TypedValue, error)
214214
if lhs.schema != rhs.schema {
215215
return nil, errorf("expected objects with types from the same schema")
216216
}
217-
if !lhs.typeRef.Equals(rhs.typeRef) {
217+
if !lhs.typeRef.Equals(&rhs.typeRef) {
218218
return nil, errorf("expected objects of the same type, but got %v and %v", lhs.typeRef, rhs.typeRef)
219219
}
220220

0 commit comments

Comments
 (0)