feat(marketplace-agreement): emulate AWS Marketplace Agreement API - #3339
Conversation
|
🎉 Thanks for your first pull request to Floci! Your CI checks need a maintainer to approve them before they run. That is GitHub's standard gate on first-time contributors, not a problem with your PR — so if the checks look like they are doing nothing, that is why. Once a maintainer approves, CI and the compatibility suite start automatically. Nothing is needed from you in the meantime. While you wait, a couple of things that make review faster:
Come join us in Slack — it is the fastest way to reach maintainers if you get stuck, or want feedback on an approach before investing more time in it. |
|
| Filename | Overview |
|---|---|
| src/main/java/io/github/hectorvent/floci/services/marketplace/MarketplaceAgreementService.java | Implements account-scoped Marketplace Agreement operations, including the corrections requested by the previous review. |
| src/main/java/io/github/hectorvent/floci/services/marketplace/MarketplaceJsonHandler.java | Dispatches Marketplace Agreement JSON actions through the service-specific controller. |
| src/main/java/io/github/hectorvent/floci/services/marketplace/MarketplaceAgreementController.java | Provides the required thin controller layer for Agreement operations. |
| src/main/java/io/github/hectorvent/floci/core/common/ResolvedServiceCatalog.java | Registers both REST JSON and JSON Marketplace protocols with distinct routing signals. |
| src/test/java/io/github/hectorvent/floci/services/marketplace/MarketplaceAgreementServiceTest.java | Covers focused agreement validation, filtering, ordering, resources, and lifecycle behavior. |
| src/test/java/io/github/hectorvent/floci/services/marketplace/MarketplaceAgreementControllerIntegrationTest.java | Verifies Agreement protocol routing and operation behavior through the HTTP layer. |
Sequence Diagram
sequenceDiagram
participant SDK as AWS SDK
participant JSON as AwsJsonController
participant Handler as MarketplaceJsonHandler
participant Controller as MarketplaceAgreementController
participant Service as MarketplaceAgreementService
participant Storage as Account-aware Storage
SDK->>JSON: POST / + X-Amz-Target
JSON->>Handler: action, body, region
Handler->>Controller: handle(action, request, region)
Controller->>Service: handle(action, request, region)
Service->>Storage: Read or mutate account-scoped records
Storage-->>Service: Agreement state
Service-->>SDK: AWS-compatible JSON response
Reviews (6): Last reviewed commit: "feat(marketplace-agreement): emulate AWS..." | Re-trigger Greptile
|
👋 Thanks for the pull request! It looks like you now have 10 open pull requests in this repository, which is above our advisory limit of 5. To keep the review backlog focused and make sure every contribution gets proper attention, we ask contributors to shepherd their existing PRs to the finish line before opening new ones — responding to review feedback, rebasing, or closing any that are no longer needed. Here is your list of open PRs. Nothing is blocked and this PR stays open — this is just a friendly nudge. The If some of these PRs are stacked or intentionally related, just say so here and a maintainer will take that into account. |
264c9fe to
2391e46
Compare
pgermosen
left a comment
There was a problem hiding this comment.
Nice work getting this split out cleanly. A few real gaps in SearchAgreements against the documented API: AgreementType is required per AWS's own docs (any combination of the other filters is allowed as long as it's included), but this treats the whole filter list as optional and returns everything when none are given. Your own test calls it with an empty body and expects success, which real AWS would reject.
Several other documented filters, BeforeStartTime, AfterStartTime, BeforeLastUpdateTime, AfterLastUpdateTime, EndTimeBehaviorType, EndTimeBehaviorReasonCode, InitialAgreementId, and LicenseArn, aren't handled at all, so a client filtering on any of them silently gets back zero results instead of the right set or a clear error. ProposerAccountId isn't a real filter either, only AcceptorAccountId is documented. And the sort restriction, StartTime/LastUpdateTime only valid when PartyType is Proposer, isn't enforced.
Separately, this is the first service doc in the recent run that doesn't have a Known deviations section, and there are a couple worth naming: the flat 365-day agreement duration regardless of what's actually in requestedTerms, and the PartyType filter that validates its value but doesn't otherwise change which agreements match.
Also worth rebasing onto current main before the next round.
b9e7436 to
a58fe65
Compare
a58fe65 to
b89c265
Compare
pgermosen
left a comment
There was a problem hiding this comment.
All four SearchAgreements gaps from the last round are fixed and confirmed against the docs: AgreementType required, the eight missing filters implemented, ProposerAccountId gone, and the sort-by/PartyType restriction enforced. The Known deviations section now names exactly the two real simplifications. And the descriptor collision with #3338 is resolved cleanly, one combined descriptor carrying both protocols and controllers, matching the sso/ssoadmin pattern already in the file. Approving.
Summary
Adds AWS Marketplace Agreement API emulation as an independent Marketplace API-family contribution. Includes the JSON protocol handler, agreement lifecycle/search operations, account-aware persistence, integration coverage, docs, and an AWS SDK compatibility smoke test.
This is one of the API-family splits of the previous combined Marketplace contribution (#3303).
Type of change
fix:)feat:)feat!:orfix!:)AWS Compatibility
Reviewed against the current AWS Marketplace Agreement Service API reference. Senior review corrected the SearchAgreements resource filter name to
ResourceIdentifier, matching AWS.make docs-checkandgit diff --checkpass. Heavy local Maven execution was intentionally not run on this machine; CI is expected to execute the full test suite.Checklist
./mvnw testpasses locally