Skip to content

Commit

Permalink
fix(dbrpv2): lookup by org name
Browse files Browse the repository at this point in the history
  • Loading branch information
gavincabbage committed Jun 2, 2020
1 parent efbc4ae commit 8bf7f30
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 14 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

1. [18279](https://github.com/influxdata/influxdb/pull/18279): Make all pkg applications stateful via stacks

### Bug Fixes

1. [18331](https://github.com/influxdata/influxdb/pull/18331): Support organization name in addition to ID in DBRP operations

## v2.0.0-beta.11 [2020-05-26]

### Features
Expand Down
12 changes: 10 additions & 2 deletions dbrp/http_client_dbrp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

func setup(t *testing.T) (*dbrp.Client, func()) {
t.Helper()
svc := &mock.DBRPMappingServiceV2{
dbrpSvc := &mock.DBRPMappingServiceV2{
CreateFn: func(ctx context.Context, dbrp *influxdb.DBRPMappingV2) error {
dbrp.ID = 1
return nil
Expand All @@ -34,7 +34,15 @@ func setup(t *testing.T) (*dbrp.Client, func()) {
return []*influxdb.DBRPMappingV2{}, 0, nil
},
}
server := httptest.NewServer(dbrp.NewHTTPHandler(zaptest.NewLogger(t), svc))
orgSvc := &mock.OrganizationService{
FindOrganizationF: func(ctx context.Context, filter influxdb.OrganizationFilter) (*influxdb.Organization, error) {
return &influxdb.Organization{
ID: *filter.ID,
Name: "org",
}, nil
},
}
server := httptest.NewServer(dbrp.NewHTTPHandler(zaptest.NewLogger(t), dbrpSvc, orgSvc))
client, err := httpc.New(httpc.WithAddr(server.URL), httpc.WithStatusFn(http.CheckError))
if err != nil {
t.Fatal(err)
Expand Down
48 changes: 39 additions & 9 deletions dbrp/http_server_dbrp.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ type Handler struct {
api *kithttp.API
log *zap.Logger
dbrpSvc influxdb.DBRPMappingServiceV2
orgSvc influxdb.OrganizationService
}

// NewHTTPHandler constructs a new http server.
func NewHTTPHandler(log *zap.Logger, dbrpSvc influxdb.DBRPMappingServiceV2) *Handler {
func NewHTTPHandler(log *zap.Logger, dbrpSvc influxdb.DBRPMappingServiceV2, orgSvc influxdb.OrganizationService) *Handler {
h := &Handler{
api: kithttp.NewAPI(kithttp.WithLog(log)),
log: log,
dbrpSvc: dbrpSvc,
orgSvc: orgSvc,
}

r := chi.NewRouter()
Expand Down Expand Up @@ -57,6 +59,7 @@ type createDBRPRequest struct {
Database string `json:"database"`
RetentionPolicy string `json:"retention_policy"`
Default bool `json:"default"`
Org string `json:"organization"`
OrganizationID influxdb.ID `json:"organization_id"`
BucketID influxdb.ID `json:"bucket_id"`
}
Expand All @@ -73,6 +76,21 @@ func (h *Handler) handlePostDBRP(w http.ResponseWriter, r *http.Request) {
return
}

if !req.OrganizationID.Valid() {
if req.Org == "" {
h.api.Err(w, r, influxdb.ErrInvalidID)
return
}
org, err := h.orgSvc.FindOrganization(r.Context(), influxdb.OrganizationFilter{
Name: &req.Org,
})
if err != nil {
h.api.Err(w, r, influxdb.ErrOrgNotFound)
return
}
req.OrganizationID = org.ID
}

dbrp := &influxdb.DBRPMappingV2{
Database: req.Database,
RetentionPolicy: req.RetentionPolicy,
Expand All @@ -92,7 +110,7 @@ type getDBRPsResponse struct {
}

func (h *Handler) handleGetDBRPs(w http.ResponseWriter, r *http.Request) {
filter, err := getFilterFromHTTPRequest(r)
filter, err := h.getFilterFromHTTPRequest(r)
if err != nil {
h.api.Err(w, r, err)
return
Expand Down Expand Up @@ -129,7 +147,7 @@ func (h *Handler) handleGetDBRP(w http.ResponseWriter, r *http.Request) {
return
}

orgID, err := mustGetOrgIDFromHTTPRequest(r)
orgID, err := h.mustGetOrgIDFromHTTPRequest(r)
if err != nil {
h.api.Err(w, r, err)
return
Expand Down Expand Up @@ -168,7 +186,7 @@ func (h *Handler) handlePatchDBRP(w http.ResponseWriter, r *http.Request) {
return
}

orgID, err := mustGetOrgIDFromHTTPRequest(r)
orgID, err := h.mustGetOrgIDFromHTTPRequest(r)
if err != nil {
h.api.Err(w, r, err)
return
Expand Down Expand Up @@ -226,7 +244,7 @@ func (h *Handler) handleDeleteDBRP(w http.ResponseWriter, r *http.Request) {
return
}

orgID, err := mustGetOrgIDFromHTTPRequest(r)
orgID, err := h.mustGetOrgIDFromHTTPRequest(r)
if err != nil {
h.api.Err(w, r, err)
return
Expand All @@ -240,9 +258,9 @@ func (h *Handler) handleDeleteDBRP(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusNoContent)
}

func getFilterFromHTTPRequest(r *http.Request) (f influxdb.DBRPMappingFilterV2, err error) {
func (h *Handler) getFilterFromHTTPRequest(r *http.Request) (f influxdb.DBRPMappingFilterV2, err error) {
// Always provide OrgID.
f.OrgID, err = mustGetOrgIDFromHTTPRequest(r)
f.OrgID, err = h.mustGetOrgIDFromHTTPRequest(r)
if err != nil {
return f, err
}
Expand Down Expand Up @@ -289,13 +307,25 @@ func getIDFromHTTPRequest(r *http.Request, key string) (*influxdb.ID, error) {
return &id, nil
}

func mustGetOrgIDFromHTTPRequest(r *http.Request) (*influxdb.ID, error) {
// mustGetOrgIDFromHTTPRequest returns the org ID parameter from the request, falling
// back to looking up the org ID by org name if the ID parameter is not present.
func (h *Handler) mustGetOrgIDFromHTTPRequest(r *http.Request) (*influxdb.ID, error) {
orgID, err := getIDFromHTTPRequest(r, "orgID")
if err != nil {
return nil, err
}
if orgID == nil {
return nil, influxdb.ErrOrgNotFound
name := r.URL.Query().Get("org")
if name == "" {
return nil, influxdb.ErrOrgNotFound
}
org, err := h.orgSvc.FindOrganization(r.Context(), influxdb.OrganizationFilter{
Name: &name,
})
if err != nil {
return nil, influxdb.ErrOrgNotFound
}
orgID = &org.ID
}
return orgID, nil
}
Expand Down
78 changes: 76 additions & 2 deletions dbrp/http_server_dbrp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,25 @@ func initHttpService(t *testing.T) (influxdb.DBRPMappingServiceV2, *httptest.Ser
t.Helper()
ctx := context.Background()
bucketSvc := mock.NewBucketService()
orgSvc := &mock.OrganizationService{
FindOrganizationF: func(ctx context.Context, filter influxdb.OrganizationFilter) (*influxdb.Organization, error) {
if filter.Name == nil || *filter.Name != "org" {
return nil, errors.New("not found")
}
return &influxdb.Organization{
Name: "org",
ID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
}, nil
},
}

s := inmem.NewKVStore()
svc, err := dbrp.NewService(ctx, bucketSvc, s)
if err != nil {
t.Fatal(err)
}

server := httptest.NewServer(dbrp.NewHTTPHandler(zaptest.NewLogger(t), svc))
server := httptest.NewServer(dbrp.NewHTTPHandler(zaptest.NewLogger(t), svc, orgSvc))
return svc, server, func() {
server.Close()
}
Expand All @@ -52,6 +63,19 @@ func Test_handlePostDBRP(t *testing.T) {
"database": "mydb",
"retention_policy": "autogen",
"default": false
}`),
ExpectedDBRP: &influxdb.DBRPMappingV2{
OrganizationID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
},
},
{
Name: "Create valid dbrp by org name",
Input: strings.NewReader(`{
"bucket_id": "5555f7ed2a035555",
"organization": "org",
"database": "mydb",
"retention_policy": "autogen",
"default": false
}`),
ExpectedDBRP: &influxdb.DBRPMappingV2{
OrganizationID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
Expand All @@ -72,6 +96,17 @@ func Test_handlePostDBRP(t *testing.T) {
Err: influxdb.ErrInvalidID.Err,
},
},
{
Name: "Create with invalid org name",
Input: strings.NewReader(`{
"bucket_id": "5555f7ed2a035555",
"organization": "invalid",
"database": "mydb",
"retention_policy": "autogen",
"default": false
}`),
ExpectedErr: influxdb.ErrOrgNotFound,
},
}

for _, tt := range table {
Expand Down Expand Up @@ -106,7 +141,7 @@ func Test_handlePostDBRP(t *testing.T) {
}

if !dbrp.ID.Valid() {
t.Fatalf("expected invalid id, got an invalid one %s", dbrp.ID.String())
t.Fatalf("expected valid id, got an invalid one %s", dbrp.ID.String())
}

if dbrp.OrganizationID != tt.ExpectedDBRP.OrganizationID {
Expand Down Expand Up @@ -186,6 +221,20 @@ func Test_handleGetDBRPs(t *testing.T) {
},
},
},
{
Name: "org name",
QueryParams: "org=org",
ExpectedDBRPs: []influxdb.DBRPMappingV2{
{
ID: influxdbtesting.MustIDBase16("1111111111111111"),
BucketID: influxdbtesting.MustIDBase16("5555f7ed2a035555"),
OrganizationID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
Database: "mydb",
RetentionPolicy: "autogen",
Default: true,
},
},
},
}

ctx := context.Background()
Expand Down Expand Up @@ -262,6 +311,22 @@ func Test_handlePatchDBRP(t *testing.T) {
Input: strings.NewReader(`{
"retention_policy": "updaterp",
"database": "wont_change"
}`),
ExpectedDBRP: &influxdb.DBRPMappingV2{
ID: influxdbtesting.MustIDBase16("1111111111111111"),
BucketID: influxdbtesting.MustIDBase16("5555f7ed2a035555"),
OrganizationID: influxdbtesting.MustIDBase16("059af7ed2a034000"),
Database: "mydb",
RetentionPolicy: "updaterp",
Default: true,
},
},
{
Name: "happy path update by org name",
URLSuffix: "/1111111111111111?org=org",
Input: strings.NewReader(`{
"retention_policy": "updaterp",
"database": "wont_change"
}`),
ExpectedDBRP: &influxdb.DBRPMappingV2{
ID: influxdbtesting.MustIDBase16("1111111111111111"),
Expand Down Expand Up @@ -368,6 +433,10 @@ func Test_handleDeleteDBRP(t *testing.T) {
Name: "delete",
URLSuffix: "/1111111111111111?orgID=059af7ed2a034000",
},
{
Name: "delete by org name",
URLSuffix: "/1111111111111111?org=org",
},
{
Name: "invalid org",
URLSuffix: "/1111111111111111?orgID=invalid",
Expand All @@ -376,6 +445,11 @@ func Test_handleDeleteDBRP(t *testing.T) {
Msg: "invalid ID",
},
},
{
Name: "invalid org name",
URLSuffix: "/1111111111111111?org=invalid",
ExpectedErr: influxdb.ErrOrgNotFound,
},
{
Name: "no org",
URLSuffix: "/1111111111111111",
Expand Down
2 changes: 1 addition & 1 deletion http/api_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func NewAPIHandler(b *APIBackend, opts ...APIHandlerOptFn) *APIHandler {
backupBackend.BackupService = authorizer.NewBackupService(backupBackend.BackupService)
h.Mount(prefixBackup, NewBackupHandler(backupBackend))

h.Mount(dbrp.PrefixDBRP, dbrp.NewHTTPHandler(b.Logger, b.DBRPService))
h.Mount(dbrp.PrefixDBRP, dbrp.NewHTTPHandler(b.Logger, b.DBRPService, b.OrganizationService))

writeBackend := NewWriteBackend(b.Logger.With(zap.String("handler", "write")), b)
h.Mount(prefixWrite, NewWriteHandler(b.Logger, writeBackend,
Expand Down
4 changes: 4 additions & 0 deletions http/swagger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11683,6 +11683,7 @@ components:
DBRP:
required:
- orgID
- org
- bucketID
- database
- retention_policy
Expand All @@ -11694,6 +11695,9 @@ components:
orgID:
type: string
description: the organization ID that owns this mapping.
org:
type: string
description: the organization that owns this mapping.
bucketID:
type: string
description: the bucket ID used as target for the translation.
Expand Down

0 comments on commit 8bf7f30

Please sign in to comment.