From cef9872b3932dd1098b656528f1dfc641c73f30a Mon Sep 17 00:00:00 2001 From: Alex Vanin Date: Fri, 11 Mar 2022 12:46:21 +0300 Subject: [PATCH] [#168] policy: Adopt replacement of pointer slices with struct slices Signed-off-by: Alex Vanin --- policy/encode.go | 10 +++++----- policy/json.go | 31 ++++++++++++++++--------------- policy/query.go | 16 ++++++++-------- policy/query_test.go | 25 +++++++++++-------------- 4 files changed, 40 insertions(+), 42 deletions(-) diff --git a/policy/encode.go b/policy/encode.go index 080ba7a5..39c9dcc3 100644 --- a/policy/encode.go +++ b/policy/encode.go @@ -40,7 +40,7 @@ func Encode(p *netmap.PlacementPolicy) []string { return result } -func encodeReplicas(replicas []*netmap.Replica, dst *[]string) { +func encodeReplicas(replicas []netmap.Replica, dst *[]string) { builder := new(strings.Builder) for _, replica := range replicas { @@ -57,7 +57,7 @@ func encodeReplicas(replicas []*netmap.Replica, dst *[]string) { } } -func encodeSelectors(selectors []*netmap.Selector, dst *[]string) { +func encodeSelectors(selectors []netmap.Selector, dst *[]string) { builder := new(strings.Builder) for _, selector := range selectors { @@ -94,13 +94,13 @@ func encodeSelectors(selectors []*netmap.Selector, dst *[]string) { } } -func encodeFilters(filters []*netmap.Filter, dst *[]string) { +func encodeFilters(filters []netmap.Filter, dst *[]string) { builder := new(strings.Builder) for _, filter := range filters { builder.WriteString("FILTER ") - builder.WriteString(encodeFilter(filter)) + builder.WriteString(encodeFilter(&filter)) *dst = append(*dst, builder.String()) builder.Reset() @@ -129,7 +129,7 @@ func encodeFilter(filter *netmap.Filter) string { builder.WriteString(" ") } - builder.WriteString(encodeFilter(subfilter)) + builder.WriteString(encodeFilter(&subfilter)) } if n := filter.Name(); n != "" && !unspecified { diff --git a/policy/json.go b/policy/json.go index 53cb9c50..595fceb4 100644 --- a/policy/json.go +++ b/policy/json.go @@ -41,15 +41,15 @@ func ToJSON(np *netmap.PlacementPolicy) ([]byte, error) { p.CBF = np.ContainerBackupFactor() p.Filters = make([]filter, len(np.Filters())) for i, f := range np.Filters() { - p.Filters[i].fromNetmap(f) + p.Filters[i].fromNetmap(&f) } p.Selectors = make([]selector, len(np.Selectors())) for i, s := range np.Selectors() { - p.Selectors[i].fromNetmap(s) + p.Selectors[i].fromNetmap(&s) } p.Replicas = make([]replica, len(np.Replicas())) for i, r := range np.Replicas() { - p.Replicas[i].fromNetmap(r) + p.Replicas[i].fromNetmap(&r) } return json.Marshal(p) } @@ -61,32 +61,32 @@ func FromJSON(data []byte) (*netmap.PlacementPolicy, error) { return nil, err } - rs := make([]*netmap.Replica, len(p.Replicas)) + rs := make([]netmap.Replica, len(p.Replicas)) for i := range p.Replicas { - rs[i] = p.Replicas[i].toNetmap() + rs[i] = *p.Replicas[i].toNetmap() } - var fs []*netmap.Filter + var fs []netmap.Filter if len(p.Filters) != 0 { - fs = make([]*netmap.Filter, len(p.Filters)) + fs = make([]netmap.Filter, len(p.Filters)) for i := range p.Filters { f, err := p.Filters[i].toNetmap() if err != nil { return nil, err } - fs[i] = f + fs[i] = *f } } - var ss []*netmap.Selector + var ss []netmap.Selector if len(p.Selectors) != 0 { - ss = make([]*netmap.Selector, len(p.Selectors)) + ss = make([]netmap.Selector, len(p.Selectors)) for i := range p.Selectors { s, err := p.Selectors[i].toNetmap() if err != nil { return nil, err } - ss[i] = s + ss[i] = *s } } @@ -134,15 +134,16 @@ func (f *filter) toNetmap() (*netmap.Filter, error) { return nil, fmt.Errorf("%w: '%s'", ErrUnknownOp, f.Op) } - var fs []*netmap.Filter + var fs []netmap.Filter if len(f.Filters) != 0 { - fs = make([]*netmap.Filter, len(f.Filters)) + fs = make([]netmap.Filter, len(f.Filters)) for i := range f.Filters { var err error - fs[i], err = f.Filters[i].toNetmap() + fsp, err := f.Filters[i].toNetmap() if err != nil { return nil, err } + fs[i] = *fsp } } @@ -182,7 +183,7 @@ func (f *filter) fromNetmap(nf *netmap.Filter) { if nf.InnerFilters() != nil { f.Filters = make([]filter, len(nf.InnerFilters())) for i, sf := range nf.InnerFilters() { - f.Filters[i].fromNetmap(sf) + f.Filters[i].fromNetmap(&sf) } } } diff --git a/policy/query.go b/policy/query.go index d291758a..b1abd624 100644 --- a/policy/query.go +++ b/policy/query.go @@ -81,14 +81,14 @@ func (p *policyVisitor) VisitPolicy(ctx *parser.PolicyContext) interface{} { pl := new(netmap.PlacementPolicy) repStmts := ctx.AllRepStmt() - rs := make([]*netmap.Replica, 0, len(repStmts)) + rs := make([]netmap.Replica, 0, len(repStmts)) for _, r := range repStmts { res, ok := r.Accept(p).(*netmap.Replica) if !ok { return nil } - rs = append(rs, res) + rs = append(rs, *res) } pl.SetReplicas(rs...) @@ -101,21 +101,21 @@ func (p *policyVisitor) VisitPolicy(ctx *parser.PolicyContext) interface{} { } selStmts := ctx.AllSelectStmt() - ss := make([]*netmap.Selector, 0, len(selStmts)) + ss := make([]netmap.Selector, 0, len(selStmts)) for _, s := range selStmts { res, ok := s.Accept(p).(*netmap.Selector) if !ok { return nil } - ss = append(ss, res) + ss = append(ss, *res) } pl.SetSelectors(ss...) filtStmts := ctx.AllFilterStmt() - fs := make([]*netmap.Filter, 0, len(filtStmts)) + fs := make([]netmap.Filter, 0, len(filtStmts)) for _, f := range filtStmts { - fs = append(fs, f.Accept(p).(*netmap.Filter)) + fs = append(fs, *f.Accept(p).(*netmap.Filter)) } pl.SetFilters(fs...) @@ -194,8 +194,8 @@ func (p *policyVisitor) VisitFilterExpr(ctx *parser.FilterExprContext) interface op := operationFromString(ctx.GetOp().GetText()) f.SetOperation(op) - f1 := ctx.GetF1().Accept(p).(*netmap.Filter) - f2 := ctx.GetF2().Accept(p).(*netmap.Filter) + f1 := *ctx.GetF1().Accept(p).(*netmap.Filter) + f2 := *ctx.GetF2().Accept(p).(*netmap.Filter) // Consider f1=(.. AND ..) AND f2. This can be merged because our AND operation // is of arbitrary arity. ANTLR generates left-associative parse-tree by default. diff --git a/policy/query_test.go b/policy/query_test.go index 92fb8a6e..888e481b 100644 --- a/policy/query_test.go +++ b/policy/query_test.go @@ -13,8 +13,8 @@ import ( func TestSimple(t *testing.T) { q := `REP 3` expected := new(netmap.PlacementPolicy) - expected.SetFilters([]*netmap.Filter{}...) - expected.SetSelectors([]*netmap.Selector{}...) + expected.SetFilters([]netmap.Filter{}...) + expected.SetSelectors([]netmap.Selector{}...) expected.SetReplicas(newReplica("", 3)) r, err := Parse(q) @@ -25,8 +25,8 @@ func TestSimple(t *testing.T) { func TestSimpleWithHRWB(t *testing.T) { q := `REP 3 CBF 4` expected := new(netmap.PlacementPolicy) - expected.SetFilters([]*netmap.Filter{}...) - expected.SetSelectors([]*netmap.Selector{}...) + expected.SetFilters([]netmap.Filter{}...) + expected.SetSelectors([]netmap.Selector{}...) expected.SetReplicas(newReplica("", 3)) expected.SetContainerBackupFactor(4) @@ -39,7 +39,7 @@ func TestFromSelect(t *testing.T) { q := `REP 1 IN SPB SELECT 1 IN City FROM * AS SPB` expected := new(netmap.PlacementPolicy) - expected.SetFilters([]*netmap.Filter{}...) + expected.SetFilters([]netmap.Filter{}...) expected.SetSelectors(newSelector(1, netmap.ClauseUnspecified, "City", "*", "SPB")) expected.SetReplicas(newReplica("SPB", 1)) @@ -55,7 +55,7 @@ func TestFromSelectNoAttribute(t *testing.T) { SELECT 6 FROM *` expected := new(netmap.PlacementPolicy) - expected.SetFilters([]*netmap.Filter{}...) + expected.SetFilters([]netmap.Filter{}...) expected.SetSelectors(newSelector(6, netmap.ClauseUnspecified, "", "*", "")) expected.SetReplicas(newReplica("", 2)) @@ -101,7 +101,7 @@ FILTER Property EQ %s AND Something NE 7 AS Filt` expected := newFilter("Filt", "", "", netmap.OpAND, newFilter("", "Property", s[1:len(s)-1], netmap.OpEQ), newFilter("", "Something", "7", netmap.OpNE)) - require.EqualValues(t, []*netmap.Filter{expected}, r.Filters()) + require.EqualValues(t, []netmap.Filter{expected}, r.Filters()) }) } } @@ -112,7 +112,7 @@ SELECT 3 IN Country FROM * SELECT 2 IN SAME City FROM * SELECT 1 IN DISTINCT Continent FROM *` expected := new(netmap.PlacementPolicy) - expected.SetFilters([]*netmap.Filter{}...) + expected.SetFilters([]netmap.Filter{}...) expected.SetSelectors( newSelector(3, netmap.ClauseUnspecified, "Country", "*", ""), newSelector(2, netmap.ClauseSame, "City", "*", ""), @@ -299,8 +299,7 @@ FILTER "UN-LOCODE" EQ "RU LED" AS F` require.EqualValues(t, expected, r) } -func newFilter(name, key, value string, op netmap.Operation, sub ...*netmap.Filter) *netmap.Filter { - f := new(netmap.Filter) +func newFilter(name, key, value string, op netmap.Operation, sub ...netmap.Filter) (f netmap.Filter) { f.SetName(name) f.SetKey(key) f.SetValue(value) @@ -309,15 +308,13 @@ func newFilter(name, key, value string, op netmap.Operation, sub ...*netmap.Filt return f } -func newReplica(s string, c uint32) *netmap.Replica { - r := new(netmap.Replica) +func newReplica(s string, c uint32) (r netmap.Replica) { r.SetSelector(s) r.SetCount(c) return r } -func newSelector(count uint32, c netmap.Clause, attr, f, name string) *netmap.Selector { - s := new(netmap.Selector) +func newSelector(count uint32, c netmap.Clause, attr, f, name string) (s netmap.Selector) { s.SetCount(count) s.SetClause(c) s.SetAttribute(attr)