Update pagination query to conform to API spec

While looking for API shape mismatches for #389, I stumbled upon a major issue
with our pagination parameters across the entire package. This fixes that
discrepency, but is done via a pretty intrusive breaking change by removing the
embedded `APIListObject` from all `List*Options` types, replacing them with
three distinct fields: `Limit`, `Offset`, and `Total`.

For the traditional API pagination, there is a `total` URL query parameter that
tells the API to include the total count of records in the response. For the
query parameter it's a bool flag, with a value of `true` telling the API to
include the count.

When the response comes back from the API, there is a `total` field in the JSON
that is the total count of items in the collection. This value is only set if
that `total` query parameter is set to `true` because it's an expensive
operation that PagerDuty recommends you don't enable.

The problem is we tried to reuse the `APIListObject` struct type for both the
pagination URL query parameters, and the pagination fields in the JSON response
body. The issue with doing that is for requests the `Total` field needs to be a
bool, but within the `APIListObject` it's a `uint` because it was really only designed
to be used for response body parsing.

So since we're intending to remove the embedded struct types in v2 (#318), it
feels like the best course of action here is to move forward with that plan for
these paginated query parameter struct types so that we can conform to the
PagerDuty API spec.

Related to #318
Related to #389

addon.go

@@ -18,7 +18,25 @@ type Addon struct {
 
 // ListAddonOptions are the options available when calling the ListAddons API endpoint.
 type ListAddonOptions struct {
-	APIListObject
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
+
 	Includes   []string `url:"include,omitempty,brackets"`
 	ServiceIDs []string `url:"service_ids,omitempty,brackets"`
 	Filter     string   `url:"filter,omitempty"`

business_service.go

@@ -44,7 +44,24 @@ type ListBusinessServicesResponse struct {
 
 // ListBusinessServiceOptions is the data structure used when calling the ListBusinessServices API endpoint.
 type ListBusinessServiceOptions struct {
-	APIListObject
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
 }
 
 // ListBusinessServices lists existing business services. This method currently

business_service_test.go

@@ -20,13 +20,18 @@ func TestBusinessService_List(t *testing.T) {
 	listObj := APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
 	client := defaultTestClient(server.URL, "foo")
 	opts := ListBusinessServiceOptions{
-		APIListObject: listObj,
+		Limit:  listObj.Limit,
+		Offset: listObj.Offset,
 	}
 	res, err := client.ListBusinessServices(opts)
 	if err != nil {
 		t.Fatal(err)
 	}
 	want := &ListBusinessServicesResponse{
+		Limit:  listObj.Limit,
+		Offset: listObj.Offset,
+		More:   listObj.More,
+		Total:  listObj.Total,
 		BusinessServices: []*BusinessService{
 			{
 				ID: "1",

client.go

@@ -55,10 +55,10 @@ type APIObject struct {
 
 // APIListObject are the fields used to control pagination when listing objects.
 type APIListObject struct {
-	Limit  uint `url:"limit,omitempty"`
-	Offset uint `url:"offset,omitempty"`
-	More   bool `url:"more,omitempty"`
-	Total  uint `url:"total,omitempty"`
+	Limit  uint `json:"limit,omitempty"`
+	Offset uint `json:"offset,omitempty"`
+	More   bool `json:"more,omitempty"`
+	Total  uint `json:"total,omitempty"`
 }
 
 // APIReference are the fields required to reference another API object.

command/vendor_list.go

@@ -2,11 +2,12 @@ package main
 
 import (
 	"fmt"
+	"strings"
+
 	"github.com/PagerDuty/go-pagerduty"
-	log "github.com/sirupsen/logrus"
 	"github.com/mitchellh/cli"
+	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v2"
-	"strings"
 )
 
 type VendorList struct {
@@ -49,9 +50,7 @@ func (c *VendorList) Run(args []string) int {
 		return -1
 	}
 	client := c.Meta.Client()
-	opts := pagerduty.ListVendorOptions{
-		Query: query,
-	}
+	opts := pagerduty.ListVendorOptions{}
 	if resp, err := client.ListVendors(opts); err != nil {
 		log.Error(err)
 		return -1

escalation_policy.go

@@ -45,7 +45,25 @@ type ListEscalationRulesResponse struct {
 
 // ListEscalationPoliciesOptions is the data structure used when calling the ListEscalationPolicies API endpoint.
 type ListEscalationPoliciesOptions struct {
-	APIListObject
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
+
 	Query    string   `url:"query,omitempty"`
 	UserIDs  []string `url:"user_ids,omitempty,brackets"`
 	TeamIDs  []string `url:"team_ids,omitempty,brackets"`

extension.go

@@ -28,7 +28,25 @@ type ListExtensionResponse struct {
 
 // ListExtensionOptions are the options to use when listing extensions.
 type ListExtensionOptions struct {
-	APIListObject
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
+
 	ExtensionObjectID string `url:"extension_object_id,omitempty"`
 	ExtensionSchemaID string `url:"extension_schema_id,omitempty"`
 	Query             string `url:"query,omitempty"`

extension_schema.go

@@ -32,8 +32,24 @@ type ListExtensionSchemaResponse struct {
 // ListExtensionSchemaOptions are the options to send with the
 // ListExtensionSchema reques(s).
 type ListExtensionSchemaOptions struct {
-	APIListObject
-	Query string `url:"query,omitempty"`
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
 }
 
 // ListExtensionSchemas lists all of the extension schemas. Each schema

extension_schema_test.go

@@ -17,8 +17,8 @@ func TestExtensionSchema_List(t *testing.T) {
 	listObj := APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
 	client := defaultTestClient(server.URL, "foo")
 	opts := ListExtensionSchemaOptions{
-		APIListObject: listObj,
-		Query:         "foo",
+		Limit:  listObj.Limit,
+		Offset: listObj.Offset,
 	}
 
 	res, err := client.ListExtensionSchemas(opts)

extension_test.go

@@ -18,8 +18,9 @@ func TestExtension_List(t *testing.T) {
 	listObj := APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
 	client := defaultTestClient(server.URL, "foo")
 	opts := ListExtensionOptions{
-		APIListObject: listObj,
-		Query:         "foo",
+		Limit:  listObj.Limit,
+		Offset: listObj.Offset,
+		Query:  "foo",
 	}
 
 	res, err := client.ListExtensions(opts)

incident.go

@@ -97,7 +97,25 @@ type ListIncidentsResponse struct {
 
 // ListIncidentsOptions is the structure used when passing parameters to the ListIncident API endpoint.
 type ListIncidentsOptions struct {
-	APIListObject
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
+
 	Since       string   `url:"since,omitempty"`
 	Until       string   `url:"until,omitempty"`
 	DateRange   string   `url:"date_range,omitempty"`
@@ -406,7 +424,25 @@ type ListAlertsResponse struct {
 
 // ListIncidentAlertsOptions is the structure used when passing parameters to the ListIncidentAlerts API endpoint.
 type ListIncidentAlertsOptions struct {
-	APIListObject
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
+
 	Statuses []string `url:"statuses,omitempty,brackets"`
 	SortBy   string   `url:"sort_by,omitempty"`
 	Includes []string `url:"include,omitempty,brackets"`
@@ -534,7 +570,25 @@ type ListIncidentLogEntriesResponse struct {
 
 // ListIncidentLogEntriesOptions is the structure used when passing parameters to the ListIncidentLogEntries API endpoint.
 type ListIncidentLogEntriesOptions struct {
-	APIListObject
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
+
 	Includes   []string `url:"include,omitempty,brackets"`
 	IsOverview bool     `url:"is_overview,omitempty"`
 	TimeZone   string   `url:"time_zone,omitempty"`

incident_test.go

@@ -583,8 +583,9 @@ func TestIncident_ListIncidentAlertsWithOpts(t *testing.T) {
 	id := "1"
 
 	alertOpts := ListIncidentAlertsOptions{
-		APIListObject: listObj,
-		Includes:      []string{},
+		Limit:    listObj.Limit,
+		Offset:   listObj.Offset,
+		Includes: []string{},
 	}
 
 	res, err := client.ListIncidentAlertsWithOpts(id, alertOpts)
@@ -721,10 +722,11 @@ func TestIncident_ListLogEntries(t *testing.T) {
 	client := defaultTestClient(server.URL, "foo")
 	id := "1"
 	entriesOpts := ListIncidentLogEntriesOptions{
-		APIListObject: listObj,
-		Includes:      []string{},
-		IsOverview:    true,
-		TimeZone:      "UTC",
+		Limit:      listObj.Limit,
+		Offset:     listObj.Offset,
+		Includes:   []string{},
+		IsOverview: true,
+		TimeZone:   "UTC",
 	}
 	res, err := client.ListIncidentLogEntries(id, entriesOpts)
 
@@ -761,12 +763,13 @@ func TestIncident_ListLogEntriesSinceUntil(t *testing.T) {
 	client := defaultTestClient(server.URL, "foo")
 	id := "1"
 	entriesOpts := ListIncidentLogEntriesOptions{
-		APIListObject: listObj,
-		Includes:      []string{},
-		IsOverview:    true,
-		TimeZone:      "UTC",
-		Since:         "2020-03-27T22:40:00-0700",
-		Until:         "2020-03-28T22:50:00-0700",
+		Limit:      listObj.Limit,
+		Offset:     listObj.Offset,
+		Includes:   []string{},
+		IsOverview: true,
+		TimeZone:   "UTC",
+		Since:      "2020-03-27T22:40:00-0700",
+		Until:      "2020-03-28T22:50:00-0700",
 	}
 	res, err := client.ListIncidentLogEntries(id, entriesOpts)
 

log_entry.go

@@ -55,7 +55,25 @@ type ListLogEntryResponse struct {
 
 // ListLogEntriesOptions is the data structure used when calling the ListLogEntry API endpoint.
 type ListLogEntriesOptions struct {
-	APIListObject
+	// Limit is the pagination parameter that limits the number of results per
+	// page. PagerDuty defaults this value to 25 if omitted, and sets an upper
+	// bound of 100.
+	Limit uint `url:"limit,omitempty"`
+
+	// Offset is the pagination parameter that specifies the offset at which to
+	// start pagination results. When trying to request the next page of
+	// results, the new Offset value should be currentOffset + Limit.
+	Offset uint `url:"offset,omitempty"`
+
+	// Total is the pagination parameter to request that the API return the
+	// total count of items in the response. If this field is omitted or set to
+	// false, the total number of results will not be sent back from the PagerDuty API.
+	//
+	// Setting this to true will slow down the API response times, and so it's
+	// recommended to omit it unless you've a specific reason for wanting the
+	// total count of items in the collection.
+	Total bool `url:"total,omitempty"`
+
 	TimeZone   string   `url:"time_zone,omitempty"`
 	Since      string   `url:"since,omitempty"`
 	Until      string   `url:"until,omitempty"`

log_entry_test.go

@@ -18,10 +18,11 @@ func TestLogEntry_List(t *testing.T) {
 	listObj := APIListObject{Limit: 0, Offset: 0, More: false, Total: 0}
 	client := defaultTestClient(server.URL, "foo")
 	entriesOpts := ListLogEntriesOptions{
-		APIListObject: listObj,
-		Includes:      []string{},
-		IsOverview:    true,
-		TimeZone:      "UTC",
+		Limit:      listObj.Limit,
+		Offset:     listObj.Offset,
+		Includes:   []string{},
+		IsOverview: true,
+		TimeZone:   "UTC",
 	}
 	res, err := client.ListLogEntries(entriesOpts)