Conversation
NitinKumar004
left a comment
There was a problem hiding this comment.
Review notes
Real data-plane engine (per #427): N/A.
Findings
Medium · docs — docs/services.md not updated with OCI Notifications — unmet Definition-of-done checkbox
docs/services.md:23
If a user consults docs/services.md (the human-facing service catalog) to learn what OCI surface CloudEmu emulates -> they find no ONS topics/subscriptions/publish entry and conclude the service is unimplemented, because the vertical slice's documentation step was skipped even though the code and generated coverage exist.
git diff merge-base..HEAD shows docs/services.md untouched; only auto-generated docs/coverage/* changed. oci-conventions.md 'Definition of done' requires 'Operations added to docs/services.md', and sibling OCI services each have a hand-written section (OCI Monitoring line 623, OCI identity line 749, OCI VCN note line 407). Notifications has none.
Low · structure — Per-feature filename not mirrored across provider/wire layers for topics and publish
providers/oci/notifications/publish.go:1
If a maintainer greps for the publish feature by filename (publish.go) -> they find only the provider side and miss that the wire implementation lives in topics.go, because the feature file name is not mirrored on the wire layer; a navigation cost, not a behavioral defect.
STRUCTURE.md §3: 'A feature uses the same filename across all three layers.' subscriptions.go matches (provider+wire), but topic CRUD is provider notifications.go vs wire topics.go, and publish is provider publish.go vs wire (folded into topics.go). provider notifications.go is defensible as the §4 .go core-CRUD file, but publish.go/topics.go breaks the one-grep goal.
NitinKumar004
left a comment
There was a problem hiding this comment.
Review — OCI Notifications
Verdict: Request changes — one blocker (persistence completeness), otherwise clean and faithful to the existing OCI convention.
Blocking
Mock is not Snapshottable — providers/oci/notifications/notifications.go (Mock).
The Mock holds three memstore.Store fields (topics, subscriptions, deliveries) but implements no snapshot.Snapshottable, and there's no snapshot.go. Every sibling OCI provider ships one (providers/oci/{identity,vcn,monitoring}/snapshot.go). On a tree that includes the persistence-completeness guard, go test ./persist/... fails:
oci: field Notifications (*notifications.Mock) holds a memstore.Store but is not
Snapshottable — add persistence (see #582) or justify in guardExclude
Consequence: CI go test ./... goes red, and a serve stop/start silently drops every topic and subscription.
Fix: add providers/oci/notifications/snapshot.go with var _ snapshot.Snapshottable = (*Mock)(nil) plus Snapshot/Restore over topics + subscriptions (decide whether the transient deliveries store persists or is justified in guardExclude). The branch is also behind development, so rebase + re-run go generate before merge.
Non-blocking
- Create returns
200, not201—server/oci/notifications/topics.go(CreateTopic) andsubscriptions.go(CreateSubscription). Real ONS returns201 Created. SDK/Terraform accept any 2xx, but a strict raw-HTTP client asserting 201 diverges. if-match/etag not enforced —server/oci/notifications/topics.go(update/delete). The etag is minted, stored, and returned correctly, but the handler ignores theif-matchheader, so a stale-etag update never returns412.- No endpoint-format / message-size validation —
providers/oci/notifications/subscriptions.go(endpoint) andpublish.go(body). EMAIL endpoints aren't checked for@, CUSTOM_HTTPS endpoints aren't required to be https, and the message body has no 64 KB cap; real ONS400s these.
Verified good
Full lifecycle exercised over the wire (create topic → subscribe → confirm → publish → delete with cascade): computed IDs/timeCreated stable across reads, etag rotates on update, dup→409, missing→404, subscription-under-missing-topic→404, bad protocol→400, delete returns Opc-Work-Request-Id, subscription cascade-deleted with its topic. Wiring is complete end-to-end (provider field + New() + monitoring wire + registration + from_provider.go + work-request store). No check-then-set or shared-pointer aliasing races (-race clean; every Get/List deep-copies). Structure, lint, and go generate idempotency all pass.
b22f8b3 to
344b484
Compare
|
Rebased onto Blocking —
|
| package | before | after |
|---|---|---|
providers/oci/notifications |
95.8% | 99.0% |
server/oci/notifications |
78.6% | 96.2% |
Verification
go build ./... clean
go test ./... exit 0
go test ./persist/... ok (the blocker)
go test -race (both notifications packages) 2/2 ok
golangci-lint (both packages) 0 issues
go generate ./... re-running produces no diff
no OCI op in docs/coverage/{aws,azure,gcp}/*.md confirmed
Two lint findings surfaced and were fixed rather than waived: canonicalheader on my if-match (to If-Match; Header.Get is case-insensitive so wire behaviour is unchanged) and gocritic hugeParam on Publish, given the same //nolint the neighbouring interface methods already carry.
E2E, port 4616
CreateTopic -> 201 Created, etag-00000003
CreateSubscription -> 201 Created, PENDING, confirmationToken token-00000009
Publish (PENDING) -> 200, delivers to nobody
Confirm -> 200
Publish (confirmed) -> 200
if-match fresh -> 200
if-match replayed -> 412 NoEtagMatch, and the resource is unchanged
bad EMAIL endpoint -> 400 "... must hold an @"
CUSTOM_HTTPS over http -> 400 "... must be an https URL"
body 64KiB+1 -> 400 "message body is 65537 bytes; ONS caps a message at 65536"
body exactly 64KiB -> 200
DeleteTopic -> 204 + Opc-Work-Request-Id
work request -> DELETE_TOPIC / SUCCEEDED / 100%
GET topic / subscription -> 404 / 404 (cascade)
On the cmd/cloudemu flake
An interim run showed cmd/cloudemu failing. It is not this branch — the diff touches nothing under cmd/ or persist/ (verified: 0 files). With nothing else running, go test ./cmd/cloudemu/ passes on clean development in 5.4s. The failures come from parallel worktree suites colliding on the shared ~/.cloudemu daemon lock.
NitinKumar004
left a comment
There was a problem hiding this comment.
This is close. The ONS surface is broad and the PENDING to ACTIVE flow is right for most protocols. A few wire-shape gaps still show up when a real SDK client is used. The branch also needs a rebase onto current development.
Earlier review points
- Mock not Snapshottable: fixed.
snapshot.gois there andgo test ./persist/...passes on the merged tree. A topic created before a gracefulserve --persistrestart was still there afterwards. - Create returned 200: fixed. CreateTopic and CreateSubscription now answer 201.
- if-match ignored: fixed for topics and subscriptions. A stale etag gets 412 NoEtagMatch. See the Low note on atomicity below.
- Endpoint and message-size validation: fixed. A bad EMAIL endpoint, a CUSTOM_HTTPS endpoint over http, and a 64 KiB + 1 byte body are each rejected with 400.
- docs/services.md missing ONS: fixed. There is now an "OCI Notifications (ONS)" section.
- Publish not mirrored on the wire: fixed.
server/oci/notifications/publish.gonow exists.
High
- Branch does not merge cleanly with current development.
git merge origin/developmentconflicts indocs/coverage/README.md. Development has since regenerated it with plain hyphens and new services.docs/coverage/coverage.jsonanddocs/services.mdmerge on their own. After I took development's README and re-rango run ./internal/coveragegen, the only diff left was the new OCI Notifications cell. Fix: rebase onto development, re-rungo generate ./...and commit the regenerateddocs/coverage.
Medium
-
GetSubscription loses the delivery policy for SDK and Terraform clients.
server/oci/notifications/types.go:75
The ONSSubscriptionmodel (used by Get, Create and Update) carries the policy as a JSON string indeliverPolicy. OnlySubscriptionSummary(List) uses thedeliveryPolicyobject. The handler sendsdeliveryPolicyas an object on every response.
Repro with oci-go-sdk v65: call UpdateSubscription withBackoffRetryPolicy{MaxRetryDuration: 7200000, PolicyType: EXPONENTIAL}, then GetSubscription.DeliverPolicyis nil, while ListSubscriptions returns the policy. The Terraformoci_ons_subscription.delivery_policyattribute is read fromDeliverPolicy, so a configured policy drifts on every plan.
Fix: insubscriptionResponse, senddeliverPolicyas the marshalled JSON string. Keep the object form only for list items. -
PublishMessage reads messageType from the query string, but the SDK sends it as a header.
server/oci/notifications/publish.go:27
PublishMessageRequest.MessageTypeiscontributesTo:"header" name:"messageType". Repro:curl -X POST -H 'messageType: NOPE' .../20181201/topics/{id}/messages -d '{"body":"b"}'returns 200. A JSON publish from the SDK is stored as RAW_TEXT. The handler's 400 for an unknown type only works through?messageType=, which no client sends.
Fix: readr.Header.Get("messageType"). The query fallback can stay if you want it. -
ORACLE_FUNCTIONS subscriptions are created PENDING.
providers/oci/notifications/subscriptions.go:151
The OCI docs say "Confirmation isn't required for function subscriptions." Here a Functions subscription starts PENDING and gets a confirmation token, so a publish delivers nothing until the caller confirms something real ONS never asks it to confirm. Repro: create a subscription withprotocol: ORACLE_FUNCTIONSand you get"lifecycleState":"PENDING"plus aconfirmationToken.
Fix: create ORACLE_FUNCTIONS subscriptions ACTIVE with no token, and add a driver test and a wire test for it. -
Compartments are not validated, and topic names are unique per compartment instead of per tenancy.
server/oci/notifications/topics.go:87,providers/oci/notifications/notifications.go:138
Development now gates VCN creates on an existing compartment throughvcnHandler.SetCompartmentCheckerinserver/oci/oci.go.POST /20160918/vcnsintoocid1.compartment.oc1..bogusreturns 404 NotAuthorizedOrNotFound. ONS accepts the same compartment with 201, for both CreateTopic and ChangeTopicCompartment (202), and CreateSubscription accepts any compartment too.
Separately,CreateTopicDetails.namein the SDK says "The topic name must be unique across the tenancy". Creatingt1in the root compartment and thent1in another compartment returns 201 both times. Real ONS returns 409.
Fix: give the notifications handler the same compartment checker hook as VCN and wire it inserver/oci/oci.go. MaketopicByNamelook across the whole tenancy, and apply the same check on compartment moves.
Low
-
The if-match check and the write are not atomic.
server/oci/notifications/topics.go:164,handler.go:207
topicIfMatchreads the etag and the update then takes the provider lock separately. In a throwaway test, 8 goroutines sentPUT /topics/{id}with the same current etag. In some rounds 2 of them got 200 where real OCI accepts one and returns 412 to the rest. The run was-raceclean, so this is lost-update semantics and not a data race. Fix: pass the expected etag into the provider methods and compare it underm.mu. -
There is no
etagresponse header. The SDK mapsCreateTopicResponse.EtagandGetTopicResponse.Etagfrom theetagheader, and both come back nil against serve. Only the body field is set. Fix: setetagon topic create, get and update. -
A bad page token silently restarts the listing.
server/oci/notifications/topics.go:362
?limit=1&page=garbagereturns 200 with page one. A client with a corrupted cursor can loop forever. Thispaginateis also a copy of the one inserver/oci/vcn/handler.go. Fix: return 400 InvalidParameter for a token that does not parse. A follow-up could move onepaginatehelper intoocirestso the two handlers share it. -
ChangeTopicCompartment returns 202 with an opc-work-request-id. In the SDK,
ChangeTopicCompartmentResponse(like DeleteTopic) only hasopc-request-id. SDK clients ignore the extra header, so this is harmless. The handler docs should not say real ONS returns a work request here, though. -
Confirm and unsubscribe treat
protocolas optional.GetConfirmSubscriptionRequestandGetUnsubscriptionRequestmark it mandatory. Fix: return 400 when it is missing (server/oci/notifications/subscriptions.go:282). -
Delivery history grows without a limit.
providers/oci/notifications/publish.go:120appends every message to every ACTIVE subscription forever, andsnapshot.gopersists all of it. A long-running serve with a busy topic grows memory and state-file size without bound. Fix: keep only the last N messages per subscription. -
ONS metrics always land in the default compartment with a
topicIddimension.providers/oci/monitoring/monitoring.go:88,notifications.go:325Realoci_notificationmetrics are scoped to the topic's compartment and keyed byresourceId. Also,PutMetricDatanow acceptsoci_namespaces from any portable-API caller, not only from sibling mocks. -
Nits: the
shortTopicIDcomment says "leading run" but the code takes the trailing 8 characters (notifications.go:344). Development's comments have moved to plain hyphens, but this diff still adds em dashes in docs/services.md and in code comments. -
For later, not this PR: development has no OCI gate under
--enforce-auth. Unsigned ONS and VCN calls both succeed with it on. When an OCI gate is added,/subscriptions/{id}/confirmationand/unsubscriptionmust be exempt as token-authenticated public endpoints.
Checked
- Build, vet and
go teston the merged tree (PR plus current development) for providers/oci, server/oci, persist, internal/coveragegen and server/wire/ocirest: all green. -raceon both notifications packages and providers/oci/monitoring, plus a concurrent publish, list, subscribe and update probe: no races.- golangci-lint 2.14 on the changed packages: clean. The 6 goconst hits are in
providers/oci/monitoring/query.go, which this PR does not touch. - Wiring: provider field,
New(),wireMonitoring,DriversFromand handler registration all in place. go generateis idempotent after the conflict is resolved.- Real oci-go-sdk v65.126.1 against
serve: CreateTopic, GetTopic, CreateSubscription, UpdateSubscription, GetSubscription, ListSubscriptions, PublishMessage, ListTopics, ChangeTopicCompartment, DeleteSubscription and DeleteTopic all work, and GetTopic returns 404 after the delete. - Raw wire against
serve: full confirmation flow, unsubscribe link, and topic delete cascading to its subscriptions. - Persistence: a topic survives a graceful
serve --persistrestart. - Library and serve use the same provider.
- Terraform with oracle/oci was not run: the provider builds
https://<service>.<region>.<realm-domain>endpoints and has no per-service endpoint override for a plain-HTTP listener, so the real SDK was used instead.
|
Please merge the latest |
Implements OCI Notifications against the portable notification driver. providers/oci/notifications: a Mock over memstore.Store guarded by a single RWMutex. Topics carry an ocid1.onstopic OCID, a compartment recorded at create and filtered on every list, a short topic id, lifecycle state, etag and creation time. Subscriptions carry an ocid1.onssubscription OCID and start PENDING with a confirmation token; a publish reaches only ACTIVE ones, so an unconfirmed endpoint receives nothing. Deleting a topic takes its subscriptions with it, as ONS does. server/oci/notifications: the /20181201 wire handler for topics, subscriptions, the two token-authenticated confirmation endpoints, the changeCompartment and resendConfirmation actions, and PublishMessage on the topic's own endpoint. Real ONS splits control and data plane by host rather than by prefix, so a topic reports the requesting origin as its apiEndpoint and a publish lands back on the same listener. DeleteTopic is asynchronous: it records a work request and answers 204 with opc-work-request-id. The OCI-only surface — subscription compartments, tags, metadata, delivery policy, the confirmation handshake and a topic's lifecycle state — is declared consumer-side as an Extras interface in the handler, with its value types in the provider. A driver that does not satisfy it is served 501. Nothing was added to services/notification/driver. Input the emulator cannot honour is refused rather than stored unused: message attributes, defined tags, protocols outside the ONS set, unknown message types and unsupported sort keys all answer 400 naming what is unsupported.
…n the wire Adds the hand-written OCI Notifications section docs/oci-conventions.md's Definition of done requires, splits the wire publish handler out of topics.go so the feature has one filename in both layers, and raises coverage on both packages. PutMetricData is the in-process path a sibling mock emits its own service metrics on, so it now admits the oci_ namespaces those metrics live under; ONS's oci_notification counters were being dropped.
…t limits Mock holds three memstore stores but implemented no snapshot.Snapshottable, so the persistence-completeness guard failed and a serve stop/start dropped every topic and subscription. Adds snapshot.go over topics, subscriptions and deliveries, mirroring providers/oci/vcn's shared storeDump table so Snapshot and Restore cannot drift. Deliveries are snapshotted rather than excluded: they stand in for the endpoint inbox real ONS pushes to and are the only thing Deliveries reads, so dropping them would report an ACTIVE subscription as having received nothing. Also brings three responses in line with real ONS: both creates answer 201, update and delete honour an if-match precondition (412 NoEtagMatch on a stale etag), and a malformed endpoint or an over-64 KB message body is rejected naming what is wrong instead of being stored unused.
…c if-match Wire shapes, checked against oci-go-sdk's ons package: - Create/Get/UpdateSubscription return ONS's Subscription, which carries the delivery policy as a JSON string in deliverPolicy; only the List item (SubscriptionSummary) keeps the deliveryPolicy object. Terraform's oci_ons_subscription.delivery_policy reads the former. - PublishMessage reads messageType from the header the SDK sends it in; the query parameter stays as a fallback. - Create, get and update of topics and subscriptions set the etag response header the SDK maps Etag from. - Confirm and unsubscribe require protocol, which ONS marks mandatory. Behaviour: - ORACLE_FUNCTIONS subscriptions are created ACTIVE with no token; ONS does not ask a function to confirm. - Topic names are unique across the tenancy, not per compartment. - CreateTopic, ChangeTopicCompartment, CreateSubscription and ChangeSubscriptionCompartment reject a nonexistent compartment with 404, via the same SetCompartmentChecker hook VCN uses, wired from Identity. - if-match is compared under the provider lock on topic update, delete and compartment move and on subscription update and delete, so of several writers holding one etag exactly one wins; the rest get 412 NoEtagMatch. - A page token the listing never issued is 400 instead of restarting. - Delivery history keeps the last 100 messages per subscription. - ONS metrics land in the topic's compartment keyed by resourceId, through a new monitoring PostServiceMetricData. PutMetricData is back to rejecting oci_ namespaces for portable callers. Docs: DeleteTopic and ChangeTopicCompartment return only opc-request-id in real ONS; CloudEmu's extra work request is now described as such. Em dashes this branch added are replaced with plain punctuation.
344b484 to
a2b74eb
Compare
|
Rebased onto High1. Rebase. Only Medium2. 3. 4. ORACLE_FUNCTIONS. Created ACTIVE with no token and receives the next publish. Resending a confirmation returns 409, since there is nothing to confirm. Driver and wire tests both added. 5. Compartments and tenancy-wide names. Copied the VCN pattern from development exactly:
One part I would push back on: "enforce it on compartment moves too". A move never changes the name, so a uniqueness check at move time could never fire. Low6. Atomic if-match. The etag is now compared under 7. 8. Bad page token. Garbage or negative tokens get 400 9. Work-request docs corrected. This also corrects a claim I made in an earlier reply on this PR: I wrote that DeleteTopic returns 204 with an 10. 11. Delivery history capped at the last 100 messages per subscription, copied with 12. Metrics. Now land in the topic's compartment keyed by Note for merge order: #422 also touches 13. Nits. The 14. OCI Verification
E2E against
|
NitinKumar004
left a comment
There was a problem hiding this comment.
Nearly there. Every earlier finding is fixed and checks out with the real SDK. One new wire bug is left: resending a subscription confirmation 404s for SDK clients.
Earlier findings
- Merge with development: fixed. The branch is on current development (770204c), so it merges cleanly.
go generate ./...leaves no diff indocs/coverage. deliverPolicyon Get: fixed. After UpdateSubscription with a 7200000 ms EXPONENTIAL policy,GetSubscriptionreturnsDeliverPolicyas the JSON string, andListSubscriptionsreturns theDeliveryPolicyobject. There is one leftover on the Update response, listed as a new Low below.messageTypeheader: fixed.PublishMessagewithPublishMessageMessageTypeJsonfrom the SDK returns 200, and a rawmessageType: NOPEheader returns 400.- ORACLE_FUNCTIONS: fixed. The subscription is created ACTIVE with no
confirmationTokenin the body. - Compartments and tenancy-wide names: fixed. A bogus compartment gets 404 on CreateTopic, ChangeTopicCompartment, CreateSubscription and ChangeSubscriptionCompartment.
t1in a second compartment gets 409, and the name is still taken in the old compartment after a move. Fair point on the move check, the name can't change there. - Atomic if-match: fixed. 8 concurrent SDK
UpdateTopiccalls with the same etag gave exactly one 200 and seven 412. A stale if-match on ChangeTopicCompartment and DeleteSubscription also returns 412. etagheader: fixed.Etagis set on CreateTopic, GetTopic, UpdateTopic, CreateSubscription and GetSubscription responses, and it matches the body.- Bad page token: fixed.
page=garbagereturns 400 on both ListTopics and ListSubscriptions. - Work-request docs: fixed.
protocolrequired: fixed. Confirm and unsubscribe withoutprotocolboth return 400.- Delivery history cap: fixed. After 152 publishes, the saved state file holds exactly 100 messages for each subscription.
- Metrics: fixed.
ListMetricsin the topic's compartment showsPublishedMessagesandDeliveredMessagesunderoci_notificationwith aresourceIddimension. A portablePostMetricDataintooci_notificationis rejected with 400 again. - Nits: fixed. The comment now says "trailing", and the diff adds no em dashes.
- OCI auth gate: left for later, as agreed.
New findings
Medium
- ResendSubscriptionConfirmation is served on the wrong path.
server/oci/notifications/subscriptions.go:24and:36,handler.go:14,docs/services.md:1520
The SDK sendsPOST /20181201/subscriptions/{id}/resendConfirmation(ons_notificationdataplane_client.go:620). It has no/actions/segment. The handler only routes/subscriptions/{id}/actions/resendConfirmation.
Repro: create a PENDING EMAIL subscription, then POST/20181201/subscriptions/{id}/resendConfirmation. You get 404, and/actions/resendConfirmationgets 200.ResendSubscriptionConfirmationfrom oci-go-sdk returns 404 for every subscription.
Fix: routert.Sub == "resendConfirmation"with no action toresendConfirmation. Update the path in the tests and indocs/services.md, and add an SDK-path wire test. While there, set theetagheader on that response too, since it returns aSubscription.
Low
-
The UpdateSubscription response body is the wrong shape for the SDK.
server/oci/notifications/subscriptions.go:179
In the SDK,UpdateSubscriptionResponsedecodes its body asUpdateSubscriptionDetails, which reads the policy from thedeliveryPolicyobject. It does not decode aSubscription. My earlier note put Update in theSubscriptiongroup, and that was wrong. Right nowresp.DeliveryPolicyis nil after a successful update. Get and List are correct.
Fix: have the update response also senddeliveryPolicyas an object. KeepingdeliverPolicyalongside it is harmless. -
ChangeSubscriptionCompartment ignores if-match.
server/oci/notifications/subscriptions.go:275
ChangeSubscriptionCompartmentRequestcarriesif-match, the same as the topic move. A move withIfMatch: "stale"returns 204 and moves the subscription.
Fix: passifMatch(r)into the provider and check it underm.mu, the same way as the other writes. -
golangci-lint
canonicalheaderatserver/oci/notifications/handler.go:257: use"ETag". CI only reports lint findings as warnings, but this is the only finding the PR adds.
Checked
- The branch is on current development (770204c) and merges cleanly.
go generate ./...leaves no diff. go build ./...is green. vet and test pass on providers/oci, server/oci, persist, internal/coveragegen and server/wire/ocirest.-race -count=3on both notifications packages and providers/oci/monitoring: clean.- golangci-lint 2.14
--new-from-rev=origin/development: one finding, item 4 above. - Real oci-go-sdk v65.126.1 against
serve -providers oci: every earlier repro listed above, plus GetConfirmSubscription, ListMetrics and DeleteTopic cascading to its subscriptions. serve --persistgraceful restart: topics, subscriptions and the capped delivery history all come back.- Not specific to this PR: after a restore the shared idgen counter starts again from a low number. A new ONS subscription took the same OCID as a restored one and overwrote it. Every service uses that counter, so the fix belongs in idgen or persist. It should be tracked separately.
Summary
notificationdriver.services/notification/driver— OCI-only behaviour is a consumer-sideExtrasinterface, per the rule set in Move OCI-only capabilities out of shared driver packages #393.Closes #415. Part of #376.
Changes
providers/oci/notifications/—Mockovermemstoreimplementingdriver.Notification, guarded by async.RWMutex.server/oci/notifications/— the/20181201/surface. Topics: Create/Get/List/Update/Delete/ChangeCompartment. Subscriptions: Create/Get/List/Update/Delete/Confirm/Unsubscribe/ResendConfirmation/ChangeCompartment. Data plane: PublishMessage.providers/oci/oci.goandserver/oci/oci.go.The confirmation flow
CreateSubscriptionmints an OCID, setslifecycleState: PENDINGand a confirmation token. Real ONS mails the token to the endpoint; the emulator has no channel, so the token rides back in the response body and is dropped once the subscription is ACTIVE.GET /subscriptions/{id}/confirmationvalidates token and protocol, flips to ACTIVE, and returns anunsubscribeUrlpointing at this emulator's own origin.PublishMessageskips anything not ACTIVE, so a publish to a PENDING subscription delivers to nobody — asserted at both driver and wire layers, and visible in the transcript below.Two corrections to the issue as written
apiEndpoint), both on/20181201. Implemented asPOST /20181201/topics/{id}/messages, with every topic reporting the requesting origin as itsapiEndpoint, so an SDK pointed at the returned endpoint lands back on the same listener.DeleteTopicreturns 204, not 202. Real ONS answers204with anopc-work-request-id.docs/oci-conventions.md's generic 202 is the common case, not ONS's. The work request is still recorded and pollable.Never accept-and-ignore
Refused with a named 400 rather than silently dropped: message attributes (ONS has no such field),
definedTags, protocols outside the ONS set, unknownmessageType, unsupportedsortBy/sortOrder. Protocol aliases (email,https→CUSTOM_HTTPS) are mapped, not dropped.Provider Coverage
Checklist
go test ./...) — see note belowgolangci-lint run --timeout=9m) — 0 issuescloudemu_test.go— driver + handler tests insteadTest Plan
Correction on a suite note. An earlier run of this branch showed
cmd/cloudemu TestServeOutOfProcessfailing, and I first attributed it to the sandbox. That was wrong. It is contention on the shared~/.cloudemudaemon lock between the six Wave 2 worktrees running their suites in parallel — self-inflicted, not a repo bug. Re-run with nothing else running, the whole suite is exit 0 including that test.Coverage leak check clean: no OCI operation appears in
docs/coverage/{aws,azure,gcp}/*.md, andgit diff development -- services/is empty.End-to-end on a running server (port 4616):
Left out
No
oci-go-sdkcompat test — the SDK splits ONS across two clients with a host override, needing more scaffolding than it would buy; wire shapes are asserted field-by-field instead. Publishing records deliveries rather than pushing to real HTTPS/email endpoints, matching howproviders/aws/snshandles non-SQS protocols.definedTagsis refused rather than modelled, consistent with the rest of the OCI surface.