Modernize the gem and add the V2 client (2.0.0) - #18
Merged
Merged
Conversation
CI moves to GitHub Actions in a later commit. The .gitmodules listed a typo'd path and a submodule that was never registered.
MIT license, real author email, and a Ruby 3.3 floor (nothing older runs in production). Adds RuboCop as a dev dependency.
spec_helper now requires webmock/rspec and timecop once and returns Timecop after every example (time stayed frozen at 2018-09-01 for the rest of the suite). Examples that mutate the thread-local default config or Aws.config now reset them, which random-order runs proved necessary.
Pins current behavior before the modernization refactors: Configuration region/endpoint/host logic, the RaiseError status-to-error map and message formats, ApiClient request building and deserialization, Session auth and RDT caching (incl. grantless), TokenExchangeAuth, the module helpers, ApiResponse/ApiError, and the 14 used sp_* require paths with their API class names. The token_exchange_auth spec also fixes a bug where a Configuration was passed as the refresh_token argument, so it tested the wrong config. Known bugs from the modernization plan stay unpinned on purpose; their fixes start with failing tests. Suite grows from 21 to 191 examples and passes in random order.
NewCops enabled; vendor/, generated sp_* shims, and models repo excluded; Metrics off for now. Remaining offenses live in .rubocop_todo.yml to shrink over time. Only safe (semantics-preserving) corrections were applied to the handwritten code, verified by the characterization suite. This also drops a dangling 'private' in uploader.rb.
RSpec on Ruby 3.3 and 3.4, plus a RuboCop job.
Regeneration needed Java plus a working swagger-codegen install, and the Rakefile was broken in three ways (missing amzn-models checkout, undeclared active_support, undocumented fd). The new generator under lib/generator is plain Ruby + ERB with no extra dependencies, modeled on the peddler gem's generator. Reproducibility: the generator clones Amazon's official spec repo into amzn-models/ (gitignored) and checks out the exact revision recorded in selling-partner-api-models.sha. `rake generate` regenerates at that pin; `rake generate:update` is the only path that advances it. The generator emits only what actually loads: the API classes, the vendor entry files, and the sp_*.rb require shims. It reproduces the old swagger-codegen output byte for byte (except the header comment), including its quirks, so the regeneration diff stays reviewable. codegen-config.yml now lists only the 14 modules with real consumers; the 17 unused vendored modules stay untouched until the planned dead-code removal. The mustache templates and the old codegen rake tasks are gone. The generator is excluded from the shipped gem.
Output of `bundle exec rake generate` at the pinned spec revision (3822f16, 2025-02-26, the newest commit before the previous regeneration date). The API classes are unchanged except the header comment. The vendor entry files lose the commented-out model requires, and the shims lose a trailing space and gain a final newline.
CI now runs `rake generate:verify`: it regenerates at the pinned spec revision and fails when the committed files drift from the generator output. A nightly workflow runs `rake generate:update` and opens a PR (via a machine-user PAT, so the PR triggers CI) whenever Amazon changes the specs.
No consumer requires these modules. A sweep of every Dropstream repo found only active_cart and active_fulfillment using the gem in code, and together they use 14 modules — the ones in codegen-config.yml. Removes the vendor/ directories and their sp_*.rb require shims for: catalog_items_2020, catalog_items_v0, fba_inbound, fulfillment_inbound_v0, merchant_fulfillment_v0, messaging, notifications, product_fees_v0, product_pricing_v0, sales, sellers, services, shipping, solicitations, uploads_2020, vdf_payments_v1, vdf_transactions_v1.
The old swagger-codegen emitted a full standalone gem per module. Only the entry file and the API classes were ever loaded; the new generator emits exactly those. Delete the rest: models/ (1,037 files with zero requires), per-vendor api_client/configuration/api_error/ version (62 of them require the undeclared typhoeus gem), and the gem scaffolding (README, Rakefile, Gemfile, gemspec, git_push.sh, .rspec, .rubocop.yml, .gitignore, .swagger-codegen*). Each vendor module now holds only lib/sp_<name>.rb and lib/sp_<name>/api/*.rb — what `rake generate` writes.
None of this was reachable: - ServiceError: never raised; the middleware raises Faraday errors. - Configuration: basic_auth_token (calls undefined username/password), the host= writer (the host reader recomputes from region, so writes never surfaced), and attrs only the old typhoeus client read: temp_folder_path, verify_ssl, verify_ssl_host, ssl_ca_cert, cert_file, key_file, params_encoding, inject_format, force_ending_format, plus the dead @refresh_token ivar. - ApiClient: typo'd @api_req_otps, the typhoeus-era req_opts keys that call_api never passes to Faraday, the commented-out update_params_for_auth! and download_file remains, and the 'File' return path reading a never-assigned @tempfile. - RaiseError: :sts in VALID_SERVICE — no caller ever passes it (STS goes through aws-sdk, not this middleware). Pinned by a new spec.
Amazon dropped the SigV4 requirement for SP-API in October 2023; only the LWA access token is needed. This deletes the SigV4 middleware, the eager synchronous AssumeRole call that every new_session made for nothing, and the aws-sdk-core / aws-sigv4 dependencies. The Configuration attrs consumers still write (access_key, secret_key, role_arn, credentials_provider) stay as accepted-but- ignored, marked deprecated, so 1.8.0 is a drop-in upgrade. They go away in 2.0. Behavior changes: - new_session no longer makes an STS network call. - STS failures can no longer surface as laundered Faraday::ForbiddenError. - Session#role_credentials and Session#credentials_provider are gone; no consumer reads them (verified in active_cart, active_fulfillment, beagle_event_logger). Also: raise_error.rb and configuration.rb now require faraday and logger themselves (aws-sdk-core and the signer file used to load them first), and webmock is bumped because Ruby 3.4 no longer ships base64 — it previously arrived via aws-sdk-core.
Session#refresh tested @grantles, a misspelling of @grantless that is never assigned, so the branch never ran. An expired grantless session fell through to the refresh-token branch, which is also a no-op there (client_credentials responses carry no refresh token), and kept using the stale access token until Amazon returned 403.
call_api passed query params as the second positional argument for every verb. Faraday's get/delete accept (url, params), but post/put/ patch accept (url, body) — the params landed in the body slot and were then overwritten by the real body. Every POST/PUT/PATCH with query params silently lost them: 21 generated operations, including put_listings_item and patch_listings_item, whose required marketplaceIds is a query param. Set params on the request object instead, which works for all verbs.
build_request merged the generated APIs' header_params (per-op Accept, Content-Type) into req_opts[:headers], but call_api never read it — only the connection defaults and the x-amz pair reached the wire. Harmless for Content-Type (every operation happens to use application/json, same as the default) but the Accept header was never sent at all, including application/hal+json where specs ask for it. Merge req_opts[:headers] into the request before the x-amz pair.
RaiseError copied the full request headers and body into Faraday::Error#response, which consumers log. That exposed the LWA or restricted data token (x-amz-access-token), SigV4-era authorization and x-amz-security-token headers, and — for the token service — client_secret and refresh_token in the form body. Replace those header values with [FILTERED] and filter the whole request body for the token service. Response details and non-token request bodies are unchanged; consumers only read e.message.
config.endpoint = 'zz' used to corrupt @region to nil and then die with KeyError 'key not found: nil' from deep inside region=. Now set_region_by_endpoint raises ArgumentError naming the bad value before touching any state.
The envelope guard accepted string 'payload'/'errors' keys but the branch read only symbol keys, so a string-keyed hash matched the guard and came out as a nil payload. Envelope keys are always symbols in practice (deserialize symbolizes), so drop the string checks: a string-keyed hash now lands intact in payload like any other unrecognized shape.
MARKETPLACE_ENDPOINT_MAP and set_region_by_endpoint both know the Saudi marketplace (A17E79C6D8DWNP, 'sa'), but MARKETPLACE_IDS was missing the key. A spec now keeps the two constants in sync.
Only ApiClient honored config.timeout. TokenExchangeAuth, Uploader, Downloader, and download_report_document built Faraday connections with no timeout at all, so a hung LWA or S3 endpoint blocked the caller forever. All connections now pass the configured timeout (default 60s).
Expiry was stored as a strftime string and re-parsed with strptime on every check — needless work, and a wrong-zone parse would shift the comparison silently. Store the Time directly and drop the String branch. Also renames the consistently misspelled @restricted_data_token_expirest_at ivar.
Changelog gains the 1.8.0 section (deletions, bug fixes, deprecated AWS attrs). README drops the AWS IAM settings from the usage example and explains the deprecation. Note for consumers: active_cart and active_fulfillment pin '~> 1.7.0' in their gemspecs and must bump it to pick this release up.
The version bump commit changed the gemspec without re-locking, so CI's frozen-mode bundle install failed with 'the gemspecs for path gems changed'. Re-locking also floats multi_xml to 0.9.1 (its bigdecimal bound widened); the suite is green with it.
Follow-ups to the redaction commit, from review: - Redact on a Faraday::Utils::Headers copy instead of converting to a plain Hash, so header lookup in the error payload stays case-insensitive as before. - Strip the query string from uploads-service urls in the payload. Those are presigned S3 urls whose query string is a live, time-limited credential; it leaked into consumer logs on any S3 error. The path stays for debugging; spapi urls keep their query. - Pin that non-sensitive request headers survive redaction.
set_endpoint_by_marketplace_id assigned @marketplace_id before the fetch that validates it, so an unknown id raised KeyError but left the config holding the bogus id. Look the endpoint up first.
active_cart and active_fulfillment set access_key, secret_key, role_arn, and credentials_provider unconditionally at boot. Nothing in the suite exercised the writers after the SigV4 removal, so a future cleanup could delete them with a green build and break every consumer. This spec keeps them writable and inert until 2.0.
The gem carried its CI config, Rakefile, bin/ scripts, lockfile, dotfiles and the generator config. Consumers need none of it. The built gem now holds lib/, vendor/, the gemspec and the top-level docs.
The gemspec had no allowed_push_host, so `rake release` would have pushed this public repo's gem to rubygems.org. Set the Dropstream registry as the only push host, name the repository the package belongs to, and fill in the source and changelog links. Disable the bundler `release` task. Releases run from a version tag in CI, not from a laptop. A spec pins the push host and the list of shipped files.
A push of a v* tag runs the suite, builds the gem, pushes it to the Dropstream registry on GitHub Packages with the workflow token, and opens a GitHub Release with the CHANGELOG entry for that version. The job stops when the tag does not match AmazonSpClients::VERSION or when CHANGELOG.md has no entry for it, so a wrong tag publishes nothing. GitHub Packages rejects a version it already has, so a failed release needs a new version, not a re-run.
README: install from the GitHub Packages source block, pin a tag if a git source stays, and the release steps (version, re-lock, changelog date, annotated tag). CLAUDE.md: the release path, and why master must not take 2.0.0 before consumers move off git sources.
JSON.generate calls to_json with a State, and ActiveSupport's to_json
then defers to the stdlib. So a Time in a request body went out as
Time#to_s ("2026-08-31 15:50:50 UTC") instead of ISO 8601. Amazon
rejects that in createReport's dataStartTime, which active_cart's
returns import sends as a TimeWithZone. v1 called to_json; V2 does
again, so a consumer's encoders apply.
An LWA 5xx dropped its error and error_description, so the message
read "503 (no error details)" and a consumer's retry rule on the text
could not match. A 4xx from API Gateway carries {"message": ...}
instead of an errors array and read the same way. Both now show the
text the server sent.
A string-keyed or url-less Hash raised KeyError from deep inside the helper. It is a caller error: the payload must be ApiResponse#payload, with symbol keys. Raise ArgumentError saying so, with the keys given.
Every consumer that downloads a restricted report builds the same GET /reports/2021-06-30/documents/<id> resource by hand, path literal included. RDT.report_document(id) returns it. A spec checks the path against the one the generated getReportDocument requests, so the preset cannot drift when the module moves.
Rows the first V2 consumer needed and the table lacked: transport failures that fell through v1's last rescue are ConnectionError now, the message text changed and is not a contract, an empty array is still sent as an empty value, and upload_feed_document returns nil.
The gem goes public. The tag workflow now pushes to rubygems.org through Trusted Publishing: the job's OIDC token is traded for a short-lived key, so no API key is stored anywhere. rubygems.org must trust the workflow first; README, "Releasing", has the one-time setup. Consumers install a version from rubygems.org instead of a git branch. The push host in the gemspec changes with it; the MFA rule stays.
The README opens with the status: large changes in progress, interfaces may move between releases, and peddler is the recommended client for production use. The gemspec summary and description say the same, so the rubygems.org page does too.
The gem goes public on rubygems.org, and "amazon" is Amazon's name, not ours. Only the gem name changes: the AmazonSpClients namespace, the require paths, vendor/ and the generator stay as they are, so a consumer changes one Gemfile line. lib/sp_api_clients.rb makes Bundler's automatic require work for the new name. The V2 default User-Agent names the gem, so it follows. The lockfiles, the release workflow and the rubocop todo follow the gemspec file name.
The install line, the rubygems.org link and the trusted publisher form use the new gem name. The repository keeps its old name, so the form asks for both. The README states that the project is not affiliated with Amazon.
examples/auth_check.rb runs live checks with nothing but the public gem: a made-up token is rejected, a real access token is accepted without any AWS signature, and a refresh needs the LWA client id and secret. It prints what each result means, so someone who does not know the gem can settle the question "what do we need in production" in a minute. Credentials come from environment variables only. The endpoint defaults to na and the marketplace to amazon.com. Loaded into a process that already runs under Bundler, the script uses the gem when that bundle has it, falls back to Net::HTTP with the same request otherwise, and never calls exit. Run from a shell it installs the public gem. examples/ stays out of the built gem.
The repository and the gem are public. Comments named the applications that consume the gem; the facts hold for any consumer.
nina-saule
force-pushed
the
version-upgrade
branch
from
September 7, 2026 15:59
b614180 to
8e00882
Compare
rake release:prepare[X.Y.Z] refuses a dirty tree, a bad version, an existing tag or a missing CHANGELOG entry, then runs the suite, sets the version, dates the CHANGELOG heading, relocks the three lockfiles, commits and creates the annotated tag. It stops before the push: the tag push is the publish, and rubygems.org never accepts a version number twice, so the suite runs before anything is tagged. tasks/ stays out of the built gem.
README, "Releasing" lists the three steps a release takes now; CLAUDE.md gets the command.
The Orders API v2026-01-01 model exists only upstream, so the pin moves from 3822f16 (2025-02-26) to 3659f96 (2026-08-26) before that module can be added. Regeneration changes 16 files, all of it description text except one new optional query parameter, enableContentEncodingUrlHeader, on getFeedDocument and getReportDocument. Upstream now marks every Orders v0 operation as deprecated.
Every generated V2 method takes rdt: because the specs do not say which operations are restricted. The Orders API v2026-01-01 never uses restricted data tokens, so advertising the keyword there would be misleading. A module sets `rdt: false` in codegen-config.yml to drop the keyword, its doc line and the request argument. The default is unchanged, and regeneration of the 14 modules is byte-identical.
Amazon's next Orders API replaces the seven v0 read operations with searchOrders and getOrder, selects PII and other blocks with includedData instead of restricted data tokens, pages with paginationToken, and returns the body without a payload wrapper. The module is V2 only: `client.orders_2026` gives `Orders2026` with `search_orders` (every parameter optional) and `get_order`. Neither takes rdt:. `ApiResponse#payload` is the whole body, so consumers read `payload[:orders]` and `payload.dig(:pagination, :nextToken)`. Orders v0 stays in both v1 and V2; Amazon marks it deprecated.
README gains an "Orders API v2026-01-01" section with what a caller has to know: two operations, includedData instead of separate PII calls, no restricted data token, paginationToken with a 24 hour life, renamed status values and the lower default rate. The V2 example and the migration table use the new module; the code generation section explains `rdt: false`. CHANGELOG gets the undated 2.1.0 entry for the release task to stamp.
nina-saule
marked this pull request as ready for review
September 8, 2026 10:49
wkrsz
approved these changes
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR: this branch replaces the Java code generator with plain Ruby, deletes dead code, makes v1 run on Faraday 1.10 and 2, adds
AmazonSpClients::V2, a per-merchant client with typed errors, and renames the gem tosp_api_clientsfor rubygems.org. The v1 API and theAmazonSpClientsnamespace are unchanged. Do not merge yet. Every consumer installs this gem from git master, so the merge is the release for them. The merge order is below.What is in it
rake generate:verifyin CI, 17 unused API modules deleted, SigV4 signing removed (Amazon dropped it in 2023), bug fixes.CHANGELOG.mdhas the full list.faraday_middlewaredropped, the HTTPClient adapter kept, CI matrix on both Faraday majors.V2namespace (require 'amazon_sp_clients/v2'), a release workflow that publishes to rubygems.org from av*tag, and the docs. The README opens with the experimental status and points to peddler; it has a v1 to V2 migration table and a "Releasing" section.sp_api_clients. The old name started with Amazon's name, which is not ours to publish under. Only the gem name changed: theAmazonSpClientsnamespace, theamazon_sp_clients/...require paths,vendor/and the generator are as before.lib/sp_api_clients.rbis a one-line shim sogem 'sp_api_clients'works with Bundler's automatic require. The repository keeps its name.None of these versions is released yet. Only 2.0.0 will be. It contains the other two.
How to review
CHANGELOG.md, then the README sections "V2 client", "Migrating from v1" and "Releasing".amazon_spartner_experimentalto V2 (separate PR). Its test file passes against this branch. A review of that cart against the gem produced several commits here: request bodies throughto_json(ActiveSupport time encoding), fuller LWA and gateway error messages, an RDT preset for report documents, a clearer error on a bad document payload, and four migration-table rows.yard:verifyandgenerate:verify.Merge order
sp_api_clientsand repositorydropstream/amazon_sp_clients(README, "Releasing"), then tagv2.0.0on this branch. The Release workflow publishes the gem.amazon_sp_clientstosp_api_clients,'~> 2.0', and release. For active_fulfillment this is a one-line change with no code change: 2.0.0 still ships the whole v1 API under the same constant and require paths.gem 'amazon_sp_clients', git: ...line and take both releases from step 2. Thenbundle list | grep amazon_sp_clientsmust print nothing. beagle_catalog also drops itstag: 'v1.7.0'.Why one commit in step 3: Bundler sees
amazon_sp_clients(git, 1.7.0) andsp_api_clients(2.0.0) as two different gems and installs both side by side. Both shiplib/amazon_sp_clients.rb, so load path order decides which one arequirepicks. RubyGems has no way to declare a conflict. With one gem name, a version mismatch failed loudly atbundle install; that safety net is gone, and the grep replaces it.