Repository: ably/ably-go (go)

Issue:
HTTPPaginatedResult and PaginatedResult are missing some methods
It appears https://github.com/ably/ably-go/blob/develop/ably/http_paginated_response.go is missing various methods, such as `IsLast`, `HasNext` and `First`. They should have these methods to match the other libraries.

Once added, we need to also re-add these methods to the docs. Currently they are commented out with the indicator `GO_REPLACEMENT` to easily find and uncomment them.

┆sprint: SDK 2022-Q1-S6


Pull request description:
Add missing pagination methods
Fixes #120 
First doesn't appear to be missing

Patch:
diff --git a/ably/doc.go b/ably/doc.go
index 2d7a41f..da31933 100644
--- a/ably/doc.go
+++ b/ably/doc.go
@@ -58,6 +58,13 @@
 // from the iterator object. Finally, once it returns false, the Err method must
 // be called to check if the iterator stopped due to some error, or else, it
 // just finished going through all pages.
+
+// Calling the First method on the PaginatedResults returns the first page of the
+// results. However, the Next method has to be called before inspecting the items.
+//
+// For every page in the PaginatedResults, the HasNext method can be called to check if there
+// are more page(s) available. IsLast method checks if the page is the
+// last page. Both methods return a true or false value.
 //
 // See the PaginatedResults example.
 package ably
diff --git a/ably/rest_channel.go b/ably/rest_channel.go
index 3579a6d..c8a0747 100644
--- a/ably/rest_channel.go
+++ b/ably/rest_channel.go
@@ -236,6 +236,20 @@ func (p *MessagesPaginatedResult) Next(ctx context.Context) bool {
 	return p.next(ctx, &p.items)
 }
 
+// IsLast returns true if the page is last page.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *MessagesPaginatedResult) IsLast(ctx context.Context) bool {
+	return !p.HasNext(ctx)
+}
+
+// HasNext returns true is there are more pages available.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *MessagesPaginatedResult) HasNext(ctx context.Context) bool {
+	return p.nextLink != ""
+}
+
 // Items returns the current page of results.
 //
 // See the "Paginated results" section in the package-level documentation.
diff --git a/ably/rest_client.go b/ably/rest_client.go
index add5cb4..564724a 100644
--- a/ably/rest_client.go
+++ b/ably/rest_client.go
@@ -244,6 +244,20 @@ func (p *StatsPaginatedResult) Next(ctx context.Context) bool {
 	return p.next(ctx, &p.items)
 }
 
+// IsLast returns true if the page is last page.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *StatsPaginatedResult) IsLast(ctx context.Context) bool {
+	return !p.HasNext(ctx)
+}
+
+// HasNext returns true is there are more pages available.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *StatsPaginatedResult) HasNext(ctx context.Context) bool {
+	return p.nextLink != ""
+}
+
 // Items returns the current page of results.
 //
 // See the "Paginated results" section in the package-level documentation.
@@ -427,6 +441,20 @@ func (p *HTTPPaginatedResponse) Next(ctx context.Context) bool {
 	return p.next(ctx, &p.items)
 }
 
+// IsLast returns true if the page is last page.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *HTTPPaginatedResponse) IsLast(ctx context.Context) bool {
+	return !p.HasNext(ctx)
+}
+
+// HasNext returns true is there are more pages available.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *HTTPPaginatedResponse) HasNext(ctx context.Context) bool {
+	return p.nextLink != ""
+}
+
 // Items unmarshals the current page of results as JSON into the provided
 // variable.
 //
@@ -470,6 +498,20 @@ func (p *RESTPaginatedItems) Next(ctx context.Context) bool {
 	return true
 }
 
+// IsLast returns true if the page is last page.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *RESTPaginatedItems) IsLast(ctx context.Context) bool {
+	return !p.HasNext(ctx)
+}
+
+// HasNext returns true is there are more pages available.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *RESTPaginatedItems) HasNext(ctx context.Context) bool {
+	return p.nextLink != ""
+}
+
 // Item unmarshal the current result as JSON into the provided variable.
 //
 // See the "Paginated results" section in the package-level documentation.
diff --git a/ably/rest_presence.go b/ably/rest_presence.go
index bc0a6ba..715f8d8 100644
--- a/ably/rest_presence.go
+++ b/ably/rest_presence.go
@@ -140,6 +140,17 @@ func (p *PresencePaginatedResult) Next(ctx context.Context) bool {
 	return p.next(ctx, p.decoder(&p.items))
 }
 
+// IsLast returns true if the page is last page.
+//
+// See "Paginated results" section in the package-level documentation.
+func (p *PresencePaginatedResult) IsLast(ctx context.Context) bool {
+	return !p.HasNext(ctx)
+}
+
+func (p *PresencePaginatedResult) HasNext(ctx context.Context) bool {
+	return p.nextLink != ""
+}
+
 // Items returns the current page of results.
 //
 // See the "Paginated results" section in the package-level documentation.


