From 92e82884a60a2023a1785b2189dddb2574508545 Mon Sep 17 00:00:00 2001 From: Kelvin Wang Date: Thu, 11 Apr 2019 11:17:05 -0400 Subject: [PATCH] feat(http): accept label id for template post --- bolt/label.go | 2 +- document.go | 10 +- http/document_service.go | 15 +-- http/document_test.go | 209 +++++++++++++++++++++++++++++++++++++++ http/label_test.go | 6 +- http/swagger.yml | 2 +- inmem/label_service.go | 6 +- kv/document.go | 33 ++----- kv/label.go | 2 +- label.go | 2 +- testing/document.go | 8 +- testing/label_service.go | 8 +- testing/util.go | 4 +- 13 files changed, 249 insertions(+), 58 deletions(-) diff --git a/bolt/label.go b/bolt/label.go index a34ea39a8c..39c92a1746 100644 --- a/bolt/label.go +++ b/bolt/label.go @@ -63,7 +63,7 @@ func (c *Client) findLabelByID(ctx context.Context, tx *bolt.Tx, id influxdb.ID) if len(v) == 0 { return nil, &influxdb.Error{ Code: influxdb.ENotFound, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, } } diff --git a/document.go b/document.go index eac139e31e..d5a24b1961 100644 --- a/document.go +++ b/document.go @@ -58,7 +58,7 @@ type DocumentIndex interface { FindOrganizationByName(n string) (ID, error) FindOrganizationByID(id ID) error - FindLabelByName(n string) (ID, error) + FindLabelByID(id ID) error AddDocumentLabel(docID, labelID ID) error RemoveDocumentLabel(docID, labelID ID) error @@ -222,11 +222,11 @@ func TokenAuthorizedWithOrgID(a *Authorization, orgID ID) func(ID, DocumentIndex } // WithLabel adds a label to the documents where it is applied. -func WithLabel(label string) func(ID, DocumentIndex) error { +func WithLabel(lid ID) func(ID, DocumentIndex) error { return func(id ID, idx DocumentIndex) error { // TODO(desa): turns out that labels are application global, at somepoint we'll // want to scope these by org. We should add auth at that point. - lid, err := idx.FindLabelByName(label) + err := idx.FindLabelByID(lid) if err != nil { return err } @@ -236,11 +236,11 @@ func WithLabel(label string) func(ID, DocumentIndex) error { } // WithoutLabel removes a label to the documents where it is applied. -func WithoutLabel(label string) func(ID, DocumentIndex) error { +func WithoutLabel(lid ID) func(ID, DocumentIndex) error { return func(id ID, idx DocumentIndex) error { // TODO(desa): turns out that labels are application global, at somepoint we'll // want to scope these by org. We should add auth at that point. - lid, err := idx.FindLabelByName(label) + err := idx.FindLabelByID(lid) if err != nil { return err } diff --git a/http/document_service.go b/http/document_service.go index edeed0d165..b8f3d5519e 100644 --- a/http/document_service.go +++ b/http/document_service.go @@ -112,7 +112,6 @@ func (h *DocumentHandler) handlePostDocument(w http.ResponseWriter, r *http.Requ EncodeError(ctx, err, w) return } - s, err := h.DocumentService.FindDocumentStore(ctx, req.Namespace) if err != nil { EncodeError(ctx, err, w) @@ -149,16 +148,20 @@ func (h *DocumentHandler) handlePostDocument(w http.ResponseWriter, r *http.Requ type postDocumentRequest struct { *influxdb.Document - Namespace string `json:"-"` - Org string `json:"org"` - OrgID influxdb.ID `json:"orgID,omitempty"` - Labels []string `json:"labels"` // TODO(desa): should this be IDs or strings? + Namespace string `json:"-"` + Org string `json:"org"` + OrgID influxdb.ID `json:"orgID,omitempty"` + Labels []influxdb.ID `json:"labels"` } func decodePostDocumentRequest(ctx context.Context, r *http.Request) (*postDocumentRequest, error) { req := &postDocumentRequest{} if err := json.NewDecoder(r.Body).Decode(req); err != nil { - return nil, err + return nil, &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: "document body error", + Err: err, + } } if req.Document == nil { diff --git a/http/document_test.go b/http/document_test.go index 0f660db4b6..ee8d8ec8ca 100644 --- a/http/document_test.go +++ b/http/document_test.go @@ -22,6 +22,8 @@ var ( doc2ID = influxtesting.MustIDBase16("020f755c3c082011") doc3ID = influxtesting.MustIDBase16("020f755c3c082012") doc4ID = influxtesting.MustIDBase16("020f755c3c082013") + doc5ID = influxtesting.MustIDBase16("020f755c3c082014") + doc6ID = influxtesting.MustIDBase16("020f755c3c082015") user1ID = influxtesting.MustIDBase16("020f755c3c082001") label1ID = influxtesting.MustIDBase16("020f755c3c082300") label2ID = influxtesting.MustIDBase16("020f755c3c082301") @@ -76,6 +78,31 @@ var ( }, Content: "content4", } + doc5 = influxdb.Document{ + ID: doc5ID, + Meta: influxdb.DocumentMeta{ + Name: "doc5", + }, + Content: "content5", + } + doc5JSON, _ = json.Marshal(doc5) + doc6 = influxdb.Document{ + ID: doc6ID, + Meta: influxdb.DocumentMeta{ + Name: "doc6", + }, + Content: "content6", + } + doc6JSON, _ = json.Marshal( + postDocumentRequest{ + Document: &doc6, + Labels: []influxdb.ID{ + label1ID, + label2ID, + }, + }, + ) + docs = []*influxdb.Document{ &doc1, &doc2, @@ -700,3 +727,185 @@ func TestService_handleGetDocuments(t *testing.T) { }) } } + +func TestService_handlePostDocuments(t *testing.T) { + type fields struct { + DocumentService influxdb.DocumentService + LabelService influxdb.LabelService + } + type args struct { + body *bytes.Buffer + queryParams map[string][]string + authorizer influxdb.Authorizer + } + type wants struct { + statusCode int + contentType string + body string + } + + tests := []struct { + name string + fields fields + args args + wants wants + }{ + { + name: "blank body", + fields: fields{ + DocumentService: &mock.DocumentService{}, + LabelService: &mock.LabelService{}, + }, + args: args{ + body: bytes.NewBuffer([]byte{}), + queryParams: map[string][]string{ + "org": []string{"org1"}, + }, + authorizer: &influxdb.Session{UserID: user1ID}, + }, + wants: wants{ + statusCode: http.StatusBadRequest, + contentType: "application/json; charset=utf-8", + body: `{"code":"invalid","error":"EOF","message": "document body error"}`, + }, + }, + { + name: "empty json", + fields: fields{ + DocumentService: &mock.DocumentService{}, + LabelService: &mock.LabelService{}, + }, + args: args{ + body: bytes.NewBuffer([]byte(`{}`)), + queryParams: map[string][]string{ + "org": []string{"org1"}, + }, + authorizer: &influxdb.Session{UserID: user1ID}, + }, + wants: wants{ + statusCode: http.StatusBadRequest, + contentType: "application/json; charset=utf-8", + body: `{"code":"invalid","message": "missing document body"}`, + }, + }, + { + name: "without label", + fields: fields{ + DocumentService: &mock.DocumentService{ + FindDocumentStoreFn: func(context.Context, string) (influxdb.DocumentStore, error) { + return &mock.DocumentStore{ + CreateDocumentFn: func(ctx context.Context, d *influxdb.Document, opts ...influxdb.DocumentOptions) error { + return nil + }, + }, nil + }, + }, + LabelService: &mock.LabelService{}, + }, + args: args{ + body: bytes.NewBuffer(doc5JSON), + queryParams: map[string][]string{ + "org": []string{"org1"}, + }, + authorizer: &influxdb.Session{UserID: user1ID}, + }, + wants: wants{ + statusCode: http.StatusCreated, + contentType: "application/json; charset=utf-8", + body: `{ + "content": "content5", + "id": "020f755c3c082014", + "links": { + "self": "/api/v2/documents/template/020f755c3c082014" + }, + "meta": { + "name": "doc5" + }}`, + }, + }, + { + name: "with label", + fields: fields{ + DocumentService: &mock.DocumentService{ + FindDocumentStoreFn: func(context.Context, string) (influxdb.DocumentStore, error) { + return &mock.DocumentStore{ + CreateDocumentFn: func(ctx context.Context, d *influxdb.Document, opts ...influxdb.DocumentOptions) error { + d.Labels = []*influxdb.Label{&label1, &label2} + return nil + }, + }, nil + }, + }, + LabelService: &mock.LabelService{ + FindLabelByIDFn: func(ctx context.Context, id influxdb.ID) (*influxdb.Label, error) { + if id == label1ID { + return &label1, nil + } + return &label2, nil + }, + }, + }, + args: args{ + body: bytes.NewBuffer(doc6JSON), + queryParams: map[string][]string{ + "org": []string{"org1"}, + }, + authorizer: &influxdb.Session{UserID: user1ID}, + }, + wants: wants{ + statusCode: http.StatusCreated, + contentType: "application/json; charset=utf-8", + body: `{ + "content": "content6", + "id": "020f755c3c082015", + "links": { + "self": "/api/v2/documents/template/020f755c3c082015" + }, + "labels": [{ + "id": "020f755c3c082300", + "name": "l1" + }, + { + "id": "020f755c3c082301", + "name": "l2" + }], + "meta": { + "name": "doc6" + }}`, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + documentBackend := NewMockDocumentBackend() + documentBackend.DocumentService = tt.fields.DocumentService + documentBackend.LabelService = tt.fields.LabelService + h := NewDocumentHandler(documentBackend) + r := httptest.NewRequest("POST", "http://any.url", tt.args.body) + r = r.WithContext(pcontext.SetAuthorizer(r.Context(), tt.args.authorizer)) + r = r.WithContext(context.WithValue(r.Context(), + httprouter.ParamsKey, + httprouter.Params{ + { + Key: "ns", + Value: "template", + }, + })) + w := httptest.NewRecorder() + h.handlePostDocument(w, r) + res := w.Result() + content := res.Header.Get("Content-Type") + body, _ := ioutil.ReadAll(res.Body) + + if res.StatusCode != tt.wants.statusCode { + t.Errorf("%q. handlePostDocument() = %v, want %v", tt.name, res.StatusCode, tt.wants.statusCode) + } + if tt.wants.contentType != "" && content != tt.wants.contentType { + t.Errorf("%q. handlePostDocument() = %v, want %v", tt.name, content, tt.wants.contentType) + } + if eq, diff, _ := jsonEqual(string(body), tt.wants.body); !eq { + t.Errorf("%q. handlePostDocument() = ***%s***", tt.name, diff) + } + }) + } +} diff --git a/http/label_test.go b/http/label_test.go index dd394c15fa..578f7b4fa8 100644 --- a/http/label_test.go +++ b/http/label_test.go @@ -200,7 +200,7 @@ func TestService_handleGetLabel(t *testing.T) { FindLabelByIDFn: func(ctx context.Context, id platform.ID) (*platform.Label, error) { return nil, &platform.Error{ Code: platform.ENotFound, - Err: platform.ErrLabelNotFound, + Msg: platform.ErrLabelNotFound, } }, }, @@ -382,7 +382,7 @@ func TestService_handleDeleteLabel(t *testing.T) { DeleteLabelFn: func(ctx context.Context, id platform.ID) error { return &platform.Error{ Code: platform.ENotFound, - Err: platform.ErrLabelNotFound, + Msg: platform.ErrLabelNotFound, } }, }, @@ -514,7 +514,7 @@ func TestService_handlePatchLabel(t *testing.T) { UpdateLabelFn: func(ctx context.Context, id platform.ID, upd platform.LabelUpdate) (*platform.Label, error) { return nil, &platform.Error{ Code: platform.ENotFound, - Err: platform.ErrLabelNotFound, + Msg: platform.ErrLabelNotFound, } }, }, diff --git a/http/swagger.yml b/http/swagger.yml index 0cbb42e57c..4c73f86385 100644 --- a/http/swagger.yml +++ b/http/swagger.yml @@ -7059,7 +7059,7 @@ components: description: must specify one of orgID and org labels: type: array - description: this is an array of label strings that will be added as labels to the document + description: this is an array of label IDs that will be added as labels to the document items: type: string required: diff --git a/inmem/label_service.go b/inmem/label_service.go index 246f597309..f8eb83bb4d 100644 --- a/inmem/label_service.go +++ b/inmem/label_service.go @@ -13,7 +13,7 @@ func (s *Service) loadLabel(ctx context.Context, id influxdb.ID) (*influxdb.Labe if !ok { return nil, &influxdb.Error{ Code: influxdb.ENotFound, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, } } @@ -160,7 +160,7 @@ func (s *Service) UpdateLabel(ctx context.Context, id influxdb.ID, upd influxdb. return nil, &influxdb.Error{ Code: influxdb.ENotFound, Op: OpPrefix + influxdb.OpUpdateLabel, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, } } @@ -207,7 +207,7 @@ func (s *Service) DeleteLabel(ctx context.Context, id influxdb.ID) error { return &influxdb.Error{ Code: influxdb.ENotFound, Op: OpPrefix + influxdb.OpDeleteLabel, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, } } diff --git a/kv/document.go b/kv/document.go index 368e50fc06..a45578ef2f 100644 --- a/kv/document.go +++ b/kv/document.go @@ -108,12 +108,12 @@ func (s *DocumentStore) CreateDocument(ctx context.Context, d *influxdb.Document writable: true, } for _, opt := range opts { - if err := opt(d.ID, idx); err != nil { + if err = opt(d.ID, idx); err != nil { return err } } - if err := s.decorateDocumentWithLabels(ctx, tx, d); err != nil { + if err = s.decorateDocumentWithLabels(ctx, tx, d); err != nil { return err } @@ -157,31 +157,10 @@ func (i *DocumentIndex) RemoveDocumentLabel(docID, labelID influxdb.ID) error { return nil } -// FindLabelByName retrieves a label by name. -func (i *DocumentIndex) FindLabelByName(name string) (influxdb.ID, error) { - // TODO(desa): this should be scoped by organization eventually. As of now labels are - // global to the application. - m := influxdb.LabelFilter{ - Name: name, - } - ls, err := i.service.findLabels(i.ctx, i.tx, m) - if err != nil { - return influxdb.InvalidID(), err - } - if len(ls) == 0 { - return influxdb.InvalidID(), &influxdb.Error{ - Code: influxdb.ENotFound, - Msg: "label not found", - } - } - if len(ls) > 1 { - return influxdb.InvalidID(), &influxdb.Error{ - Code: influxdb.EInternal, - Msg: "found multiple labels matching the name provided", - } - } - - return ls[0].ID, nil +// FindLabelByID retrieves a label by id. +func (i *DocumentIndex) FindLabelByID(id influxdb.ID) error { + _, err := i.service.findLabelByID(i.ctx, i.tx, id) + return err } // AddDocumentOwner creates a urm for the document id and owner id provided. diff --git a/kv/label.go b/kv/label.go index ae8e3c89a5..82c8e4a692 100644 --- a/kv/label.go +++ b/kv/label.go @@ -65,7 +65,7 @@ func (s *Service) findLabelByID(ctx context.Context, tx Tx, id influxdb.ID) (*in if IsNotFound(err) { return nil, &influxdb.Error{ Code: influxdb.ENotFound, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, } } diff --git a/label.go b/label.go index 71f262f87a..ff8a8913ea 100644 --- a/label.go +++ b/label.go @@ -5,7 +5,7 @@ import ( ) // ErrLabelNotFound is the error for a missing Label. -const ErrLabelNotFound = ChronografError("label not found") +const ErrLabelNotFound = "label not found" const ( OpFindLabels = "FindLabels" diff --git a/testing/document.go b/testing/document.go index 8b488d74f5..7e5031abba 100644 --- a/testing/document.go +++ b/testing/document.go @@ -34,7 +34,7 @@ func NewDocumentIntegrationTest(store kv.Store) func(t *testing.T) { l1 := &influxdb.Label{Name: "l1"} l2 := &influxdb.Label{Name: "l2"} mustCreateLabels(ctx, svc, l1, l2) - lBad := &influxdb.Label{Name: "bad"} + lBad := &influxdb.Label{ID: MustIDBase16(oneID), Name: "bad"} o1 := &influxdb.Organization{Name: "foo"} o2 := &influxdb.Organization{Name: "bar"} @@ -66,7 +66,7 @@ func NewDocumentIntegrationTest(store kv.Store) func(t *testing.T) { "v1": "v1", }, } - if err := s.CreateDocument(ctx, d1, influxdb.AuthorizedWithOrg(s1, o1.Name), influxdb.WithLabel(l1.Name)); err != nil { + if err := s.CreateDocument(ctx, d1, influxdb.AuthorizedWithOrg(s1, o1.Name), influxdb.WithLabel(l1.ID)); err != nil { t.Errorf("failed to create document: %v", err) } }) @@ -80,7 +80,7 @@ func NewDocumentIntegrationTest(store kv.Store) func(t *testing.T) { "i2": "i2", }, } - if err := s.CreateDocument(ctx, d2, influxdb.AuthorizedWithOrg(s2, o1.Name), influxdb.WithLabel(l2.Name)); err == nil { + if err := s.CreateDocument(ctx, d2, influxdb.AuthorizedWithOrg(s2, o1.Name), influxdb.WithLabel(l2.ID)); err == nil { t.Fatalf("should not have be authorized to create document") } @@ -119,7 +119,7 @@ func NewDocumentIntegrationTest(store kv.Store) func(t *testing.T) { "k4": "v4", }, } - err = s.CreateDocument(ctx, d4, influxdb.WithLabel(lBad.Name)) + err = s.CreateDocument(ctx, d4, influxdb.WithLabel(lBad.ID)) ErrorsEqual(t, err, &influxdb.Error{ Code: influxdb.ENotFound, Msg: "label not found", diff --git a/testing/label_service.go b/testing/label_service.go index 66e26b7128..96daf6ac68 100644 --- a/testing/label_service.go +++ b/testing/label_service.go @@ -300,7 +300,7 @@ func FindLabelByID( err: &influxdb.Error{ Code: influxdb.ENotFound, Op: influxdb.OpFindLabelByID, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, }, }, }, @@ -490,7 +490,7 @@ func UpdateLabel( err: &influxdb.Error{ Code: influxdb.ENotFound, Op: influxdb.OpUpdateLabel, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, }, }, }, @@ -587,7 +587,7 @@ func DeleteLabel( err: &influxdb.Error{ Code: influxdb.ENotFound, Op: influxdb.OpDeleteLabel, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, }, }, }, @@ -675,7 +675,7 @@ func CreateLabelMapping( err: &influxdb.Error{ Code: influxdb.ENotFound, Op: influxdb.OpDeleteLabel, - Err: influxdb.ErrLabelNotFound, + Msg: influxdb.ErrLabelNotFound, }, }, }, diff --git a/testing/util.go b/testing/util.go index 005bf5e2f1..799db69d3a 100644 --- a/testing/util.go +++ b/testing/util.go @@ -28,12 +28,12 @@ func ErrorsEqual(t *testing.T, actual, expected error) { } if platform.ErrorCode(expected) != platform.ErrorCode(actual) { - t.Logf("\nexpected: %v\nactual %v\n\n", expected, actual) + t.Logf("\nexpected: %v\nactual: %v\n\n", expected, actual) t.Errorf("expected error code %q but received %q", platform.ErrorCode(expected), platform.ErrorCode(actual)) } if platform.ErrorMessage(expected) != platform.ErrorMessage(actual) { - t.Logf("\nexpected: %v\nactual %v\n\n", expected, actual) + t.Logf("\nexpected: %v\nactual: %v\n\n", expected, actual) t.Errorf("expected error message %q but received %q", platform.ErrorMessage(expected), platform.ErrorMessage(actual)) } }