From a629f22aebf208bc93265c3a5ba02e3d2b7fa358 Mon Sep 17 00:00:00 2001 From: Caleb McCombs Date: Tue, 13 Jun 2023 12:26:10 -0700 Subject: [PATCH] Separate confluencev2.page.Bulk To BulkFiltered There are more parameters available to the bulk query which would, unfortunately, not be backward compatible with current version of Bulk so rather than affecting that, I'm switching to having one implementation that can handle all the filtering and Bulk() calls it without any filtering. --- confluence/internal/page_impl.go | 40 +++++++-- confluence/internal/page_impl_test.go | 121 +++++++++++++++++++++++++- service/confluence/page.go | 13 ++- 3 files changed, 162 insertions(+), 12 deletions(-) diff --git a/confluence/internal/page_impl.go b/confluence/internal/page_impl.go index a306c7a3..9336c020 100644 --- a/confluence/internal/page_impl.go +++ b/confluence/internal/page_impl.go @@ -41,8 +41,21 @@ func (p *PageService) Get(ctx context.Context, pageID int, format string, draft // GET /wiki/api/v2/pages // // https://docs.go-atlassian.io/confluence-cloud/v2/page#get-pages -func (p *PageService) Bulk(ctx context.Context, cursor string, limit int, pageIDs ...int) (*model.PageChunkScheme, *model.ResponseScheme, error) { - return p.internalClient.Bulk(ctx, cursor, limit, pageIDs...) +func (p *PageService) Bulk(ctx context.Context, cursor string, limit int) (*model.PageChunkScheme, *model.ResponseScheme, error) { + return p.internalClient.Bulk(ctx, cursor, limit) +} + +// BulkFiltered returns all pages that fit the filtering criteria +// +// # The number of results is limited by the limit parameter and additional results +// +// (if available) will be available through the next cursor +// +// GET /wiki/api/v2/pages +// +// https://docs.go-atlassian.io/confluence-cloud/v2/page#get-pages +func (p *PageService) BulkFiltered(ctx context.Context, status, format, cursor string, limit int, pageIDs ...int) (*model.PageChunkScheme, *model.ResponseScheme, error) { + return p.internalClient.BulkFiltered(ctx, status, format, cursor, limit, pageIDs...) } // GetsByLabel returns the pages of specified label. @@ -142,10 +155,27 @@ func (i *internalPageImpl) Get(ctx context.Context, pageID int, format string, d return page, response, nil } -func (i *internalPageImpl) Bulk(ctx context.Context, cursor string, limit int, pageIDs ...int) (*model.PageChunkScheme, *model.ResponseScheme, error) { +func (i *internalPageImpl) Bulk(ctx context.Context, cursor string, limit int) (*model.PageChunkScheme, *model.ResponseScheme, error) { + return i.BulkFiltered(ctx, "", "", cursor, limit) +} + +func (i *internalPageImpl) BulkFiltered(ctx context.Context, status, format, cursor string, limit int, pageIDs ...int) (*model.PageChunkScheme, *model.ResponseScheme, error) { query := url.Values{} query.Add("limit", strconv.Itoa(limit)) + + if status != "" { + query.Add("status", status) + } + + if format != "" { + query.Add("body-format", format) + } + + if cursor != "" { + query.Add("cursor", cursor) + } + if len(pageIDs) > 0 { ids := make([]string, 0, len(pageIDs)) for _, id := range pageIDs { @@ -154,10 +184,6 @@ func (i *internalPageImpl) Bulk(ctx context.Context, cursor string, limit int, p query.Add("id", strings.Join(ids, ",")) } - if cursor != "" { - query.Add("cursor", cursor) - } - endpoint := fmt.Sprintf("wiki/api/v2/pages?%v", query.Encode()) request, err := i.c.NewRequest(ctx, http.MethodGet, endpoint, nil) diff --git a/confluence/internal/page_impl_test.go b/confluence/internal/page_impl_test.go index 4933eb2f..94cc2b4b 100644 --- a/confluence/internal/page_impl_test.go +++ b/confluence/internal/page_impl_test.go @@ -138,8 +138,112 @@ func Test_internalPageImpl_Bulk(t *testing.T) { c service.Client } + type args struct { + ctx context.Context + cursor string + limit int + } + + testCases := []struct { + name string + fields fields + args args + on func(*fields) + wantErr bool + Err error + }{ + { + name: "when the parameters are correct", + args: args{ + ctx: context.TODO(), + cursor: "cursor-sample", + limit: 200, + }, + on: func(fields *fields) { + + client := mocks.NewClient(t) + + client.On("NewRequest", + context.Background(), + http.MethodGet, + "wiki/api/v2/pages?cursor=cursor-sample&limit=200", + nil). + Return(&http.Request{}, nil) + + client.On("Call", + &http.Request{}, + &model.PageChunkScheme{}). + Return(&model.ResponseScheme{}, nil) + + fields.c = client + }, + }, + + { + name: "when the http request cannot be created", + args: args{ + ctx: context.TODO(), + cursor: "cursor-sample", + limit: 200, + }, + on: func(fields *fields) { + + client := mocks.NewClient(t) + + client.On("NewRequest", + context.Background(), + http.MethodGet, + "wiki/api/v2/pages?cursor=cursor-sample&limit=200", + nil). + Return(&http.Request{}, errors.New("error, unable to create the http request")) + + fields.c = client + + }, + wantErr: true, + Err: errors.New("error, unable to create the http request"), + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + + if testCase.on != nil { + testCase.on(&testCase.fields) + } + + newService := NewPageService(testCase.fields.c) + + gotResult, gotResponse, err := newService.Bulk(testCase.args.ctx, testCase.args.cursor, testCase.args.limit) + + if testCase.wantErr { + + if err != nil { + t.Logf("error returned: %v", err.Error()) + } + + assert.EqualError(t, err, testCase.Err.Error()) + } else { + + assert.NoError(t, err) + assert.NotEqual(t, gotResponse, nil) + assert.NotEqual(t, gotResult, nil) + } + + }) + } +} + +func Test_internalPageImpl_BulkFiltered(t *testing.T) { + + type fields struct { + c service.Client + } + type args struct { ctx context.Context + status string + format string cursor string limit int pageIDs []int @@ -154,7 +258,7 @@ func Test_internalPageImpl_Bulk(t *testing.T) { Err error }{ { - name: "when the parameters are correct", + name: "when the parameters are minimally correct", args: args{ ctx: context.TODO(), cursor: "cursor-sample", @@ -181,9 +285,11 @@ func Test_internalPageImpl_Bulk(t *testing.T) { }, { - name: "when the page ids are included", + name: "when the parameters are maximally correct", args: args{ ctx: context.TODO(), + status: "status-sample", + format: "format-sample", cursor: "cursor-sample", limit: 200, pageIDs: []int{1, 2, 3, 4, 5, 6}, @@ -195,7 +301,7 @@ func Test_internalPageImpl_Bulk(t *testing.T) { client.On("NewRequest", context.Background(), http.MethodGet, - "wiki/api/v2/pages?cursor=cursor-sample&id=1%2C2%2C3%2C4%2C5%2C6&limit=200", + "wiki/api/v2/pages?body-format=format-sample&cursor=cursor-sample&id=1%2C2%2C3%2C4%2C5%2C6&limit=200&status=status-sample", nil). Return(&http.Request{}, nil) @@ -243,7 +349,14 @@ func Test_internalPageImpl_Bulk(t *testing.T) { newService := NewPageService(testCase.fields.c) - gotResult, gotResponse, err := newService.Bulk(testCase.args.ctx, testCase.args.cursor, testCase.args.limit, testCase.args.pageIDs...) + gotResult, gotResponse, err := newService.BulkFiltered( + testCase.args.ctx, + testCase.args.status, + testCase.args.format, + testCase.args.cursor, + testCase.args.limit, + testCase.args.pageIDs..., + ) if testCase.wantErr { diff --git a/service/confluence/page.go b/service/confluence/page.go index b8b861ae..2b9528e3 100644 --- a/service/confluence/page.go +++ b/service/confluence/page.go @@ -25,7 +25,18 @@ type PageConnector interface { // GET /wiki/api/v2/pages // // https://docs.go-atlassian.io/confluence-cloud/v2/page#get-pages - Bulk(ctx context.Context, cursor string, limit int, pageIDs... int) (*models.PageChunkScheme, *models.ResponseScheme, error) + Bulk(ctx context.Context, cursor string, limit int) (*models.PageChunkScheme, *models.ResponseScheme, error) + + // BulkFiltered returns all pages that fit the filtering criteria. + // + // The number of results is limited by the limit parameter and additional results + // + // (if available) will be available through the next cursor + // + // GET /wiki/api/v2/pages + // + // https://docs.go-atlassian.io/confluence-cloud/v2/page#get-pages + BulkFiltered(ctx context.Context, status, format, cursor string, limit int, pageIDs ...int) (*models.PageChunkScheme, *models.ResponseScheme, error) // GetsByLabel returns the pages of specified label. //