Stubbed next_token inconsistency

[ryanbrainard]

Spun off from https://github.com/aws/aws-sdk-ruby/issues/1408

When stubbing responses, the default value of the next_token is inconsistent. Sometimes it is nil, sometimes it is "String", and sometimes it is "NextToken". See the example below on the EC2 client:

pry(main)> client = Aws::EC2::Client.new(stub_responses: true)
pry(main)> client.methods.
  select { |m| m.to_s.start_with?('describe_') }.
  map { |m| client.send(m).to_s rescue '' }.
  select { |o| o.include?('next_token') }.
  each { |o| puts o }
#<struct Aws::EC2::Types::DescribeClassicLinkInstancesResult instances=[], next_token="String">
#<struct Aws::EC2::Types::DescribeEgressOnlyInternetGatewaysResult egress_only_internet_gateways=[], next_token="String">
#<struct Aws::EC2::Types::DescribeFlowLogsResult flow_logs=[], next_token="String">
#<struct Aws::EC2::Types::DescribeHostReservationOfferingsResult offering_set=[], next_token="String">
#<struct Aws::EC2::Types::DescribeHostReservationsResult host_reservation_set=[], next_token="String">
#<struct Aws::EC2::Types::DescribeHostsResult hosts=[], next_token="String">
#<struct Aws::EC2::Types::DescribeImportImageTasksResult import_image_tasks=[], next_token="String">
#<struct Aws::EC2::Types::DescribeImportSnapshotTasksResult import_snapshot_tasks=[], next_token="String">
#<struct Aws::EC2::Types::DescribeInstanceStatusResult instance_statuses=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeInstancesResult reservations=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeMovingAddressesResult moving_address_statuses=[], next_token="String">
#<struct Aws::EC2::Types::DescribeNatGatewaysResult nat_gateways=[], next_token="String">
#<struct Aws::EC2::Types::DescribePrefixListsResult prefix_lists=[], next_token="String">
#<struct Aws::EC2::Types::DescribeReservedInstancesModificationsResult reserved_instances_modifications=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeReservedInstancesOfferingsResult reserved_instances_offerings=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeScheduledInstancesResult next_token="String", scheduled_instance_set=[]>
#<struct Aws::EC2::Types::DescribeSnapshotsResult snapshots=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeSpotFleetRequestsResponse spot_fleet_request_configs=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeSpotPriceHistoryResult spot_price_history=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeTagsResult tags=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeVolumeStatusResult volume_statuses=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeVolumesResult volumes=[], next_token=nil>
#<struct Aws::EC2::Types::DescribeVpcClassicLinkDnsSupportResult vpcs=[], next_token="NextToken">
#<struct Aws::EC2::Types::DescribeVpcEndpointServicesResult service_names=[], next_token="String">
#<struct Aws::EC2::Types::DescribeVpcEndpointsResult vpc_endpoints=[], next_token="String">

The "String" vs "NextToken" distinction is immaterial, but the fact that some of them are nil means some resource types page (indefinitely) by default and some do not. I know that the stub_responses can be overridden with a hash (and that's how I've worked around it), but it is unfortunate that stub_responses: true doesn't work consistently one way or another.

If I could choose, I think that the next_token should be nil by default. This reason is that if you aren't explicitly testing pagination, it just works. You get one page and you're done. If you are testing pagination, you'll need to override with a hash either way to 1) force the next page and then 2) break out of the loop, and I would much rather only have to override when I'm explicitly testing pagination. For example:

describe_nat_gateways: [
  {
    nat_gateways: [
      { nat_gateway_id: 'nat-1' },
    ],
    next_token: 'token'
  },
  {
    nat_gateways: [
      { nat_gateway_id:'nat-2' },
    ]
  }
]

[awood45]

Interestingly, this appears to be related to whether or not a paginator is defined for a method. If a paginator is defined, we're populating nil, otherwise a stub value. I'm looking in to why this is.

[ryanbrainard]

@awood45 Thanks for looking into this. So it does sound like it's related to #1408. If so, does that mean that there are a lot of resources that need paginators defined?

[awood45]

That's very possible, though the inconsistency in stub behavior is interesting regardless.

[cjyclaire]

Some updates, pagination has been enforced from root for future new APIs. Yet for those existing APIs missing paginations, we might still need to do manual works to fix those.

Free feel to open new issue if any pagination behavior missing causes unpleasant experience for you, we will fix those. Apology for those inconsistencies so far. Closing, feel free to reopen with further comments or questions. Thanks!