Test patch:
diff --git a/ably/auth_internal_test.go b/ably/auth_internal_test.go
index cfa1505..79f783c 100644
--- a/ably/auth_internal_test.go
+++ b/ably/auth_internal_test.go
@@ -4,8 +4,10 @@
 package ably
 
 import (
+	"bytes"
 	"context"
 	"errors"
+	"log"
 	"net/http"
 	"net/url"
 	"testing"
@@ -13,6 +15,11 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+var (
+	buffer     bytes.Buffer
+	mocklogger = log.New(&buffer, "logger: ", log.Lshortfile)
+)
+
 func TestAddParams(t *testing.T) {
 	tests := map[string]struct {
 		leftHandSide   url.Values
diff --git a/ably/doc_test.go b/ably/doc_test.go
index 7a7670b..6437163 100644
--- a/ably/doc_test.go
+++ b/ably/doc_test.go
@@ -1,8 +1,46 @@
 package ably_test
 
-import "fmt"
+import (
+	"fmt"
+
+	"context"
+	"github.com/ably/ably-go/ably"
+)
 
 func Example_paginatedResults() {
-	fmt.Println("TODO once the Requests method is adapted")
-	// Output: TODO once the Requests method is adapted
+	ctx := context.Background()
+	client, err := ably.NewRealtime(ably.WithKey("xxx:xxx"))
+	if err != nil {
+		panic(err)
+	}
+	channel := client.Channels.Get("persisted:test")
+
+	err = channel.Publish(ctx, "EventName1", "EventData1")
+	if err != nil {
+		panic(err)
+	}
+
+	pages, err := channel.History().Pages(ctx)
+	if err != nil {
+		panic(err)
+	}
+
+	// Returning and iterating over the first page
+	pages.Next(ctx)
+	pages.First(ctx)
+	for _, message := range pages.Items() {
+		fmt.Println(message)
+	}
+
+	// Iteration over pages in PaginatedResult
+	for pages.Next(ctx) {
+		fmt.Println(pages.HasNext(ctx))
+		fmt.Println(pages.IsLast(ctx))
+		for _, presence := range pages.Items() {
+			fmt.Println(presence)
+		}
+	}
+	if err := pages.Err(); err != nil {
+		panic(err)
+	}
 }
diff --git a/ably/mock_test.go b/ably/mock_test.go
deleted file mode 100644
index 1c6ffdf..0000000
--- a/ably/mock_test.go
+++ /dev/null
@@ -1,32 +0,0 @@
-// mocks and helpers for unit tests.
-
-package ably
-
-import (
-	"bytes"
-	"log"
-)
-
-var (
-	buffer     bytes.Buffer
-	mocklogger = log.New(&buffer, "logger: ", log.Lshortfile)
-)
-
-// mockChannelWithState is a test helper that returns a mock channel in a specified state
-func mockChannelWithState(channelState *ChannelState, connectionState *ConnectionState) *RealtimeChannel {
-	mockChannel := RealtimeChannel{
-		client: &Realtime{
-			rest: &REST{
-				log: logger{l: &stdLogger{mocklogger}},
-			},
-			Connection: &Connection{},
-		},
-	}
-	if channelState != nil {
-		mockChannel.state = *channelState
-	}
-	if connectionState != nil {
-		mockChannel.client.Connection.state = *connectionState
-	}
-	return &mockChannel
-}
diff --git a/ably/realtime_presence_internal_test.go b/ably/realtime_presence_internal_test.go
deleted file mode 100644
index ad112fd..0000000
--- a/ably/realtime_presence_internal_test.go
+++ /dev/null
@@ -1,89 +0,0 @@
-//go:build !integration
-// +build !integration
-
-package ably
-
-import (
-	"errors"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestVerifyChanState(t *testing.T) {
-	tests := map[string]struct {
-		channel     *RealtimeChannel
-		expectedErr error
-	}{
-		`No error if the channel is in state: "INITIALIZED"`: {
-			channel:     mockChannelWithState(&ChannelStateInitialized, nil),
-			expectedErr: nil,
-		},
-		`No error if the channel is in state: "ATTACHING"`: {
-			channel:     mockChannelWithState(&ChannelStateAttaching, nil),
-			expectedErr: nil,
-		},
-		`No error if the channel is in state: "ATTACHED"`: {
-			channel:     mockChannelWithState(&ChannelStateAttached, nil),
-			expectedErr: nil,
-		},
-		`No error if the channel is in state: "SUSPENDED"`: {
-			channel:     mockChannelWithState(&ChannelStateSuspended, nil),
-			expectedErr: nil,
-		},
-		`Error if the channel is in state: "DETACHING"`: {
-			channel:     mockChannelWithState(&ChannelStateDetaching, nil),
-			expectedErr: newError(91001, errors.New("unable to enter presence channel (invalid channel state: DETACHING)")),
-		},
-		`Error if the channel is in state: "DETACHED"`: {
-			channel:     mockChannelWithState(&ChannelStateDetached, nil),
-			expectedErr: newError(91001, errors.New("unable to enter presence channel (invalid channel state: DETACHED)")),
-		},
-		`Error if the channel is in state: "FAILED"`: {
-			channel:     mockChannelWithState(&ChannelStateFailed, nil),
-			expectedErr: newError(91001, errors.New("unable to enter presence channel (invalid channel state: FAILED)")),
-		},
-	}
-
-	for testName, test := range tests {
-		t.Run(testName, func(t *testing.T) {
-			presence := newRealtimePresence(test.channel)
-			err := presence.verifyChanState()
-			assert.Equal(t, test.expectedErr, err)
-		})
-	}
-}
-
-func TestSend(t *testing.T) {
-	tests := map[string]struct {
-		channel        *RealtimeChannel
-		msg            PresenceMessage
-		expectedResult result
-		expectedErr    error
-	}{
-		`No error sending presence if the channel is in state: "ATTACHED"`: {
-			channel: mockChannelWithState(&ChannelStateAttached, nil),
-			msg: PresenceMessage{
-				Message: Message{Name: "Hello"},
-				Action:  PresenceActionEnter,
-			},
-			expectedErr: nil,
-		},
-		`Error if channel is: "DETACHED" and connection is :"CLOSED"`: {
-			channel: mockChannelWithState(&ChannelStateDetached, &ConnectionStateClosed),
-			msg: PresenceMessage{
-				Message: Message{Name: "Hello"},
-				Action:  PresenceActionEnter,
-			},
-			expectedErr: newError(80000, errors.New("cannot Attach channel because connection is in CLOSED state")),
-		},
-	}
-
-	for testName, test := range tests {
-		t.Run(testName, func(t *testing.T) {
-			presence := newRealtimePresence(test.channel)
-			_, err := presence.send(&test.msg)
-			assert.Equal(t, test.expectedErr, err)
-		})
-	}
-}
diff --git a/ablytest/pagination.go b/ablytest/pagination.go
index acd8dcd..244afc0 100644
--- a/ablytest/pagination.go
+++ b/ablytest/pagination.go
@@ -73,7 +73,11 @@ func testPagination(request reflect.Value, expectedItems []interface{}, perPage
 		var gotItems []interface{}
 		pageNum := 1
 		expectedFullPages := len(expectedItems) / perPage
+		pageMap := make(map[int]map[string]bool, expectedFullPages)
 		for pages.next() {
+			pageMap[pageNum] = map[string]bool{}
+			pageMap[pageNum]["hasNext"] = pages.hasNext()
+			pageMap[pageNum]["isLast"] = pages.isLast()
 			if (pageNum <= expectedFullPages && len(pages.items()) != perPage) ||
 				(pageNum > expectedFullPages && len(pages.items()) >= perPage) {
 				return fmt.Errorf("page #%d got %d items, expected at most %d", pageNum, len(pages.items()), perPage)
@@ -81,6 +85,20 @@ func testPagination(request reflect.Value, expectedItems []interface{}, perPage
 			gotItems = append(gotItems, pages.items()...)
 			pageNum++
 		}
+
+		lastPage := pageNum - 1
+		for idx, page := range pageMap {
+			if idx == lastPage {
+				if page["hasNext"] || !page["isLast"] {
+					return fmt.Errorf("page #%d got incorrect HasNext and IsLast boolean values", idx)
+				}
+			} else {
+				if !page["hasNext"] || page["isLast"] {
+					return fmt.Errorf("page #%d got incorrect HasNext and IsLast boolean values", idx)
+				}
+			}
+		}
+
 		if err := pages.err(); err != nil {
 			return fmt.Errorf("iterating pages: %w", err)
 		}
@@ -124,9 +142,11 @@ func testPagination(request reflect.Value, expectedItems []interface{}, perPage
 }
 
 type paginated struct {
-	next  func() bool
-	first func() error
-	err   func() error
+	next    func() bool
+	first   func() error
+	err     func() error
+	hasNext func() bool
+	isLast  func() bool
 }
 
 type paginatedResult struct {
@@ -161,6 +181,12 @@ func generalizePaginatedRequest(request reflect.Value) (
 				err, _ := r.MethodByName("First").Call([]reflect.Value{ctx})[0].Interface().(error)
 				return err
 			},
+			hasNext: func() bool {
+				return r.MethodByName("HasNext").Call([]reflect.Value{ctx})[0].Bool()
+			},
+			isLast: func() bool {
+				return r.MethodByName("IsLast").Call([]reflect.Value{ctx})[0].Bool()
+			},
 			err: func() error {
 				err, _ := r.MethodByName("Err").Call(nil)[0].Interface().(error)
 				return err