From e4539ad04d5fb6f7bc3b0e2516ebc5812036ee49 Mon Sep 17 00:00:00 2001 From: Kelvin Wang Date: Wed, 3 Jul 2019 15:12:55 -0400 Subject: [PATCH] fix(inmem): fix dbrp not found --- dbrp_mapping.go | 26 +++++++++++++----- inmem/dbrp_mapping_service.go | 50 ++++++++++++++++++++--------------- testing/dbrp_mapping.go | 42 +++++++++++++++++++++++++++-- 3 files changed, 89 insertions(+), 29 deletions(-) diff --git a/dbrp_mapping.go b/dbrp_mapping.go index 23c5406e9c..d0a809f8bb 100644 --- a/dbrp_mapping.go +++ b/dbrp_mapping.go @@ -2,7 +2,6 @@ package influxdb import ( "context" - "errors" "strconv" "strings" "unicode" @@ -39,19 +38,34 @@ type DBRPMapping struct { // Validate reports any validation errors for the mapping. func (m DBRPMapping) Validate() error { if !validName(m.Cluster) { - return errors.New("cluster must contain at least one character and only be letters, numbers, '_', '-', and '.'") + return &Error{ + Code: EInvalid, + Msg: "cluster must contain at least one character and only be letters, numbers, '_', '-', and '.'", + } } if !validName(m.Database) { - return errors.New("database must contain at least one character and only be letters, numbers, '_', '-', and '.'") + return &Error{ + Code: EInvalid, + Msg: "database must contain at least one character and only be letters, numbers, '_', '-', and '.'", + } } if !validName(m.RetentionPolicy) { - return errors.New("retentionPolicy must contain at least one character and only be letters, numbers, '_', '-', and '.'") + return &Error{ + Code: EInvalid, + Msg: "retentionPolicy must contain at least one character and only be letters, numbers, '_', '-', and '.'", + } } if !m.OrganizationID.Valid() { - return errors.New("organizationID is required") + return &Error{ + Code: EInvalid, + Msg: "organizationID is required", + } } if !m.BucketID.Valid() { - return errors.New("bucketID is required") + return &Error{ + Code: EInvalid, + Msg: "bucketID is required", + } } return nil } diff --git a/inmem/dbrp_mapping_service.go b/inmem/dbrp_mapping_service.go index 1ee4778927..48097ff407 100644 --- a/inmem/dbrp_mapping_service.go +++ b/inmem/dbrp_mapping_service.go @@ -2,28 +2,30 @@ package inmem import ( "context" - "errors" "fmt" "path" - platform "github.com/influxdata/influxdb" + "github.com/influxdata/influxdb" ) var ( - errDBRPMappingNotFound = fmt.Errorf("dbrp mapping not found") + errDBRPMappingNotFound = &influxdb.Error{ + Code: influxdb.ENotFound, + Msg: "dbrp mapping not found", + } ) func encodeDBRPMappingKey(cluster, db, rp string) string { return path.Join(cluster, db, rp) } -func (c *Service) loadDBRPMapping(ctx context.Context, cluster, db, rp string) (*platform.DBRPMapping, error) { - i, ok := c.dbrpMappingKV.Load(encodeDBRPMappingKey(cluster, db, rp)) +func (s *Service) loadDBRPMapping(ctx context.Context, cluster, db, rp string) (*influxdb.DBRPMapping, error) { + i, ok := s.dbrpMappingKV.Load(encodeDBRPMappingKey(cluster, db, rp)) if !ok { return nil, errDBRPMappingNotFound } - m, ok := i.(platform.DBRPMapping) + m, ok := i.(influxdb.DBRPMapping) if !ok { return nil, fmt.Errorf("type %T is not a dbrp mapping", i) } @@ -32,14 +34,14 @@ func (c *Service) loadDBRPMapping(ctx context.Context, cluster, db, rp string) ( } // FindBy returns a single dbrp mapping by cluster, db and rp. -func (s *Service) FindBy(ctx context.Context, cluster, db, rp string) (*platform.DBRPMapping, error) { +func (s *Service) FindBy(ctx context.Context, cluster, db, rp string) (*influxdb.DBRPMapping, error) { return s.loadDBRPMapping(ctx, cluster, db, rp) } -func (c *Service) forEachDBRPMapping(ctx context.Context, fn func(m *platform.DBRPMapping) bool) error { +func (s *Service) forEachDBRPMapping(ctx context.Context, fn func(m *influxdb.DBRPMapping) bool) error { var err error - c.dbrpMappingKV.Range(func(k, v interface{}) bool { - m, ok := v.(platform.DBRPMapping) + s.dbrpMappingKV.Range(func(k, v interface{}) bool { + m, ok := v.(influxdb.DBRPMapping) if !ok { err = fmt.Errorf("type %T is not a dbrp mapping", v) return false @@ -50,9 +52,9 @@ func (c *Service) forEachDBRPMapping(ctx context.Context, fn func(m *platform.DB return err } -func (s *Service) filterDBRPMappings(ctx context.Context, fn func(m *platform.DBRPMapping) bool) ([]*platform.DBRPMapping, error) { - mappings := []*platform.DBRPMapping{} - err := s.forEachDBRPMapping(ctx, func(m *platform.DBRPMapping) bool { +func (s *Service) filterDBRPMappings(ctx context.Context, fn func(m *influxdb.DBRPMapping) bool) ([]*influxdb.DBRPMapping, error) { + mappings := []*influxdb.DBRPMapping{} + err := s.forEachDBRPMapping(ctx, func(m *influxdb.DBRPMapping) bool { if fn(m) { mappings = append(mappings, m) } @@ -67,9 +69,12 @@ func (s *Service) filterDBRPMappings(ctx context.Context, fn func(m *platform.DB } // Find returns the first dbrp mapping that matches filter. -func (s *Service) Find(ctx context.Context, filter platform.DBRPMappingFilter) (*platform.DBRPMapping, error) { +func (s *Service) Find(ctx context.Context, filter influxdb.DBRPMappingFilter) (*influxdb.DBRPMapping, error) { if filter.Cluster == nil && filter.Database == nil && filter.RetentionPolicy == nil { - return nil, fmt.Errorf("no filter parameters provided") + return nil, &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: "no filter parameters provided", + } } // filter by dbrpMapping id @@ -91,17 +96,17 @@ func (s *Service) Find(ctx context.Context, filter platform.DBRPMappingFilter) ( // FindMany returns a list of dbrpMappings that match filter and the total count of matching dbrp mappings. // Additional options provide pagination & sorting. -func (s *Service) FindMany(ctx context.Context, filter platform.DBRPMappingFilter, opt ...platform.FindOptions) ([]*platform.DBRPMapping, int, error) { +func (s *Service) FindMany(ctx context.Context, filter influxdb.DBRPMappingFilter, opt ...influxdb.FindOptions) ([]*influxdb.DBRPMapping, int, error) { // filter by dbrpMapping id if filter.Cluster != nil && filter.Database != nil && filter.RetentionPolicy != nil { m, err := s.FindBy(ctx, *filter.Cluster, *filter.Database, *filter.RetentionPolicy) if err != nil { return nil, 0, err } - return []*platform.DBRPMapping{m}, 1, nil + return []*influxdb.DBRPMapping{m}, 1, nil } - filterFunc := func(mapping *platform.DBRPMapping) bool { + filterFunc := func(mapping *influxdb.DBRPMapping) bool { return (filter.Cluster == nil || (*filter.Cluster) == mapping.Cluster) && (filter.Database == nil || (*filter.Database) == mapping.Database) && (filter.RetentionPolicy == nil || (*filter.RetentionPolicy) == mapping.RetentionPolicy) && @@ -117,7 +122,7 @@ func (s *Service) FindMany(ctx context.Context, filter platform.DBRPMappingFilte } // Create creates a new dbrp mapping. -func (s *Service) Create(ctx context.Context, m *platform.DBRPMapping) error { +func (s *Service) Create(ctx context.Context, m *influxdb.DBRPMapping) error { if err := m.Validate(); err != nil { return nil } @@ -130,14 +135,17 @@ func (s *Service) Create(ctx context.Context, m *platform.DBRPMapping) error { } if !existing.Equal(m) { - return errors.New("dbrp mapping already exists") + return &influxdb.Error{ + Code: influxdb.EConflict, + Msg: "dbrp mapping already exists", + } } return s.PutDBRPMapping(ctx, m) } // PutDBRPMapping sets dbrpMapping with the current ID. -func (s *Service) PutDBRPMapping(ctx context.Context, m *platform.DBRPMapping) error { +func (s *Service) PutDBRPMapping(ctx context.Context, m *influxdb.DBRPMapping) error { k := encodeDBRPMappingKey(m.Cluster, m.Database, m.RetentionPolicy) s.dbrpMappingKV.Store(k, *m) return nil diff --git a/testing/dbrp_mapping.go b/testing/dbrp_mapping.go index a2aa0616e0..450d86918a 100644 --- a/testing/dbrp_mapping.go +++ b/testing/dbrp_mapping.go @@ -217,7 +217,10 @@ func CreateDBRPMapping( }, }, wants: wants{ - err: errors.New("dbrp mapping already exists"), + err: &platform.Error{ + Code: platform.EConflict, + Msg: "dbrp mapping already exists", + }, dbrpMappings: []*platform.DBRPMapping{ { Cluster: "cluster1", @@ -512,7 +515,10 @@ func FindDBRPMappingByKey( RetentionPolicy: "retention_policyA", }, wants: wants{ - err: errors.New("dbrp mapping not found"), + err: &platform.Error{ + Code: platform.ENotFound, + Msg: "dbrp mapping not found", + }, }, }, } @@ -641,6 +647,38 @@ func FindDBRPMapping( }, }, }, + { + name: "find dbrpMapping with invalid filter", + fields: DBRPMappingFields{ + DBRPMappings: []*platform.DBRPMapping{ + { + Cluster: "cluster", + Database: "database", + RetentionPolicy: "retention_policyA", + Default: false, + OrganizationID: MustIDBase16(dbrpOrg3ID), + BucketID: MustIDBase16(dbrpBucketAID), + }, + { + Cluster: "cluster", + Database: "database", + RetentionPolicy: "retention_policyB", + Default: true, + OrganizationID: MustIDBase16(dbrpOrg3ID), + BucketID: MustIDBase16(dbrpBucketBID), + }, + }, + }, + args: args{ + filter: platform.DBRPMappingFilter{}, + }, + wants: wants{ + err: &platform.Error{ + Code: platform.EInvalid, + Msg: "no filter parameters provided", + }, + }, + }, } for _, tt := range tests {