From cb30dc92a3f9f2122add28c9cb57bbbca96e4171 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Thu, 10 Sep 2026 00:16:34 +0530 Subject: [PATCH 1/8] feat(AgricultureFacility): add the POCRA facility plugin, on latest development [#91] PR #22's work, replayed onto development and adapted to it. Squashed from 22 commits: 12 of them modified internal/upstream/upstream.go, which development split into seven files, so each would have conflicted as a modify/delete. The net change to the shared package across all 22 was one exported function; the rest was churn -- fan-out added then removed, BindingPaths renamed three times. WHAT THE ADAPTATION NEEDED. development has since renamed internal/upstream to internal/common, folded capabilitybinding into it, and made auth per provider: internal/upstream -> internal/common capabilitybinding.From -> common.BindingFrom, exported for this plugin bindingPaths -> common.BindingPaths, exported for this plugin Config's flat auth fields -> Config.AuthByProvider, one profile per provider authScheme: none -> a pocra: block, nested under the participant id Two of those are exports the plugin needs and this branch adds, with the reason recorded at each: it answers "is this payload mine?" for a multi-type search, and it must answer it the same way serve does rather than reading the payload a second way. The config entry and the steps list gain AgricultureFacility alongside the three that were already there; build-plugins.sh gains it too, rather than replacing KnowledgeAdvisory as the original diff did -- that branch predated the knowledge plugin. Their tests needed the same adaptation. Every helper that builds a Config now declares pocra's auth, since a served provider without a block is refused at startup -- which is the new validation working, not a defect. 69 packages green, vet clean, no races. The other three capabilities were run separately to confirm the two new exports changed nothing for them. Original commits are preserved on feat/91-AgricultualFacility(Plugin) and on PR #22, which is untouched. --- .gitignore | 6 +- .../pocra/agriculture-facility.select.yaml | 514 +++++ config/provider-adapter.yaml | 28 + install/build-plugins.sh | 1 + .../AgricultureFacility.go | 104 + .../AgricultureFacility/README.md | 187 ++ .../AgricultureFacility/cmd/plugin.go | 120 ++ .../AgricultureFacility/cmd/plugin_test.go | 199 ++ .../AgricultureFacility/conformance_test.go | 462 +++++ .../AgricultureFacility/live_test.go | 399 ++++ .../AgricultureFacility/mappings_test.go | 1762 +++++++++++++++++ .../AgricultureFacility/payload.go | 134 ++ .../AgricultureFacility/payload_test.go | 133 ++ .../AgricultureFacility/prerequisites.go | 17 + .../AgricultureFacility/schemacache_test.go | 309 +++ .../AgricultureFacility/search.go | 398 ++++ .../AgricultureFacility/search_test.go | 249 +++ .../AgricultureFacility/testdata/.gitignore | 4 + .../implementation/internal/common/binding.go | 7 +- .../internal/common/binding_test.go | 30 +- .../implementation/internal/common/common.go | 11 +- .../implementation/internal/common/serve.go | 4 +- .../internal/concurrent/concurrent.go | 116 ++ .../internal/concurrent/concurrent_test.go | 288 +++ 24 files changed, 5459 insertions(+), 23 deletions(-) create mode 100644 config/mappings/pocra/agriculture-facility.select.yaml create mode 100644 pkg/plugin/implementation/AgricultureFacility/AgricultureFacility.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/README.md create mode 100644 pkg/plugin/implementation/AgricultureFacility/cmd/plugin.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/cmd/plugin_test.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/conformance_test.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/live_test.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/mappings_test.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/payload.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/payload_test.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/prerequisites.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/schemacache_test.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/search.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/search_test.go create mode 100644 pkg/plugin/implementation/AgricultureFacility/testdata/.gitignore create mode 100644 pkg/plugin/implementation/internal/concurrent/concurrent.go create mode 100644 pkg/plugin/implementation/internal/concurrent/concurrent_test.go diff --git a/.gitignore b/.gitignore index 6292bb58..1134973c 100644 --- a/.gitignore +++ b/.gitignore @@ -176,4 +176,8 @@ test_request.json .claude # catalogPublish handler's local output root (config/local-beckn-one-bap.yaml) -/catalog/ \ No newline at end of file +/catalog/ + +# Local, non-shipped developer notes (review summaries, scratch docs) +/dev_docs/ +/docs/ \ No newline at end of file diff --git a/config/mappings/pocra/agriculture-facility.select.yaml b/config/mappings/pocra/agriculture-facility.select.yaml new file mode 100644 index 00000000..44aee0fe --- /dev/null +++ b/config/mappings/pocra/agriculture-facility.select.yaml @@ -0,0 +1,514 @@ +# POCRA, openagrinet:AgricultureFacility, select. Both directions, one file. +# +# One file per binding-action rather than one per direction, because both legs of +# an exchange are one contract: the response has to answer the request that was +# sent, and splitting them lets one change without the other. +# +# The registry entry pointing here decides which action this serves, so nothing +# in the file names it. The filename's action segment must match that entry -- +# a mismatch would apply a correct mapping to the wrong call, silently. +# +# Both halves read: +# beckn the inbound Beckn payload +# and the response half additionally reads: +# response POCRA's answer, in its own shape +# +# WHERE THE QUERY LIVES, and why it is not in resourceAttributes. +# +# An inbound query resource is informationMode OnDemand. The search origin is +# read from the Beckn fulfillment stop and the requested type from +# supportedFacilityTypes, rather than from location/address/facilityType on +# resourceAttributes -- a convention this file chooses to keep, not a schema +# requirement: the AgricultureFacility v0.1 pack no longer forbids those +# fields under OnDemand (it did until network-specs commit b76c9ad8a5 on +# schema-packs-v0.1 quietly dropped that constraint; see +# dev_docs/schema-onDemand-forbid-removed.md). Nothing here depends on the +# constraint being back -- the search origin still has nowhere verified to go +# in resourceAttributes, since POCRA returns no per-facility coordinate for +# the searched point. +# +# PROVISIONAL. This convention was chosen on the design and has not been +# confirmed against a payload captured from the network. Confirm it before the +# registry row goes live. + +# What this capability cannot serve, refused before the provider is called. +# +# A predicate that is false refuses the request with the message beside it, so +# the caller is told what is wrong with their payload rather than that an +# expression somewhere returned false. Each check is its own expression and binds +# for itself; the first failure is the one reported. +# +# Sending an unservable payload anyway earns POCRA's own schema NACK, whose +# instanceLocation names a JSON pointer into a request the caller never saw. +required: + - check: | + ( + $geo := beckn.message.contract.commitments[0].fulfillment.stops[0].location.geo; + $exists($geo) and $geo.type = "Point" and $count($geo.coordinates) = 2 + ) + message: "this capability needs a fulfillment stop with a Point location; POCRA searches around one point at a time" + - check: | + ( + $requested := [beckn.message.contract.commitments[0].resources[0].resourceAttributes.supportedFacilityTypes]; + $governed := ["CustomHiringCentre", "KrishiVigyanKendra", "Warehouse", "SoilTestingFacility"]; + $count($requested) > 0 and $count($requested[$ in $governed]) = $count($requested) + ) + message: "every entry in supportedFacilityTypes must name a governed facility type: CustomHiringCentre, KrishiVigyanKendra, Warehouse or SoilTestingFacility" + - check: | + ( + $requested := [beckn.message.contract.commitments[0].resources[0].resourceAttributes.supportedFacilityTypes]; + $count($requested) = $count($distinct($requested)) + ) + message: "supportedFacilityTypes must not repeat a facility type" + +# ONE call per payload, and this file is written for exactly that. +# +# POCRA's search takes exactly ONE category code: a comma-separated pair +# ("kvk,warehouse") answers 200 with no providers at all, and a category array +# is refused with "Schema validation failed" -- both verified against the live +# API. So a payload asking for two facility types cannot be served by one call, +# however this file is written. +# +# The plugin handles that before this file is reached. It reads +# supportedFacilityTypes out of the payload, splits a multi-type search into +# one single-type payload per type, runs each of them concurrently and merges +# the answers -- see AgricultureFacility/search.go. So what arrives here always +# names exactly one facility type, and both halves below read it straight off +# the payload with no variable injected from Go. +# +# The cost of that, for whoever edits this file next: the required: checks +# above read supportedFacilityTypes in JSONata, and search.go reads the same +# path in Go. If the Beckn payload moves that field, BOTH change, and only +# this one is a config edit. + +# The upstream is a POST, so this object becomes the request body. +# +# This is where the extraction lives, deliberately. When POCRA wants another +# parameter it is an edit here and nothing else: no Go, no rebuild, live on the +# next cache expiry. +# +# $codes is the ONLY place POCRA's private vocabulary appears. Adding a facility +# type is one line here, one line in the inverse table below, and one more entry +# in the precondition's list. +# +# GeoJSON is [lon, lat] -- longitude first -- and POCRA's gps is "lat,lon". +# Reading them the same way round gives a point in the wrong place that is still +# a valid request, so it fails as wrong data rather than as an error. +# +# context.timestamp is epoch SECONDS as a string. Every other timestamp in both +# protocols is ISO 8601, and POCRA's schema refuses one here. +# +# domain, version, bap_id and bap_uri are POCRA's request contract rather than +# this adapter's identity. Identity on the wire is the adapter's own: it signs +# what it answers with, using the key the registry publishes for it. +request: | + ( + $commitment := beckn.message.contract.commitments[0]; + $stop := $commitment.fulfillment.stops[0]; + /* This call's own facility type. The payload names exactly one, because + the plugin split a multi-type search into one payload per type before + this ran -- reading [0] of a multi-type list here is what made such a + search return one type and silently drop the rest. */ + $requested := [beckn.message.contract.commitments[0].resources[0].resourceAttributes.supportedFacilityTypes][0]; + $codes := { + "CustomHiringCentre": "chc", + "KrishiVigyanKendra": "kvk", + "Warehouse": "warehouse", + "SoilTestingFacility": "soil_lab" + }; + { + "context": { + "domain": "advisory:mh-vistaar", + "location": { "country": { "name": "IND" } }, + "action": "search", + "version": "1.1.0", + "bap_id": "bap.mahapocra.gov.in", + "bap_uri": "https://middleware.mahapocra.gov.in/bap/", + /* This call's own id. POCRA keeps a message_id's answers for ten + minutes and returns the UNION of everything asked for under it, so + two calls sharing an id would each return the other's facilities + too. + + Read straight off the payload: the plugin stamps a fresh UUID on + context.messageId of every single-type payload it splits out, for + exactly this reason, so each call already carries its own. POCRA's + schema refuses a message_id that is not a UUID. */ + "message_id": beckn.context.messageId, + "transaction_id": beckn.context.transactionId, + "timestamp": $string($floor($millis() / 1000)) + }, + "message": { + "intent": { + "category": { "descriptor": { "code": $lookup($codes, $requested) } }, + "item": { "descriptor": { "name": "service-locations" } }, + "fulfillment": { + "stops": [ + { + "location": { + "gps": $string($stop.location.geo.coordinates[1]) & "," & $string($stop.location.geo.coordinates[0]) + }, + "time": { + "range": { + "start": $exists($stop.time.range.start) ? $stop.time.range.start : $now() + } + } + } + ] + } + } + } + } + ) + +# Keyed by direction, not by the action it produces: a select is answered by an +# on_select over the same HTTP round trip, so the callback is this half rather +# than an action of its own. +# +# POCRA answers with responses[], one entry per BPP that replied, each carrying +# its own catalog. Every facility is an item inside one of them, so the path +# flattens all four levels at once. +response: | + ( + $selected := beckn.message.contract.commitments[0]; + + /* The caller's own @context, echoed back rather than restated here. + Hardcoding the pack URL meant this file had to know which one is current, + and could contradict what the request actually declared. Not enforced by + a required check above -- an absent @context here means an absent one on + the way out too, which degrades the JSON-LD rather than breaking pack + conformance (conformance_test.go treats @context as conventional, not a + schema property). + + Backticks because @ is an operator in JSONata. */ + $ctx := $selected.resources[0].resourceAttributes.`@context`; + $resourceId := function($item) { "res:pocra:facility:" & $item.id }; + + /* The inverse of the request half's $codes. Read from the item's OWN + category tag rather than echoed from the request, so an answer carrying + more than one category still labels each facility correctly. */ + $types := { + "chc": "CustomHiringCentre", + "kvk": "KrishiVigyanKendra", + "warehouse": "Warehouse", + "soil_lab": "SoilTestingFacility", + /* The warehouse BPP labels its fulfillment GSW rather than "warehouse", + and its items carry no category tag at all, so this is the only place + a warehouse says what it is. */ + "GSW": "Warehouse" + }; + + /* EVERY type the payload asked for, as a list. The answer is filtered to + this set rather than to its first entry -- filtering to [0] is what made a + two-type search drop a type. */ + $requestedTypes := [$selected.resources[0].resourceAttributes.supportedFacilityTypes]; + + /* POCRA states a provider's facility type in the provider's own id: + COMMON_PROVIDER_KVK, COMMON_PROVIDER_CHC, COMMON_PROVIDER_SOIL_LAB. That + is the outermost and most reliable statement of what a group of items is, + because POCRA groups items under one provider per category. + + $contains first because $substringAfter returns the WHOLE string when the + separator is absent, so "WAREHOUSE001" would otherwise become the code + "warehouse001" and look like a deliberate answer rather than no answer. + Warehouse providers are named that way and state their type only in their + fulfillment, so they fall through to the sources below. */ + $providerCode := function($id) { + $contains($id, "COMMON_PROVIDER_") + ? $lowercase($substringAfter($id, "COMMON_PROVIDER_")) + }; + + /* Every facility POCRA returned, each carrying the type it actually is. + Walked per PROVIDER rather than flattened straight to items, because both + of the reliable statements of type are the provider's -- a warehouse item + carries no category tag of its own at all. + + Three sources, narrowest-trusted first: the provider's id, then the + item's own category tag, then the provider's fulfillment category. The + provider id leads because it is what POCRA groups by; the item tag is + kept as a second source so a provider that ever mixed categories still + labels each facility correctly; the fulfillment is what a warehouse has + instead of either. + + Nothing falls back to what was REQUESTED. Doing that relabels a leaked + facility as the thing being searched for, which is how a warehouse comes + back as a Krishi Vigyan Kendra. An item none of the three can type stays + untyped and is dropped by the filter below -- POCRA also returns + administrative and mandi providers, which are not facilities at all. */ + /* POCRA writes "Unknown", "N/A" and "000000" where it has no value. + Publishing them would put junk on the network under a governed schema, so + a value that is one of them is treated as absent. A ternary with no else + yields nothing, which is how a field is omitted rather than nulled. */ + $clean := function($value) { + ($exists($value) and $value != "" and $value != "Unknown" + and $value != "N/A" and $value != "000000" and $value != "-") ? $value + }; + + /* $number($substringBefore(...)) THROWS on anything it cannot parse, and + that throw is not local: it propagates out of the whole response + mapping, so one placeholder on one item's distance or capacity failed + the ENTIRE search, good facilities included. $exists() cannot guard + it -- $exists($number(...)) evaluates its argument first, so the throw + fires before $exists is ever reached. + + $clean strips the placeholders this file already knows POCRA writes + ("Unknown", "N/A", "000000", "-"); $type/$match then confirms what is + left actually looks like "" before $number ever + sees it. Same shape as $priced in agmarknet/mandi-price.select.yaml, + and for the same reason: $match() itself throws T0410 on a non-string, + so the string check has to come first rather than be folded into the + regex. + + Returns nothing when the value cannot be read as a number -- absent, + not zero, so a caller cannot mistake "not reported" for "reported as + none". */ + $numericPrefix := function($tagged) { + ( + $head := $clean($substringBefore($tagged, " ")); + $exists($head) and $type($head) = "string" and $match($head, /^[0-9]+(\.[0-9]+)?$/) + ? $number($head) + ) + }; + + $labelled := response.responses.message.catalog.providers.( + $provider := $; + $fromProvider := $lookup($types, $providerCode($provider.id)); + /* NOT a single $lookup($types, ....descriptor.code): a provider with more + than one fulfillment, or a fulfillment with more than one category, + makes descriptor.code a SEQUENCE rather than a string, and $lookup + throws T0410 on anything but a scalar key. One bad provider shape then + took the whole response down, all items included -- not just the ones + with more than one category. + + Each code is looked up on its own instead, and the first that resolves + to a governed type wins. A provider that never mixes categories still + gets exactly the $lookup($types, code) it had before, since a + one-element sequence maps to a one-element result. */ + $fromFulfillment := ( + $resolved := [$provider.fulfillments.categories.descriptor.code].($lookup($types, $)); + $resolved[$exists($)][0] + ); + $map($provider.items, function($item) { + { + "item": $item, + "type": ( + /* Same T0410 hazard as $fromFulfillment above: an item carrying + more than one category tag makes .value a SEQUENCE, and $lookup + throws on anything but a scalar key. Same fix -- look up each + code on its own and take the first that resolves. */ + $tagged := ( + $resolved := [$item.tags.list[descriptor.code = "category"].value].($lookup($types, $)); + $resolved[$exists($)][0] + ); + $exists($fromProvider) ? $fromProvider + : $exists($tagged) ? $tagged + : $fromFulfillment + ) + } + }) + ); + + /* A search is answered with the types it asked for and nothing else, + whatever POCRA chose to include. + + It includes more than was asked for, verified against the live API: it + caches per message_id for PT10M and ACCUMULATES across searches sharing + one, so a kvk search under a reused message_id comes back carrying the chc + facilities of an earlier one. The step now sends a fresh message_id per + call, which stops this adapter causing it; the filter stays because a + provider deciding what to include is not something a mapping should + trust either way. */ + $wanted := $labelled[type in $requestedTypes]; + + /* The same facility appears in more than one responses[] entry, so grouping + by id keeps one of each; [0] takes the first where a key collected + several. A repeated id in the answer would be a dangling reference the + moment anything resolved it. */ + $unique := $each($wanted{ item.id: $ }, function($grouped) { $grouped[0] }); + + /* POCRA ranks by distance and reports it only as a string with a unit -- + "165 Km". It is parsed to sort by and then dropped: the pack states that + query-relative distance is not an intrinsic facility attribute, and + belongs in result metadata rather than on the facility. + + Sorting the strings would put "165 Km" before "98 Km", which nothing + downstream could detect. An item with no distance sorts last, which is + what an absent ranking should mean. */ + $reading := function($item) { + ( + /* A duplicated distance tag makes .value a sequence, and + $numericPrefix's $substringBefore throws on anything but a string + before the $type guard inside it is ever reached. Take the first. */ + $values := [$item.tags.list[descriptor.code = "distance"].value]; + $numericPrefix($values[0]) + ) + }; + $sorted := $map($unique, function($entry) { + ( + $distance := $reading($entry.item); + { + "facility": $entry.item, + "type": $entry.type, + "distance": $exists($distance) ? $distance : 1e9 + } + ) + })^(distance); + + /* POCRA reports a warehouse's capacity as one string with a unit -- + "500 tons" -- and reports it for no other facility type. The pack has a + home for it, so dropping it would discard the most useful thing a + warehouse search returns. + + No basis: the pack's example says "total storage", but POCRA does not say + which it means and guessing would put an unverified claim under a governed + field. */ + $capacity := function($item) { + ( + /* A duplicated capacity_estimate tag makes .value a sequence -- same + T0410 hazard as distance, above. Take the first. */ + $values := [$item.tags.list[descriptor.code = "capacity_estimate"].value]; + $reported := $values[0]; + /* value guarded the same way distance is, above -- "Unknown" or any + other placeholder must not reach $number. unit is left unguarded + deliberately: it is only ever read when value parsed, and at that + point substringAfter on the same string that just matched the + numeric-prefix regex cannot itself throw. */ + $value := $numericPrefix($reported); + $exists($value) ? { + "value": $value, + "unit": $substringAfter($reported, " ") + } + ) + }; + + /* A Beckn Address. streetAddress is POCRA's one real address line; district, + taluka and village go to extendedAddress, which is where the pack's own + examples put them. addressRegion is Maharashtra because POCRA is that + state's aggregator and publishes nothing outside it. */ + $address := function($item) { + ( + $parts := [$clean($item.address.district), $clean($item.address.taluka), $clean($item.address.vilage)]; + { + "streetAddress": $clean($item.address.address), + "extendedAddress": $count($parts) > 0 ? $join($parts, ", "), + "postalCode": $clean($item.address.pinCode), + "addressRegion": "Maharashtra", + "addressCountry": "IN" + } + ) + }; + + /* Organisational contact only. The pack restricts publicContact to contact + data approved for catalog publication, and POCRA's person field is an + office rather than an individual. Omitted entirely when there is nothing + usable, rather than emitted empty. */ + $contact := function($item) { + ( + $company := $clean($item.contact.person); + $phone := $clean($item.contact.phone); + $email := $clean($item.contact.email); + ($exists($company) or $exists($phone) or $exists($email)) ? { + "company": $company, + "phone": $phone, + "email": $email + } + ) + }; + + $attributes := function($item, $facilityType) { + { + "@context": $ctx, + "@type": "openagrinet:AgricultureFacility", + "informationMode": "Direct", + /* REQUIRED by AgricultureResource v0.1, and the AgricultureFacility + pack additionally requires this list to CONTAIN "Facility". It is + the pack's broad discovery category, deliberately coarse: the + concrete kind is facilityType below. + + "Facility" alone. The pack permits further values to name the + agricultural domains a facility serves (Crop, Livestock, ...), and + POCRA states nothing about that for any item -- inventing one would + publish a guess as a governed discovery category. */ + "subjectCategories": ["Facility"], + "facilityType": $facilityType, + "address": $address($item), + "capacity": $capacity($item), + /* sourceUri rather than website: contact.webUrl is POCRA's own site + repeated on every item, so it identifies the source and not the + facility. */ + "source": { + "sourceId": "pocra", + "sourceName": "PoCRA Provider Aggregator Services", + "sourceUri": $clean($item.contact.webUrl) + }, + "publicContact": $contact($item) + } + }; + { + /* Correlation only: the ids that tie this answer to the request that + asked for it, and nothing that asserts who anybody is. + + bapId, bapUri, bppId and bppUri are deliberately absent. A mapping is a + payload transformation -- it has no business asserting network identity, + and the two Uri fields it could copy are whatever the caller happened to + send, which in a deployed stack is a container-internal address that + means nothing to anyone outside it. */ + "context": { + "version": beckn.context.version, + "action": "on_select", + "networkId": beckn.context.networkId, + "transactionId": beckn.context.transactionId, + "messageId": beckn.context.messageId, + "timestamp": $now() + }, + "message": { + "contract": { + "commitments": [ + { + /* DRAFT, not QUOTED. The Beckn v2 status enum is DRAFT, ACTIVE and + CLOSED, and a quote is still a draft: nothing is committed until + init and confirm. QUOTED read better and validated nowhere. */ + "status": { + "descriptor": { "code": "DRAFT", "name": "Draft" } + }, + /* The offer is echoed, but its references are not: the request + named an abstract facility search, and the answer returns the + concrete facilities that satisfy it. Leaving resourceIds as they + arrived would point the offer at an id that appears nowhere in + the answer. + + $merge keeps everything else the request offered -- the id, the + descriptor, the provider -- and replaces one key. */ + "offer": $merge([ + $selected.offer, + { "resourceIds": [$map($sorted, function($entry) { $resourceId($entry.facility) })] } + ]), + /* Wrapped in [] because JSONata collapses a one-element sequence + to a bare value, so a single facility would answer with an + object where every other count answers with a list. */ + "resources": [$map($sorted, function($entry) { + { + "id": $resourceId($entry.facility), + /* Required by Commitment.resources in the spec, which defines + no quantity property and no Quantity schema anywhere -- a + defect upstream. One resource is one facility, so one. */ + "quantity": 1, + /* Direct mode requires @type, informationMode, facilityType + and source, plus one of location or address. + + location is deliberately absent: POCRA returns no verified + per-facility coordinate, and the pack forbids substituting + the search origin for one. + + services, capacity, website and lastUpdatedAt are absent for + the same reason -- POCRA supplies none of them, and deriving + them from the facility type, or lastUpdatedAt from the time + of the fetch, would assert something nobody verified. */ + "resourceAttributes": $attributes($entry.facility, $entry.type) + } + })] + } + ] + } + } + } + ) diff --git a/config/provider-adapter.yaml b/config/provider-adapter.yaml index f6c798c9..8d1c7450 100644 --- a/config/provider-adapter.yaml +++ b/config/provider-adapter.yaml @@ -435,6 +435,33 @@ modules: # something that no longer parses. maxResponseBytes: 1048576 + - id: AgricultureFacility + config: + # One capability for all four governed facility types. Which one a + # request wants is a value in its payload -- supportedFacilityTypes + # -- not part of the binding key, so there is one row in the + # registry and one mapping however many types are in use. + bindingKeys: "pocra|openagrinet:AgricultureFacility" + + # POCRA's search takes no credential, which is also why its + # baseUrl can be published as it stands. Auth is per provider, in + # a block named for the participant id -- declared even when the + # scheme is none, because a served provider without a block is + # refused at startup. + pocra: + authScheme: none + + # internal/common's default is 1 (sequential), deliberately: + # POCRA's failure mode when pushed is a 200 with an EMPTY catalog, + # indistinguishable from "no results", so a parallel search can + # silently lose a facility type with no error anywhere, with + # nothing in the payload recording that a type was asked for and + # lost. Kept at 1 here until that's verified against the live + # API -- raise it in a follow-up once confirmed safe, to at most + # 4, which is every governed type at once and so full + # concurrency for this capability. + searchConcurrency: 1 + # Declaring a step above is not enough: THIS list is what runs. A step # that appears under providerSteps but not here never executes, and the # request falls through to the 404 above -- which looks like a registry @@ -445,6 +472,7 @@ modules: - WeatherObservation # its binding key, or pass through - MandiPrice # its binding key, or pass through - KnowledgeAdvisory # its binding key, or pass through + - AgricultureFacility # its binding key, or pass through - signAck # signs whatever the step answered with # ---------------------------------------------------------------------------- diff --git a/install/build-plugins.sh b/install/build-plugins.sh index 59e5b817..4e505cc0 100755 --- a/install/build-plugins.sh +++ b/install/build-plugins.sh @@ -35,6 +35,7 @@ plugins=( "WeatherObservation" "MandiPrice" "KnowledgeAdvisory" + "AgricultureFacility" "manifestloader" "reqpreprocessor" "otelsetup" diff --git a/pkg/plugin/implementation/AgricultureFacility/AgricultureFacility.go b/pkg/plugin/implementation/AgricultureFacility/AgricultureFacility.go new file mode 100644 index 00000000..17f1e8c8 --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/AgricultureFacility.go @@ -0,0 +1,104 @@ +// Package AgricultureFacility serves the openagrinet:AgricultureFacility +// capability. +// +// One package per capability, named for the capability it serves, so which +// plugin owns one is readable from its binding key. +// +// Named for the capability and not for POCRA, deliberately. A provider is a +// registry row, and more than one could serve this same capability -- a second +// state aggregator would be another row and another mapping, not another +// package. +// +// Almost nothing lives here, and that is the point. Recognising a capability, +// resolving the call plan, authenticating, calling with the registry's budget +// and translating in both directions are all internal/upstream's, because none +// of them differ by domain. What this package owns is its name, and +// prerequisites -- the work a mapping cannot express, which is domain knowledge +// by definition. +// +// The upstream this was written against is POCRA's aggregator, whose search +// takes a category code and a point, both of which an AgricultureFacility +// payload carries. So the package is a name and nothing else: see +// prerequisites.go for why that is worth stating. +package AgricultureFacility + +import ( + "context" + + "github.com/beckn-one/beckn-onix/pkg/plugin/definition" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" +) + +// Config carries everything common.Config does, plus this capability's own +// search concurrency -- which common.Config has no field for, because how +// many calls one payload becomes is not something that package knows about at +// all. A flat struct with common.Config's fields repeated rather than an +// alias (which this package used to be, and MandiPrice and WeatherObservation +// still are) or an embedded common.Config (which would break every existing +// flat struct literal, `&Config{BindingKeys: ..., AuthScheme: ...}`, since +// Go's composite literal syntax does not promote an embedded struct's fields +// the way a selector expression does). +type Config struct { + BindingKeys []string `yaml:"bindingKeys" json:"bindingKeys"` + ProviderIDAt string `yaml:"providerIdAt" json:"providerIdAt"` + CapabilityCodeAt string `yaml:"capabilityCodeAt" json:"capabilityCodeAt"` + MaxResponseBytes int64 `yaml:"maxResponseBytes" json:"maxResponseBytes"` + + // AuthByProvider carries one credential profile per provider, keyed by + // participant id. Passed through to the inner step, which is where the + // schemes are defined and validated -- see common.AuthProfile. + AuthByProvider map[string]*common.AuthProfile `yaml:"-" json:"-"` + + // SearchConcurrency is how many of a multi-type search's calls may be in + // flight at once. See search.go's DefaultSearchConcurrency and + // MaxFacilityTypes for what absent and too-large mean. + SearchConcurrency int `yaml:"searchConcurrency" json:"searchConcurrency"` +} + +// New creates the agriculture facility step. +// +// Which capabilities it answers to is configuration, with no default: a package +// serving a family cannot guess which of them a deployment has providers for. +// +// Two steps, one returned. The inner one is internal/upstream's, which serves +// one payload with one call and knows nothing about facility types. The outer +// one is this package's own (see search.go): it splits a multi-type search +// into one single-type payload per type, runs the inner step over each of them +// concurrently, and merges the answers. Everything POCRA-specific about that +// is in the outer step, which is why the inner one is the same step +// MandiPrice and WeatherObservation use unchanged. +func New(ctx context.Context, registry definition.ProviderRecordLookup, mapper definition.Mapper, + cfg *Config) (definition.Step, func() error, error) { + if cfg == nil { + cfg = &Config{} + } + + upstreamCfg := &common.Config{ + BindingKeys: cfg.BindingKeys, + ProviderIDAt: cfg.ProviderIDAt, + CapabilityCodeAt: cfg.CapabilityCodeAt, + MaxResponseBytes: cfg.MaxResponseBytes, + AuthByProvider: cfg.AuthByProvider, + } + + one, closer, err := common.New(ctx, registry, mapper, prerequisites, upstreamCfg) + if err != nil { + return nil, nil, err + } + + // The same paths the inner step resolved from the same config, so both + // answer "is this payload mine?" identically. Resolved through upstream + // rather than duplicated here: a second reading of the same two config + // fields could drift from the first. + paths, err := common.BindingPaths(upstreamCfg) + if err != nil { + return nil, nil, err + } + + return &Step{ + inner: one, + paths: paths, + bindingKeys: cfg.BindingKeys, + concurrency: searchConcurrency(cfg.SearchConcurrency), + }, closer, nil +} diff --git a/pkg/plugin/implementation/AgricultureFacility/README.md b/pkg/plugin/implementation/AgricultureFacility/README.md new file mode 100644 index 00000000..f1541eb2 --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/README.md @@ -0,0 +1,187 @@ +# Agriculture Facility Plugin + +A **provider step plugin** that serves `openagrinet:AgricultureFacility` by +calling an ordinary HTTP API that has never heard of Beckn. + +Today that API is POCRA's aggregator, which answers +`pocra|openagrinet:AgricultureFacility` for the `select` action. + +## What lives here + +Almost nothing. Recognising a capability, resolving the call plan, +authenticating, calling with the registry's budget and translating in both +directions are all `internal/upstream`'s. This package owns its name and its +prerequisites — which are empty, because a facility search names the point and +the facility type it wants, and POCRA's search takes exactly those. + +Named for the schema-pack family rather than for POCRA: a provider is a registry +row, and a second state aggregator would be another row and another mapping, not +another package. + +## Configuration + +```yaml +providerSteps: + - id: AgricultureFacility + config: + bindingKeys: "pocra|openagrinet:AgricultureFacility" + authScheme: none + searchConcurrency: 4 +``` + +## How a multi-type search is served + +POCRA's search takes exactly ONE category code: a comma-separated pair answers +200 with no providers at all, and a category array is refused outright, both +verified against the live API. A Beckn payload asking for three facility types +therefore has to become three calls. + +That is this package's own job, and all of it lives here: + +- `search.go` is the step the adapter runs. It reads the facility types out of + the payload, splits one payload into one single-type payload per type -- each + with a fresh `context.messageId`, because POCRA returns the union of + everything asked for under one id -- runs the ordinary upstream step over + each part concurrently, and merges the answers into one. +- `payload.go` is the one path read this package does in Go rather than in a + mapping, and the one place `supportedFacilityTypes` is located. Its own + comment says what that costs. +- `internal/upstream` serves one payload with one call and knows none of this. + `jsonmapper` compiles the two halves every mapping has and no third thing. + `internal/concurrent` runs N of anything, bounded and ordered, and has never + heard of Beckn. + +**Ordering, worth knowing:** within one facility type the mapping ranks by +POCRA's distance, and that ranking survives the merge. ACROSS types the answer +is type-blocked -- every KrishiVigyanKendra, then every Warehouse -- rather +than globally nearest-first, because the schema pack says query-relative +distance is not a facility attribute, so the mapping drops it before the merge +could sort on it. + +| Parameter | Required | Description | Default | +|-----------|----------|-------------|---------| +| `bindingKeys` | **Yes** | Comma-separated capabilities this step answers to. No default is possible: a package serving a family cannot guess which of them a deployment has providers for. | — | +| `providerIdAt` | No | Path override for where the provider-id half of a binding key sits in a payload. Beckn v2 convention if absent. | Beckn v2 convention | +| `capabilityCodeAt` | No | Path override for the capability-code half. Must be given together with `providerIdAt`. | Beckn v2 convention | +| `authScheme` | No | `none`, `basic`, `header` or `query`. POCRA needs none. | `none` | +| `maxResponseBytes` | No | Cap on what is read from the provider. | 4 MiB | +| `searchConcurrency` | No | How many of a multi-type search's calls run at once, up to `MaxFacilityTypes` (8, this package's own constant -- see `search.go`). 4 is every governed type at once -- full concurrency for this capability. **Trade-off:** defaults to 1 (sequential) because POCRA's failure mode when pushed is a 200 with an *empty* catalog, indistinguishable from "no results" -- a parallel search can silently drop a facility type with no error. Verify against the live API before raising it in production. | 1 (sequential) | + +This package's `Config` (`AgricultureFacility.go`, a flat struct of its own because it carries `searchConcurrency`, which `upstream.Config` has no field for) also has the `basic`/`header`/`query` auth credential pairs (`usernameEnv`/`passwordEnv`, `headerName`/`headerValueEnv`, `queryName`/`queryValueEnv`). This plugin's `parseConfig` does not wire them through -- POCRA needs none of them. A second provider on `openagrinet:AgricultureFacility` (see "What lives here") that needs one adds the corresponding line to `parseConfig`, mirroring `maxResponseBytes`. + +The id must also appear in the module's `steps:` list, and must be unique across +`steps` and `providerSteps` — a repeat is refused at startup, because both land +in one id-keyed map and one capability would otherwise be lost silently. + +## Registry rows + +Two, joined on `participantId`. + +```json +{ "participantId": "pocra", "name": "PoCRA Provider Aggregator", + "type": "upstream", "status": "active", + "baseUrl": "https://middleware-bap-client.mahapocra.gov.in" } +``` +```json +{ "bindingKey": "pocra|openagrinet:AgricultureFacility", + "participantId": "pocra", + "capabilityCode": "openagrinet:AgricultureFacility", + "status": "active", + "actions": [ { "action": "select", "method": "POST", "path": "/search", + "mappings": "/agriculture-facility.select.yaml", + "timeoutMs": 30000, "retryMax": 2, "status": "active" } ] } +``` + +`retryMax` is 2. The step marks a 4xx other than 429 as permanent and stops +retrying it, so a schema NACK caused by our own malformed request costs one +attempt rather than three. What the budget buys is resilience against a 5xx, a +429 or a transport failure, backing off exponentially from 50ms. + +## Facility types + +The governed enum maps one-to-one onto POCRA's category codes. The translation +lives in the mapping and nowhere else. + +| `FacilityType` | POCRA code | +|---|---| +| `CustomHiringCentre` | `chc` | +| `KrishiVigyanKendra` | `kvk` | +| `Warehouse` | `warehouse` | +| `SoilTestingFacility` | `soil_lab` | + +Adding a type is three lines in +`config/mappings/pocra/agriculture-facility.select.yaml` — the forward table in +the request half, the inverse in the response half, and the governed value in the +precondition's list. No rebuild. + +## Where the query lives + +An inbound query resource is `informationMode: OnDemand`. The search origin is +read from `message.contract.commitments[].fulfillment.stops[].location.geo` +and the requested type from `resourceAttributes.supportedFacilityTypes`, +rather than from `location`/`address`/`facilityType` on `resourceAttributes` -- +a convention this plugin keeps, not a schema requirement. The pack forbade +those fields under OnDemand until `network-specs` commit `b76c9ad8a5` on +`schema-packs-v0.1` dropped that constraint (see +`dev_docs/schema-onDemand-forbid-removed.md`); the plugin's behavior did not +change when that happened, since POCRA has no verified per-facility +coordinate to put there anyway. + +**This convention is provisional.** It was chosen on design and has not been +confirmed against a payload captured from the network. + +## What the answer deliberately omits + +`location`, because POCRA returns no verified per-facility coordinate — the one +`gps` in its response is a fixed stub unrelated to the point that was asked for — +and the pack forbids substituting the search origin. + +`services`, `capacity`, `website` and `lastUpdatedAt`, because POCRA supplies +none of them. Deriving services from the facility type, or `lastUpdatedAt` from +the time of the fetch, would assert something nobody verified. + +Distance, which POCRA does supply, is used to order the resources nearest-first +and then dropped: the pack states that query-relative distance is not an +intrinsic facility attribute and belongs in result metadata. + +`"Unknown"`, `"N/A"`, `"000000"` and `"-"` are treated as absent wherever POCRA +sends them, so a field is omitted rather than published as a placeholder. The +warehouse BPP uses `"-"` for a phone it does not have; the other three use +`"N/A"`. + +`@context` is echoed from the request rather than restated here, so this mapping +does not have to know which pack identifier is current and cannot contradict +what the caller declared. Which matters: the identifier the packs name in their +own `x-jsonld`, `https://schemas.openagrinet.global/…`, has no DNS record, so a +deployment tracking the published ref sends the `raw.githubusercontent.com` pack +URL instead. Both are exercised. + +## Testing + +```sh +go test ./pkg/plugin/implementation/AgricultureFacility/... +``` + +25 pass, 2 skip. The two skips are live tests against the real POCRA API, opted +into with `POCRA_LIVE=1`. + +`mappings_test.go` runs the shipped mapping through the real mapper and the real +step against a fake POCRA. It reads from `config/mappings/pocra/` rather than +from a fixture, so it breaks when what is deployed breaks. + +`conformance_test.go` validates the answer against +`openagrinet:AgricultureFacility v0.1` with a real JSON Schema validator. It +also validates the pack's own published examples — if those fail, the +compilation is wrong and nothing else in that file means anything — and covers +the two things the schema is silent on: fields the pack does not declare, and +the README's prose mapping rules. + +The schemas are not vendored. `schemacache_test.go` compiles the pack under the +URL that publishes it, + + https://github.com/OpenAgriNet/network-specs/tree/schema-packs-v0.1/schema/AgricultureFacility/v0.1 + +lets the validator resolve the pack's own `$ref`s, and caches each document +under `testdata/schema-cache/` (gitignored). A cold run needs the network; every +run after it is offline. With an empty cache **and** no network the schema tests +skip rather than fail. To refresh, delete the cache directory and re-run. diff --git a/pkg/plugin/implementation/AgricultureFacility/cmd/plugin.go b/pkg/plugin/implementation/AgricultureFacility/cmd/plugin.go new file mode 100644 index 00000000..25a13ef3 --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/cmd/plugin.go @@ -0,0 +1,120 @@ +// Command plugin builds the agriculture facility provider step as a loadable +// plugin. +// +// The filename of the built .so is the id a deployment names in providerSteps, +// so this package is AgricultureFacility's whole public surface: a config map +// in, a step out. +package main + +import ( + "context" + "errors" + "fmt" + "strconv" + "strings" + + "github.com/beckn-one/beckn-onix/pkg/log" + "github.com/beckn-one/beckn-onix/pkg/plugin/definition" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/AgricultureFacility" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" +) + +// agriFacilityProvider implements definition.ProviderStepProvider. +type agriFacilityProvider struct{} + +// newStepFunc creates a new step. Indirected for tests. +var newStepFunc = AgricultureFacility.New + +// parseConfig turns the plugin config map into a typed Config. Anything absent +// is left zero: AgricultureFacility.New applies the defaults and validates the auth +// scheme, so those rules live in one place. +func (p agriFacilityProvider) parseConfig(config map[string]string) (*AgricultureFacility.Config, error) { + cfg := &AgricultureFacility.Config{ + BindingKeys: splitList(config["bindingKeys"]), + // Absent means the Beckn v2 convention. See common.Config for why + // this is a default rather than something to set. + ProviderIDAt: config["providerIdAt"], + CapabilityCodeAt: config["capabilityCodeAt"], + } + + // One credential profile per provider, read from the flattened + // authScheme- settings. Shared with the other capability + // plugins so a scheme added in one place is not missed in three. + auth, err := common.ParseProviderAuth(config) + if err != nil { + return nil, err + } + cfg.AuthByProvider = auth + + if raw, exists := config["maxResponseBytes"]; exists && raw != "" { + value, err := strconv.ParseInt(raw, 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid maxResponseBytes value '%s': %w", raw, err) + } + if value <= 0 { + return nil, fmt.Errorf("maxResponseBytes must be positive, got %d", value) + } + cfg.MaxResponseBytes = value + } + + if raw, exists := config["searchConcurrency"]; exists && raw != "" { + value, err := strconv.Atoi(raw) + if err != nil { + return nil, fmt.Errorf("invalid searchConcurrency value '%s': %w", raw, err) + } + if value <= 0 { + return nil, fmt.Errorf("searchConcurrency must be positive, got %d", value) + } + cfg.SearchConcurrency = value + } + + return cfg, nil +} + +// New creates a new agriculture facility provider step instance. +func (p agriFacilityProvider) New(ctx context.Context, registry definition.ProviderRecordLookup, mapper definition.Mapper, config map[string]string) (definition.Step, func() error, error) { + if ctx == nil { + return nil, nil, errors.New("context cannot be nil") + } + + cfg, err := p.parseConfig(config) + if err != nil { + log.Errorf(ctx, err, "Failed to parse agriculture facility configuration") + return nil, nil, fmt.Errorf("failed to parse agriculture facility configuration: %w", err) + } + + step, closer, err := newStepFunc(ctx, registry, mapper, cfg) + if err != nil { + log.Errorf(ctx, err, "Failed to create agriculture facility step") + return nil, nil, err + } + + log.Infof(ctx, "Agriculture facility step created successfully") + return step, closer, nil +} + +// splitList reads a comma-separated config value, which is how a list reaches a +// plugin -- the config is map[string]string. Blanks are dropped and spaces +// trimmed, so a trailing comma or a wrapped line is not a config error. +// +// A comma is unambiguous here: a binding key separates its own halves with a +// pipe. +func splitList(raw string) []string { + if strings.TrimSpace(raw) == "" { + return nil + } + var out []string + for _, part := range strings.Split(raw, ",") { + if trimmed := strings.TrimSpace(part); trimmed != "" { + out = append(out, trimmed) + } + } + return out +} + +// Provider is the exported plugin instance. +var Provider = agriFacilityProvider{} + +// Compile-time proof the provider satisfies the interface the manager asserts +// against. A mismatch is otherwise a runtime cast failure at startup. +var _ definition.ProviderStepProvider = Provider diff --git a/pkg/plugin/implementation/AgricultureFacility/cmd/plugin_test.go b/pkg/plugin/implementation/AgricultureFacility/cmd/plugin_test.go new file mode 100644 index 00000000..7b1e733a --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/cmd/plugin_test.go @@ -0,0 +1,199 @@ +package main + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" + + "github.com/beckn-one/beckn-onix/pkg/model" + "github.com/beckn-one/beckn-onix/pkg/plugin/definition" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/AgricultureFacility" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" +) + +type stubRegistry struct{} + +func (stubRegistry) ProviderRecord(context.Context, string) (*model.ProviderRecord, error) { + return nil, nil +} + +type stubMapper struct{} + +func (stubMapper) Verify(context.Context, string, any) error { return nil } + +func (stubMapper) Transform(context.Context, string, definition.Direction, any) ([]byte, error) { + return nil, nil +} + +func TestParseConfig(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + config map[string]string + expected *AgricultureFacility.Config + expectedErr string + }{ + { + // Everything absent is left zero: AgricultureFacility.New defaults it, so + // the rules are defined in exactly one place. + name: "leaves everything unset for New to default", + config: map[string]string{}, + expected: &AgricultureFacility.Config{AuthByProvider: map[string]*common.AuthProfile{}}, + }, + { + // Every key this plugin reads. Auth is per provider, in settings + // named for the participant id, which pkg/plugin flattens from the + // nested block an operator writes. + name: "reads every supported setting", + config: map[string]string{ + "bindingKeys": "pocra|openagrinet:AgricultureFacility", + "providerIdAt": "who.provider", + "capabilityCodeAt": "what[].type", + "authScheme-pocra": "none", + "maxResponseBytes": "2048", + "searchConcurrency": "4", + }, + expected: &AgricultureFacility.Config{ + BindingKeys: []string{"pocra|openagrinet:AgricultureFacility"}, + ProviderIDAt: "who.provider", + CapabilityCodeAt: "what[].type", + AuthByProvider: map[string]*common.AuthProfile{ + "pocra": {Provider: "pocra", Scheme: "none"}, + }, + MaxResponseBytes: 2048, + SearchConcurrency: 4, + }, + }, + { + name: "reads several binding keys and trims them", + config: map[string]string{ + "bindingKeys": "pocra|openagrinet:AgricultureFacility, other|openagrinet:Thing ,", + }, + expected: &AgricultureFacility.Config{ + BindingKeys: []string{ + "pocra|openagrinet:AgricultureFacility", + "other|openagrinet:Thing", + }, + AuthByProvider: map[string]*common.AuthProfile{}, + }, + }, + { + name: "reads the binding key path override", + config: map[string]string{ + "providerIdAt": "who.provider", + "capabilityCodeAt": "what[].type", + }, + expected: &AgricultureFacility.Config{ + ProviderIDAt: "who.provider", + CapabilityCodeAt: "what[].type", + AuthByProvider: map[string]*common.AuthProfile{}, + }, + }, + { + name: "refuses a non-numeric maxResponseBytes", + config: map[string]string{"maxResponseBytes": "big"}, + expectedErr: "invalid maxResponseBytes", + }, + { + name: "refuses a non-positive maxResponseBytes", + config: map[string]string{"maxResponseBytes": "0"}, + expectedErr: "must be positive", + }, + { + name: "refuses a non-numeric searchConcurrency", + config: map[string]string{"searchConcurrency": "big"}, + expectedErr: "invalid searchConcurrency", + }, + { + name: "refuses a non-positive searchConcurrency", + config: map[string]string{"searchConcurrency": "0"}, + expectedErr: "must be positive", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got, err := agriFacilityProvider{}.parseConfig(tc.config) + if tc.expectedErr != "" { + if err == nil || !strings.Contains(err.Error(), tc.expectedErr) { + t.Fatalf("error = %v, want it to mention %q", err, tc.expectedErr) + } + return + } + if err != nil { + t.Fatalf("parseConfig() returned an unexpected error: %v", err) + } + if !reflect.DeepEqual(got, tc.expected) { + t.Errorf("config = %+v, want %+v", got, tc.expected) + } + }) + } +} + +func TestNewRefusesANilContext(t *testing.T) { + t.Parallel() + + step, closer, err := agriFacilityProvider{}.New(nil, stubRegistry{}, stubMapper{}, map[string]string{}) //nolint:staticcheck // passing nil is what this asserts is refused + if err == nil { + t.Fatal("expected a nil context to be refused") + } + if step != nil || closer != nil { + t.Error("a refused construction must return neither a step nor a closer") + } +} + +func TestNewPropagatesAConstructionFailure(t *testing.T) { + original := newStepFunc + defer func() { newStepFunc = original }() + newStepFunc = func(context.Context, definition.ProviderRecordLookup, definition.Mapper, + *AgricultureFacility.Config) (definition.Step, func() error, error) { + return nil, nil, errors.New("boom") + } + + _, _, err := agriFacilityProvider{}.New(context.Background(), stubRegistry{}, stubMapper{}, + map[string]string{"bindingKeys": "pocra|openagrinet:AgricultureFacility"}) + if err == nil || !strings.Contains(err.Error(), "boom") { + t.Fatalf("error = %v, want it to carry the construction failure", err) + } +} + +// bindingKeys has no default, because a package serving a family cannot guess +// which of them a deployment has providers for. Absent must fail at startup +// rather than leave a step that answers nothing. +func TestNewRequiresBindingKeys(t *testing.T) { + t.Parallel() + + _, _, err := agriFacilityProvider{}.New(context.Background(), stubRegistry{}, stubMapper{}, + map[string]string{}) + if err == nil || !strings.Contains(err.Error(), "bindingKeys") { + t.Fatalf("error = %v, want it to name bindingKeys", err) + } +} + +func TestNewBuildsAStep(t *testing.T) { + t.Parallel() + + step, closer, err := agriFacilityProvider{}.New(context.Background(), stubRegistry{}, stubMapper{}, + map[string]string{ + "bindingKeys": "pocra|openagrinet:AgricultureFacility", + // A served provider declares its own auth, even when it is none. + "authScheme-pocra": "none", + }) + if err != nil { + t.Fatalf("New() returned an unexpected error: %v", err) + } + if step == nil { + t.Fatal("New() returned no step") + } + if closer == nil { + t.Fatal("New() returned no closer") + } + if err := closer(); err != nil { + t.Errorf("closer() returned %v", err) + } +} diff --git a/pkg/plugin/implementation/AgricultureFacility/conformance_test.go b/pkg/plugin/implementation/AgricultureFacility/conformance_test.go new file mode 100644 index 00000000..4eaecfdc --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/conformance_test.go @@ -0,0 +1,462 @@ +package AgricultureFacility_test + +// conformance_test.go validates what the mapping produces against the +// openagrinet:AgricultureFacility v0.1 schema pack, with a real JSON Schema +// validator. +// +// Every other test in this package asserts what the pack requires by hand, from +// a reading of attributes.yaml. That is the weakest link in the suite: a +// misreading produces a test that agrees with the mistake. This one hands the +// answer and the published schema to a validator and lets it decide. +// +// The schemas are fetched from the ref they are published under and cached +// under testdata/schema-cache, so this is offline and deterministic after a +// cold run -- see schemacache_test.go for the loader and how to refresh. + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/santhosh-tekuri/jsonschema/v6" +) + +// facilitySchemaURI is the AgricultureFacility schema inside its OpenAPI +// document, addressed by the URL that actually serves it. +// +// The URI matters more than the bytes: the packs $ref each other by URL, and a +// validator resolves a ref against the base URI of the document it is written +// in. AgricultureFacility refs "../../AgricultureResource/v0.1/attributes.yaml" +// relative to its own base, so compiling under the published URL is what makes +// that resolve -- and, unlike the schemas.openagrinet.global path this used to +// be registered under, it is an address something answers. +// +// The packs are OpenAPI 3.1.1, whose schema objects are JSON Schema 2020-12, so +// a validator can compile one directly out of components.schemas. +const facilitySchemaURI = facilityPackURL + "#/components/schemas/AgricultureFacility" + +// facilitySchema compiles the pack once per test. +func facilitySchema(t *testing.T) *jsonschema.Schema { + t.Helper() + + loader := newPackLoader() + compiler := jsonschema.NewCompiler() + compiler.DefaultDraft(jsonschema.Draft2020) + compiler.UseLoader(jsonschema.SchemeURLLoader{"https": loader}) + + schema, err := compiler.Compile(facilitySchemaURI) + if err != nil { + loader.skipIfOffline(t, err) + t.Fatalf("could not compile the AgricultureFacility schema: %v", err) + } + + // Both OAN documents have to have been loaded. A compile that satisfied + // every ref without reading AgricultureResource would mean the composed + // half of the schema went unenforced, and every result below would be + // measured against half a pack. + for _, required := range []string{facilityPackURL, resourcePackURL} { + if !loader.sawLoaded(required) { + t.Fatalf("the schema compiled without loading %s -- the pack is not the one "+ + "being validated against", required) + } + } + return schema +} + +// validateFacilities checks every resource in an answer against the pack, and +// reports the validator's own explanation when one fails. +func validateFacilities(t *testing.T, schema *jsonschema.Schema, answer map[string]any) int { + t.Helper() + + resources := answerResources(t, answer) + if len(resources) == 0 { + t.Fatal("the answer carries no resources to validate") + } + for _, entry := range resources { + id, _ := dig(entry, "id").(string) + attributes := dig(entry, "resourceAttributes") + if attributes == nil { + t.Fatalf("resource %s carries no resourceAttributes", id) + } + if err := schema.Validate(attributes); err != nil { + // The validator's message names the failing keyword and the + // instance location, which is the whole reason for this test. + t.Errorf("resource %s does not satisfy AgricultureFacility v0.1:\n%v\n%s", + id, err, mustIndent(t, attributes)) + } + } + return len(resources) +} + +func TestAnswerSatisfiesTheSchemaPack(t *testing.T) { + schema := facilitySchema(t) + + for _, tc := range []struct { + name string + facilityType string + provider string + }{ + // The three that carry their own category tag. + {"a KVK search", "KrishiVigyanKendra", providerResponse}, + // The one that does not, and whose type is derived from its provider's + // GSW fulfillment. This is the case that was silently omitting a + // REQUIRED field, so it is the reason this test exists. + {"a warehouse search", "Warehouse", warehouseResponse}, + // A mixed answer, so the filtered survivors are validated too. + {"a filtered mixed answer", "CustomHiringCentre", mixedResponse}, + } { + t.Run(tc.name, func(t *testing.T) { + answer := runAgainst(t, requestFor(t, tc.facilityType), tc.provider) + count := validateFacilities(t, schema, answer) + t.Logf("%d %s facilities validated against AgricultureFacility v0.1", + count, tc.facilityType) + }) + } +} + +// A resource missing facilityType must be REFUSED by the validator. Without +// this, a green suite would not distinguish "the pack is satisfied" from "the +// validator is not actually checking anything". +func TestTheValidatorRefusesAnIncompleteFacility(t *testing.T) { + schema := facilitySchema(t) + + answer := runAgainst(t, requestFor(t, "Warehouse"), warehouseResponse) + attributes, ok := dig(answerResources(t, answer)[0], "resourceAttributes").(map[string]any) + if !ok { + t.Fatal("the first resource carries no resourceAttributes") + } + if err := schema.Validate(attributes); err != nil { + t.Fatalf("the unmodified answer should validate: %v", err) + } + + for _, required := range []string{"facilityType", "informationMode", "source"} { + t.Run("without "+required, func(t *testing.T) { + broken := map[string]any{} + for key, value := range attributes { + if key != required { + broken[key] = value + } + } + if err := schema.Validate(broken); err == nil { + t.Errorf("the validator accepted a Direct resource with no %s, "+ + "so it is not enforcing the pack", required) + } + }) + } + + // And the whole point of the GSW fallback: the value has to be one the pack + // governs, not merely present. + t.Run("with an ungoverned facilityType", func(t *testing.T) { + broken := map[string]any{} + for key, value := range attributes { + broken[key] = value + } + broken["facilityType"] = "TractorShed" + if err := schema.Validate(broken); err == nil { + t.Error("the validator accepted a facilityType outside the governed enum") + } + }) +} + +func mustIndent(t *testing.T, value any) string { + t.Helper() + pretty, err := json.MarshalIndent(value, "", " ") + if err != nil { + return fmt.Sprintf("%v", value) + } + return string(pretty) +} + +// packExamples are the schema pack's own published examples, by file name. +// +// Listed rather than discovered, so a lost example is a failure rather than a +// shorter loop. Both information modes are published and they have disjoint +// required sets -- OnDemand forbids most of what Direct requires -- so the set +// is what gives the check below its reach. +var packExamples = []string{ + "custom-hiring-centre.json", + "krishi-vigyan-kendra.json", + "on-demand-facility-discovery.json", + "soil-testing-facility.json", + "warehouse.json", +} + +// The pack's OWN examples must satisfy the schema as this test compiles it. +// +// This checks the compilation rather than the mapping. Everything else here +// validates what this plugin produces, which cannot distinguish "the answer is +// right" from "the schema was compiled too loosely to reject anything". If the +// pack's published examples fail, the compilation is wrong and every other +// conformance result in this file is worth nothing. +// +// It is also a drift alarm: the examples are fetched from the same ref as the +// schemas, so one that stops validating means the pack moved under this plugin. +func TestThePacksOwnExamplesValidate(t *testing.T) { + schema := facilitySchema(t) + loader := newPackLoader() + + for _, name := range packExamples { + t.Run(name, func(t *testing.T) { + url := packExampleBase + "/" + name + instance, err := loader.Load(url) + if err != nil { + loader.skipIfOffline(t, err) + t.Fatalf("could not read %s: %v", url, err) + } + if err := schema.Validate(instance); err != nil { + t.Errorf("the pack's own example %s does not satisfy the schema as compiled here, "+ + "so the compilation is wrong:\n%v", name, err) + } + }) + } +} + +// governedFields are the property names AgricultureFacility v0.1 declares, +// across its own branch and the AgricultureResource it composes with. +// +// @context is in the list and is NOT a schema property -- it appears only under +// x-jsonld, which a validator ignores. Every one of the pack's own examples +// carries it, so it is conventional rather than governed, and it is listed here +// deliberately rather than by oversight. +var governedFields = map[string]bool{ + "@context": true, // conventional, see above + "@type": true, + "informationMode": true, + "subjectCategories": true, + "agricultureSubjects": true, + "languages": true, + "coverageAreas": true, + "supportedFacilityTypes": true, + "facilityType": true, + "location": true, + "address": true, + "services": true, + "capacity": true, + "publicContact": true, + "website": true, + "source": true, + "lastUpdatedAt": true, +} + +// Nothing this mapping emits may be a field the pack does not declare. +// +// The validator cannot check this. Neither AgricultureFacility nor +// AgricultureResource sets additionalProperties: false at the top level, so an +// invented field passes validation in silence -- and publishing an ungoverned +// field under a governed schema is how a private convention leaks onto the +// network and becomes load-bearing for somebody else. +// +// The nested types DO forbid extras -- SourceReference, Address, +// FacilityCapacity, Contact and Descriptor all set additionalProperties: false +// -- so the validator already covers everything inside source, address, +// capacity and publicContact. This test covers the one level it cannot. +func TestTheAnswerInventsNoUngovernedField(t *testing.T) { + for _, tc := range []struct { + name string + facilityType string + provider string + }{ + {"a KVK search", "KrishiVigyanKendra", providerResponse}, + {"a warehouse search", "Warehouse", warehouseResponse}, + {"a filtered mixed answer", "CustomHiringCentre", mixedResponse}, + } { + t.Run(tc.name, func(t *testing.T) { + answer := runAgainst(t, requestFor(t, tc.facilityType), tc.provider) + for _, entry := range answerResources(t, answer) { + id, _ := dig(entry, "id").(string) + attributes, ok := dig(entry, "resourceAttributes").(map[string]any) + if !ok { + t.Fatalf("resource %s carries no resourceAttributes", id) + } + for field := range attributes { + if !governedFields[field] { + t.Errorf("%s carries %q, which AgricultureFacility v0.1 does not declare. "+ + "The top level sets no additionalProperties, so the validator will not "+ + "catch it -- either the pack governs it and this list is stale, or the "+ + "mapping is inventing a field", id, field) + } + } + } + }) + } +} + +// The list above has to match the pack, or the test above proves nothing. +// +// Read straight out of the vendored schema rather than trusted: a field added +// upstream should widen what the mapping may emit, and a field removed should +// narrow it, without either going unnoticed. +func TestTheGovernedFieldListMatchesThePack(t *testing.T) { + loader := newPackLoader() + + declared := map[string]bool{"@context": true} // conventional, not a property + for _, section := range []struct{ url, schema, until string }{ + {facilityPackURL, " AgricultureFacility:", " FacilityType:"}, + {resourcePackURL, " AgricultureResource:", " AdministrativeAreaReference:"}, + } { + // Read as text, not as a compiled schema: the point is which property + // names the document declares, and a compiled schema has already + // flattened its allOf branches together. + raw, err := loader.text(section.url) + if err != nil { + loader.skipIfOffline(t, err) + t.Fatalf("could not read %s: %v", section.url, err) + } + + text := string(raw) + start := strings.Index(text, section.schema) + end := strings.Index(text, section.until) + if start < 0 || end < 0 || end < start { + t.Fatalf("could not locate %s in %s", section.schema, section.url) + } + for _, name := range propertyNames(text[start:end]) { + declared[name] = true + } + } + + for field := range governedFields { + if !declared[field] { + t.Errorf("governedFields lists %q, which the pack does not declare", field) + } + } + for field := range declared { + if !governedFields[field] { + t.Errorf("the pack declares %q, which governedFields omits -- the mapping is "+ + "permitted less than the pack allows", field) + } + } +} + +// propertyNames pulls the property names out of a schema section's properties +// blocks, keyed off indentation so nested object properties are not mistaken +// for top-level ones. +func propertyNames(section string) []string { + var names []string + lines := strings.Split(section, "\n") + for index, line := range lines { + trimmed := strings.TrimSpace(line) + if trimmed != "properties:" { + continue + } + indent := len(line) - len(strings.TrimLeft(line, " ")) + for _, next := range lines[index+1:] { + if strings.TrimSpace(next) == "" { + continue + } + nextIndent := len(next) - len(strings.TrimLeft(next, " ")) + if nextIndent <= indent { + break + } + if nextIndent != indent+2 { + continue + } + name := strings.TrimSuffix(strings.TrimSpace(next), ":") + name = strings.Trim(name, `"`) + if name != "" && !strings.Contains(name, " ") { + names = append(names, name) + } + } + } + return names +} + +// The INBOUND query resource must satisfy the pack too, in OnDemand mode. +// +// Every other conformance test here validates the answer. Nothing validated +// the request, and the request is where this plugin made its most debatable +// choice: the search origin is read from the Beckn fulfillment stop rather +// than from resourceAttributes, because an OnDemand resource used to be +// schema-forbidden from carrying location, address or facilityType there. +// +// That forbid clause is gone as of network-specs commit b76c9ad8a5 on +// schema-packs-v0.1 (see dev_docs/schema-onDemand-forbid-removed.md) -- +// OnDemand now permits, but does not require, all of those fields. This test +// used to assert the forbid; it no longer can, because the schema no longer +// enforces it. The convention -- origin in the fulfillment stop, not in +// resourceAttributes -- is kept anyway, as a choice rather than a schema +// requirement, so what is left to check is only that the fixture is still a +// valid OnDemand AgricultureFacility at all. +// +// If the fixture the whole suite is built on is not a valid OnDemand +// AgricultureFacility, then the convention is wrong and every test that uses it +// is testing the wrong shape. +func TestTheRequestResourceSatisfiesOnDemandMode(t *testing.T) { + schema := facilitySchema(t) + + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes := attributes(t, payload) + + if attributes["informationMode"] != "OnDemand" { + t.Fatalf("informationMode = %v, want OnDemand for a query resource", + attributes["informationMode"]) + } + if err := schema.Validate(attributes); err != nil { + t.Fatalf("the request resource does not satisfy AgricultureFacility v0.1 in OnDemand mode:\n%v\n%s", + err, mustIndent(t, attributes)) + } +} + +// The pack's README carries two mapping rules a validator cannot enforce, and +// both bind this plugin directly. They are asserted here because the schema is +// silent on them: an answer that breaks either still validates. +// +// "An adapter must not copy a request coordinate into location unless the +// Provider confirms that the returned coordinate belongs to the facility." +// +// "Query-relative distance, ranking, and price are not intrinsic facility +// attributes. Distance belongs in result metadata. Price and booking terms +// belong in the applicable Beckn offer or transaction contract." +// +// POCRA supplies material for every one of these -- a stub gps, a distance +// string, and a price and rating on warehouse items -- so each is a live +// temptation rather than a hypothetical. +func TestTheAnswerFollowsThePacksMappingRules(t *testing.T) { + for _, tc := range []struct { + name string + facilityType string + provider string + }{ + {"a KVK search", "KrishiVigyanKendra", providerResponse}, + // The warehouse fixture is the one carrying price and rating. + {"a warehouse search", "Warehouse", warehouseResponse}, + } { + t.Run(tc.name, func(t *testing.T) { + answer := runAgainst(t, requestFor(t, tc.facilityType), tc.provider) + + for _, entry := range answerResources(t, answer) { + id, _ := dig(entry, "id").(string) + attributes, ok := dig(entry, "resourceAttributes").(map[string]any) + if !ok { + t.Fatalf("resource %s carries no resourceAttributes", id) + } + + // No coordinate. POCRA publishes one gps per answer and it is a + // fixed stub unrelated to the point asked for, so there is no + // verified facility geometry to carry -- and the search origin + // must not be substituted for one. + if _, present := attributes["location"]; present { + t.Errorf("%s carries location; the pack forbids substituting the search origin "+ + "and POCRA confirms no facility coordinate", id) + } + + // No price, no rating, no distance, under any spelling. + encoded, err := json.Marshal(attributes) + if err != nil { + t.Fatalf("could not re-encode %s: %v", id, err) + } + for _, forbidden := range []string{"price", "rating", "distance", + "estimated_value", "minimum_value", "Km", " km"} { + if strings.Contains(string(encoded), forbidden) { + t.Errorf("%s carries %q; query-relative distance, ranking and price are not "+ + "facility attributes and belong in the Beckn offer or in result metadata", + id, forbidden) + } + } + } + }) + } +} diff --git a/pkg/plugin/implementation/AgricultureFacility/live_test.go b/pkg/plugin/implementation/AgricultureFacility/live_test.go new file mode 100644 index 00000000..90534d7e --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/live_test.go @@ -0,0 +1,399 @@ +package AgricultureFacility_test + +// live_test.go runs the shipped mapping against the REAL POCRA API. +// +// Skipped unless POCRA_LIVE=1, because it needs the public internet and a +// government API that takes about eight seconds to answer. Everything it covers +// that can be covered offline is covered offline in mappings_test.go; what only +// this can prove is that the request this mapping builds is one POCRA actually +// accepts, and that the answer it actually returns still maps. +// +// POCRA_LIVE=1 go test ./pkg/plugin/implementation/AgricultureFacility/ -run TestLive -v +// +// It was worth writing. The captured sample in the design notes covered kvk +// alone, and running it against all four types is what found that a warehouse +// comes from a different BPP with no category tag on its items. + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/beckn-one/beckn-onix/pkg/model" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/AgricultureFacility" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common/util" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/jsonmapper" +) + +const pocraBaseURL = "https://middleware-bap-client.mahapocra.gov.in" + +// governedTypes is every facility type the pack defines. All four are exercised, +// because they do not all come from the same upstream BPP. +var governedTypes = []string{ + "KrishiVigyanKendra", + "CustomHiringCentre", + "SoilTestingFacility", + "Warehouse", +} + +// liveRequest builds a select for one facility type, with ids that have never +// been used before. +// +// The fresh transaction id matters: POCRA accumulates answers per transaction, +// so reusing one returns everything previously asked for under it as well. +func liveRequest(t *testing.T, facilityType string) string { + t.Helper() + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["supportedFacilityTypes"] = []any{facilityType} + + // UUIDs, not just unique strings. POCRA's schema constrains transaction_id + // and message_id to the uuid format, and the mapping passes the Beckn ids + // through verbatim -- so a caller sending anything else earns a 400 from + // POCRA that names the field. + beckncontext := payload["context"].(map[string]any) + beckncontext["transactionId"] = uuid.NewString() + beckncontext["messageId"] = uuid.NewString() + + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not build the request: %v", err) + } + return string(body) +} + +// recordingProxy forwards to POCRA and keeps a copy of what came back. +// +// It exists so one upstream call answers two questions at once: what POCRA +// actually returned, and what the mapping made of it. Without it a zero-facility +// answer is ambiguous -- POCRA having nothing to say and the mapping dropping +// everything look identical from the outside, and only one of those is a bug. +// +// POCRA takes about eight seconds per call, so asking it twice to disambiguate +// would double an already slow test. +func recordingProxy(t *testing.T, captured *[]byte) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + sent, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("could not read the request bound for POCRA: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } + + request, err := http.NewRequestWithContext(r.Context(), r.Method, + pocraBaseURL+r.URL.Path, bytes.NewReader(sent)) + if err != nil { + t.Errorf("could not build the request to POCRA: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } + request.Header.Set("Content-Type", "application/json") + + response, err := (&http.Client{Timeout: 60 * time.Second}).Do(request) + if err != nil { + // POCRA times out often enough that this is a normal outcome + // rather than a test failure. Passing the status through lets the + // step report it the way it would in production. + t.Logf("POCRA did not answer: %v", err) + w.WriteHeader(http.StatusBadGateway) + return + } + defer response.Body.Close() + + body, err := io.ReadAll(response.Body) + if err != nil { + t.Errorf("could not read POCRA's answer: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } + *captured = body + w.WriteHeader(response.StatusCode) + _, _ = w.Write(body) + })) +} + +// pocraItemCount counts the facilities POCRA actually returned, across every +// responses[] entry, before the mapping touches them. +func pocraItemCount(t *testing.T, body []byte) int { + t.Helper() + if len(body) == 0 { + return 0 + } + var answer struct { + Responses []struct { + Message struct { + Catalog struct { + Providers []struct { + Items []json.RawMessage `json:"items"` + } `json:"providers"` + } `json:"catalog"` + } `json:"message"` + } `json:"responses"` + } + if err := json.Unmarshal(body, &answer); err != nil { + t.Fatalf("POCRA answered with something that is not JSON: %v", err) + } + count := 0 + for _, response := range answer.Responses { + for _, provider := range response.Message.Catalog.Providers { + count += len(provider.Items) + } + } + return count +} + +func TestLiveAgainstPocra(t *testing.T) { + if os.Getenv("POCRA_LIVE") != "1" { + t.Skip("set POCRA_LIVE=1 to run this against the real POCRA API") + } + + mappings := serveMappings(t) + defer mappings.Close() + + mapper, closeMapper, err := jsonmapper.New(context.Background(), &jsonmapper.Config{}) + if err != nil { + t.Fatalf("failed to build the mapper: %v", err) + } + defer closeMapper() + + var captured []byte + proxy := recordingProxy(t, &captured) + defer proxy.Close() + + registry := &stubRegistry{plan: &model.ProviderRecord{ + BindingKey: shippedBindingKey, + ParticipantID: "pocra", + CapabilityCode: shippedCapability, + BaseURL: proxy.URL, + Actions: map[string]model.ActionPlan{ + // The same budget the registry row publishes. POCRA answers in + // about eight seconds, so 30s is the right order of magnitude and + // 15s would be uncomfortably close. + "select": {Method: http.MethodPost, Path: "/search", + Mappings: mappings.URL + "/" + shippedMapping, TimeoutMs: 30000, RetryMax: 2}, + }, + }} + + step, closeStep, err := AgricultureFacility.New(context.Background(), registry, mapper, + &AgricultureFacility.Config{ + BindingKeys: []string{shippedBindingKey}, + // Auth is per provider now; POCRA needs no credential and says so. + AuthByProvider: map[string]*common.AuthProfile{ + strings.Split(shippedBindingKey, "|")[0]: {Scheme: util.AuthSchemeNone}, + }, + }) + if err != nil { + t.Fatalf("failed to build the step: %v", err) + } + defer closeStep() + + // The published pack, compiled by a real validator. The hand-written checks + // below say what this plugin believes the pack requires; this says what it + // actually does, against data nobody curated. + schema := facilitySchema(t) + + for _, facilityType := range governedTypes { + t.Run(facilityType, func(t *testing.T) { + captured = nil + stepCtx := &model.StepContext{Context: t.Context(), Body: []byte(liveRequest(t, facilityType))} + started := time.Now() + if err := step.Run(stepCtx); err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + elapsed := time.Since(started) + + var answer map[string]any + if err := json.Unmarshal(stepCtx.ResponseBody, &answer); err != nil { + t.Fatalf("the answer is not JSON: %v", err) + } + resources := answerResources(t, answer) + offered := pocraItemCount(t, captured) + t.Logf("%s: POCRA returned %d items -> %d facilities in %s, %d bytes", + facilityType, offered, len(resources), elapsed.Round(time.Millisecond), + len(stepCtx.ResponseBody)) + + // An empty answer is only a bug when POCRA had something to say. + // Its BPPs answer with responses[] empty often enough that failing + // on it would make this test red for reasons nothing here controls + // -- chc did exactly that, minutes after returning five facilities. + if offered == 0 { + if len(resources) != 0 { + t.Errorf("POCRA returned nothing but the mapping produced %d facilities", + len(resources)) + } + t.Skipf("POCRA returned no %s facilities for this point", facilityType) + } + if len(resources) == 0 { + t.Fatalf("POCRA returned %d items and the mapping produced no facilities", offered) + } + // Deduplication only ever removes. + if len(resources) > offered { + t.Errorf("got %d facilities from %d items; the mapping invented some", + len(resources), offered) + } + + for _, entry := range resources { + id, _ := dig(entry, "id").(string) + attrs, ok := dig(entry, "resourceAttributes").(map[string]any) + if !ok { + t.Fatalf("resource %s carries no resourceAttributes", id) + } + // Direct mode's required set, which is what a consumer + // validating against the pack will refuse an answer for. + if attrs["facilityType"] != facilityType { + t.Errorf("%s: facilityType = %v, want %s", id, attrs["facilityType"], facilityType) + } + if attrs["informationMode"] != "Direct" { + t.Errorf("%s: informationMode = %v, want Direct", id, attrs["informationMode"]) + } + if attrs["source"] == nil { + t.Errorf("%s: Direct mode requires source", id) + } + if attrs["address"] == nil && attrs["location"] == nil { + t.Errorf("%s: Direct mode requires one of address or location", id) + } + if _, present := attrs["location"]; present { + t.Errorf("%s: carries location; POCRA publishes no verified facility geometry", id) + } + } + + // The real answer, against the real pack. Every offline conformance + // case is a captured body; this is the only check that runs the + // validator over data POCRA chose today. + validateFacilities(t, schema, answer) + + // Nothing POCRA uses as a placeholder may reach the network. + raw, err := json.Marshal(answer) + if err != nil { + t.Fatalf("could not re-encode the answer: %v", err) + } + for _, placeholder := range []string{"Unknown", "N/A", "000000", `"-"`} { + if strings.Contains(string(raw), placeholder) { + t.Errorf("the answer carries the placeholder %s", placeholder) + } + } + }) + } +} + +// TestLiveCategoryLeakIsFiltered provokes the real hazard and checks the mapping +// holds against it. +// +// POCRA caches per message_id for PT10M and accumulates across searches sharing +// one, so two searches with the same messageId and different categories make the +// second answer carry both. Beckn requires a fresh messageId per message, but an +// adapter cannot assume every caller obeys that -- a retry that reuses one is the +// obvious way in -- so the mapping filters to the requested type rather than +// trusting the upstream to have done it. +// +// This is the only test that proves the filter against POCRA's actual behaviour +// rather than a fixture built from it. +func TestLiveCategoryLeakIsFiltered(t *testing.T) { + if os.Getenv("POCRA_LIVE") != "1" { + t.Skip("set POCRA_LIVE=1 to run this against the real POCRA API") + } + + mappings := serveMappings(t) + defer mappings.Close() + + mapper, closeMapper, err := jsonmapper.New(context.Background(), &jsonmapper.Config{}) + if err != nil { + t.Fatalf("failed to build the mapper: %v", err) + } + defer closeMapper() + + var captured []byte + proxy := recordingProxy(t, &captured) + defer proxy.Close() + + registry := &stubRegistry{plan: &model.ProviderRecord{ + BindingKey: shippedBindingKey, BaseURL: proxy.URL, + Actions: map[string]model.ActionPlan{ + "select": {Method: http.MethodPost, Path: "/search", + Mappings: mappings.URL + "/" + shippedMapping, TimeoutMs: 30000}, + }, + }} + + step, closeStep, err := AgricultureFacility.New(context.Background(), registry, mapper, + &AgricultureFacility.Config{ + BindingKeys: []string{shippedBindingKey}, + // Auth is per provider now; POCRA needs no credential and says so. + AuthByProvider: map[string]*common.AuthProfile{ + strings.Split(shippedBindingKey, "|")[0]: {Scheme: util.AuthSchemeNone}, + }, + }) + if err != nil { + t.Fatalf("failed to build the step: %v", err) + } + defer closeStep() + + // One messageId, deliberately shared. Different transaction ids, to show it + // is the messageId the upstream keys its cache on. + shared := uuid.NewString() + + search := func(facilityType string) (raw []byte, answer map[string]any) { + t.Helper() + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["supportedFacilityTypes"] = []any{facilityType} + beckncontext := payload["context"].(map[string]any) + beckncontext["messageId"] = shared + beckncontext["transactionId"] = uuid.NewString() + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not build the request: %v", err) + } + + captured = nil + stepCtx := &model.StepContext{Context: t.Context(), Body: body} + if err := step.Run(stepCtx); err != nil { + t.Fatalf("%s: Run() returned an unexpected error: %v", facilityType, err) + } + if err := json.Unmarshal(stepCtx.ResponseBody, &answer); err != nil { + t.Fatalf("%s: the answer is not JSON: %v", facilityType, err) + } + return captured, answer + } + + search("KrishiVigyanKendra") + raw, answer := search("CustomHiringCentre") + + // Did the hazard actually happen? If POCRA has stopped accumulating there is + // nothing here to filter, and passing would say more than it knows. + if !bytes.Contains(raw, []byte(`"kvk"`)) { + t.Skip("POCRA did not leak kvk into the chc search; nothing to filter this run") + } + t.Log("POCRA leaked kvk facilities into a chc search, as expected") + + resources := answerResources(t, answer) + if len(resources) == 0 { + t.Fatal("the mapping filtered everything away") + } + for _, entry := range resources { + id, _ := dig(entry, "id").(string) + attrs, ok := dig(entry, "resourceAttributes").(map[string]any) + if !ok { + t.Fatalf("resource %s carries no resourceAttributes", id) + } + if attrs["facilityType"] != "CustomHiringCentre" { + t.Errorf("%s: facilityType = %v, want CustomHiringCentre -- a leaked facility reached the answer", + id, attrs["facilityType"]) + } + } + t.Logf("kept %d facilities, all CustomHiringCentre", len(resources)) +} diff --git a/pkg/plugin/implementation/AgricultureFacility/mappings_test.go b/pkg/plugin/implementation/AgricultureFacility/mappings_test.go new file mode 100644 index 00000000..d0966ec5 --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/mappings_test.go @@ -0,0 +1,1762 @@ +package AgricultureFacility_test + +// mappings_test.go runs the shipped agriculture facility mapping through the +// real mapper and the real provider step. It is the only test that proves the +// three pieces fit: a mapping is JSONata inside YAML fetched over HTTP, and +// nothing but running it establishes that what is published actually produces +// valid Beckn. +// +// An external test package on purpose -- it uses the plugins exactly as the +// adapter does, through their exported surface and nothing else. + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "slices" + "sort" + "strings" + "sync" + "testing" + + "github.com/google/uuid" + + "github.com/beckn-one/beckn-onix/pkg/model" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/AgricultureFacility" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common/util" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/jsonmapper" +) + +// mappingsDir is where the shipped mappings live, relative to this package. +// Reading the published file rather than a fixture is the whole point: this test +// breaks when what is deployed breaks. +const mappingsDir = "../../../../config/mappings/pocra" + +// shippedMapping is the file this binding-action publishes: one file, both +// directions. The action segment of the name must match the action the registry +// entry declares -- a mismatch would apply a correct mapping to the wrong call, +// silently. +const shippedMapping = "agriculture-facility.select.yaml" + +// shippedCapability is what the pack calls this capability, and the second half +// of the binding key the registry indexes the provider's record by. +const shippedCapability = "openagrinet:AgricultureFacility" + +const shippedBindingKey = "pocra|" + shippedCapability + +// selectRequest is a facility search in OnDemand mode. +// +// The query lives in the Beckn layer, not in resourceAttributes: the pack +// forbids an OnDemand resource from carrying location or facilityType, so the +// search origin is a fulfillment stop and the requested type is +// supportedFacilityTypes. +// +// PROVISIONAL: this convention was chosen on the design and has not yet been +// confirmed against a payload captured from the network. +// declaredContext is the @context the fixture declares, and the one the answer +// must echo back. +// +// It is the pack's published URL because that URL resolves. The identifier the +// pack names in its own x-jsonld, https://schemas.openagrinet.global/..., does +// not: the host has no DNS record, so a JSON-LD processor handed it can fetch +// nothing. Both are exercised in TestShippedMappingEchoesTheCallersContext, +// since the mapping must not care which it is given. +const declaredContext = "https://raw.githubusercontent.com/OpenAgriNet/network-specs/schema-packs-v0.1/schema/AgricultureFacility/v0.1/context.jsonld" + +// callerMessageID is the messageId selectRequest declares. Named so the +// fan-out assertion that no upstream call reuses it reads from one place rather +// than repeating the literal. +const callerMessageID = "a1b2c3d4-e5f6-4789-abcd-ef1234567890" + +const selectRequest = `{ + "context": { "version": "2.0.0", "action": "select", + "networkId": "oan-dev", + "transactionId": "f9e8d7c6-b5a4-4321-9876-543210fedcba", + "messageId": "a1b2c3d4-e5f6-4789-abcd-ef1234567890", + "timestamp": "2026-09-02T06:12:01.330Z" }, + "message": { "contract": { "commitments": [{ + "status": { "descriptor": { "code": "DRAFT", "name": "Draft" } }, + "resources": [{ + "id": "res:pocra:facility-search", + "quantity": 1, + "resourceAttributes": { + "@context": "https://raw.githubusercontent.com/OpenAgriNet/network-specs/schema-packs-v0.1/schema/AgricultureFacility/v0.1/context.jsonld", + "@type": "openagrinet:AgricultureFacility", + "informationMode": "OnDemand", + "subjectCategories": ["Facility"], + "supportedFacilityTypes": ["KrishiVigyanKendra"] + } + }], + "offer": { + "id": "offer:pocra:common-services", + "resourceIds": ["res:pocra:facility-search"], + "provider": { "id": "pocra", + "descriptor": { "code": "POCRA-01", "name": "PoCRA Provider Aggregator" } } + }, + "fulfillment": { + "stops": [{ + "location": { "geo": { "type": "Point", "coordinates": [74.5321, 19.5132] } }, + "time": { "range": { "start": "2026-09-19T08:00:00.108Z" } } + }] + } + }] } } +}` + +// providerResponse is POCRA's own shape, trimmed from a body captured against +// the live API (not checked into this repo). +// +// TWO responses[] entries, because POCRA returns one per answering BPP and the +// captured sample carried four. The second repeats COMMON-55043, which is what +// the dedupe has to collapse. +// +// Distances are deliberately out of order, and both items carry the placeholder +// values POCRA really sends: "Unknown", "N/A" and "000000". +const providerResponse = `{ + "context": { "action": "search", "version": "1.1.0" }, + "responses": [ + { + "context": { "action": "on_search", "version": "1.1.0" }, + "message": { "catalog": { + "descriptor": { "name": "Pocra Provider Aggregator Services" }, + "providers": [{ + "id": "COMMON_PROVIDER_KVK", + "fulfillments": [{ "id": "f1_kvk", "type": "Service", + "locations": { "id": "l1", "gps": "17.385,78.4867" }, + "categories": [{ "id": "c_kvk", "name": "KVK", + "descriptor": { "code": "kvk", "name": "KVK" } }] }], + "items": [ + { + "id": "COMMON-55043", + "descriptor": { "name": "Krishi Vigyan Kendra, Biloli" }, + "address": { "address": "Krishi Vigyan Kendra, Village- Sagroli, Taluka- Biloli, Distt.-Nanded", + "district": "Nanded", "region": "Unknown", "taluka": "Unknown", + "vilage": "Unknown", "pinCode": "000000" }, + "contact": { "person": "Chairman, Sanskriti Samvardhan Mandal, Sagroli", + "email": "N/A", "phone": "N/A", + "webUrl": "https://provider.mahapocra.gov.in" }, + "fulfillment_ids": ["f1_kvk"], + "category_ids": ["c_kvk"], + "tags": [{ "list": [ + { "descriptor": { "code": "distance" }, "value": "165 Km" }, + { "descriptor": { "code": "organization" }, "value": "Krishi Vigyan Kendra" }, + { "descriptor": { "code": "category" }, "value": "kvk" } ] }] + }, + { + "id": "COMMON-55007", + "descriptor": { "name": "Krishi Vigyan Kendra, Latur" }, + "address": { "address": "Krishi Vigyan Kendra, Chincholirao Wadi, MIDC, Latur-413512", + "district": "Latur", "region": "Unknown", "taluka": "Unknown", + "vilage": "Unknown", "pinCode": "000000" }, + "contact": { "person": "N/A", "email": "N/A", "phone": "N/A", + "webUrl": "https://provider.mahapocra.gov.in" }, + "fulfillment_ids": ["f1_kvk"], + "category_ids": ["c_kvk"], + "tags": [{ "list": [ + { "descriptor": { "code": "distance" }, "value": "98 Km" }, + { "descriptor": { "code": "category" }, "value": "kvk" } ] }] + } + ] + }] + } } + }, + { + "context": { "action": "on_search", "version": "1.1.0" }, + "message": { "catalog": { + "descriptor": { "name": "Pocra Provider Aggregator Services" }, + "providers": [{ + "id": "COMMON_PROVIDER_KVK", + "fulfillments": [{ "id": "f1_kvk", "type": "Service", + "locations": { "id": "l1", "gps": "17.385,78.4867" }, + "categories": [{ "id": "c_kvk", "name": "KVK", + "descriptor": { "code": "kvk", "name": "KVK" } }] }], + "items": [ + { + "id": "COMMON-55043", + "descriptor": { "name": "Krishi Vigyan Kendra, Biloli" }, + "address": { "address": "Krishi Vigyan Kendra, Village- Sagroli, Taluka- Biloli, Distt.-Nanded", + "district": "Nanded", "region": "Unknown", "taluka": "Unknown", + "vilage": "Unknown", "pinCode": "000000" }, + "contact": { "person": "Chairman, Sanskriti Samvardhan Mandal, Sagroli", + "email": "N/A", "phone": "N/A", + "webUrl": "https://provider.mahapocra.gov.in" }, + "fulfillment_ids": ["f1_kvk"], + "category_ids": ["c_kvk"], + "tags": [{ "list": [ + { "descriptor": { "code": "distance" }, "value": "165 Km" }, + { "descriptor": { "code": "category" }, "value": "kvk" } ] }] + } + ] + }] + } } + } + ] +}` + +// serveMappings publishes the shipped mapping files over HTTP, which is how the +// mapper fetches them in production. +func serveMappings(t *testing.T) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := os.ReadFile(filepath.Join(mappingsDir, filepath.Base(r.URL.Path))) + if err != nil { + t.Errorf("could not read the mapping %q: %v", r.URL.Path, err) + w.WriteHeader(http.StatusNotFound) + return + } + fmt.Fprint(w, string(body)) + })) +} + +// stubRegistry answers with the call plan the live registry holds for this +// capability. +type stubRegistry struct{ plan *model.ProviderRecord } + +func (s *stubRegistry) ProviderRecord(context.Context, string) (*model.ProviderRecord, error) { + return s.plan, nil +} + +// runSelect drives the real step over the real mapping against a fake POCRA, and +// returns the body POCRA was sent alongside the answer. +// +// It returns the error rather than failing on it, because refusing a payload is +// as much of this mapping's job as serving one. +func runSelect(t *testing.T, request string) (sent map[string]any, answer map[string]any, runErr error) { + t.Helper() + + mappings := serveMappings(t) + defer mappings.Close() + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := json.NewDecoder(r.Body).Decode(&sent); err != nil { + t.Errorf("POCRA was sent something that is not JSON: %v", err) + } + fmt.Fprint(w, providerResponse) + })) + defer upstream.Close() + + mapper, closeMapper, err := jsonmapper.New(context.Background(), &jsonmapper.Config{}) + if err != nil { + t.Fatalf("failed to build the mapper: %v", err) + } + defer closeMapper() + + registry := &stubRegistry{plan: &model.ProviderRecord{ + BindingKey: shippedBindingKey, + ParticipantID: "pocra", + CapabilityCode: shippedCapability, + BaseURL: upstream.URL, + Actions: map[string]model.ActionPlan{ + "select": {Method: http.MethodPost, Path: "/search", + Mappings: mappings.URL + "/" + shippedMapping, TimeoutMs: 30000, RetryMax: 2}, + }, + }} + + step, closeStep, err := AgricultureFacility.New(context.Background(), registry, mapper, + &AgricultureFacility.Config{ + BindingKeys: []string{shippedBindingKey}, + // Auth is per provider now; POCRA needs no credential and says so. + AuthByProvider: map[string]*common.AuthProfile{ + strings.Split(shippedBindingKey, "|")[0]: {Scheme: util.AuthSchemeNone}, + }, + }) + if err != nil { + t.Fatalf("failed to build the step: %v", err) + } + defer closeStep() + + stepCtx := &model.StepContext{Context: t.Context(), Body: []byte(request)} + runErr = step.Run(stepCtx) + if runErr != nil { + return sent, nil, runErr + } + if len(stepCtx.ResponseBody) == 0 { + t.Fatal("the step produced no answer") + } + if err := json.Unmarshal(stepCtx.ResponseBody, &answer); err != nil { + t.Fatalf("the answer is not JSON: %v\n%s", err, stepCtx.ResponseBody) + } + return sent, answer, nil +} + +// dig walks a decoded JSON object by key, returning nil at the first miss so a +// failing assertion reports the path rather than panicking. +func dig(node any, path ...string) any { + for _, key := range path { + object, ok := node.(map[string]any) + if !ok { + return nil + } + node = object[key] + } + return node +} + +// answerResources returns the resources of the single commitment in an answer. +func answerResources(t *testing.T, answer map[string]any) []any { + t.Helper() + commitments, ok := dig(answer, "message", "contract", "commitments").([]any) + if !ok || len(commitments) != 1 { + t.Fatalf("got %d commitments, want 1", len(commitments)) + } + resources, ok := dig(commitments[0], "resources").([]any) + if !ok { + t.Fatal("the quoted commitment carries no resources") + } + return resources +} + +func TestShippedMappingSendsWhatPocraExpects(t *testing.T) { + sent, _, err := runSelect(t, selectRequest) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + stops, _ := dig(sent, "message", "intent", "fulfillment", "stops").([]any) + if len(stops) != 1 { + t.Fatalf("got %d stops, want 1", len(stops)) + } + // GPS is "lat,lon" and OAN GeoJSON is [lon, lat]. Reading them the same way + // round gives a valid request for a point in the wrong place, so it fails as + // wrong data rather than as an error. + if gps := dig(stops[0], "location", "gps"); gps != "19.5132,74.5321" { + t.Errorf("gps = %v, want 19.5132,74.5321 in lat,lon order", gps) + } + if start := dig(stops[0], "time", "range", "start"); start != "2026-09-19T08:00:00.108Z" { + t.Errorf("time.range.start = %v, want the stop's own start", start) + } + + // The governed type is translated into POCRA's private vocabulary here and + // nowhere else. + if code := dig(sent, "message", "intent", "category", "descriptor", "code"); code != "kvk" { + t.Errorf("category code = %v, want kvk for KrishiVigyanKendra", code) + } + if name := dig(sent, "message", "intent", "item", "descriptor", "name"); name != "service-locations" { + t.Errorf("item name = %v, want service-locations", name) + } + + // POCRA's request contract, pinned. These are not this adapter's identity, + // which travels in the signed Authorization header. + for _, f := range []struct{ key, want string }{ + {"domain", "advisory:mh-vistaar"}, + {"action", "search"}, + {"version", "1.1.0"}, + {"bap_id", "bap.mahapocra.gov.in"}, + {"bap_uri", "https://middleware.mahapocra.gov.in/bap/"}, + } { + if got := dig(sent, "context", f.key); got != f.want { + t.Errorf("context.%s = %v, want %v", f.key, got, f.want) + } + } + + // Correlation survives, so POCRA's logs can be matched to ours. + if got := dig(sent, "context", "transaction_id"); got != "f9e8d7c6-b5a4-4321-9876-543210fedcba" { + t.Errorf("transaction_id = %v, want the one from the request", got) + } + + // Epoch seconds as a string, unlike every other timestamp in either + // protocol. A wrong shape here is one of POCRA's schema NACKs. + timestamp, ok := dig(sent, "context", "timestamp").(string) + if !ok { + t.Fatalf("context.timestamp = %v, want a string", dig(sent, "context", "timestamp")) + } + if strings.ContainsAny(timestamp, "-:") { + t.Errorf("context.timestamp = %q, want epoch seconds rather than ISO 8601", timestamp) + } + if len(timestamp) < 10 { + t.Errorf("context.timestamp = %q, want at least 10 digits of epoch seconds", timestamp) + } +} + +// commitment reaches into a decoded payload, which is how the refusal cases are +// built. +// +// Editing the fixture's TEXT instead would be a trap: removing the last member +// of an object leaves a trailing comma, and the resulting parse error would look +// like a mapping failure rather than the case under test. +func commitment(t *testing.T, payload map[string]any) map[string]any { + t.Helper() + message, ok := payload["message"].(map[string]any) + if !ok { + t.Fatal("the fixture carries no message") + } + contract, ok := message["contract"].(map[string]any) + if !ok { + t.Fatal("the fixture carries no contract") + } + commitments, ok := contract["commitments"].([]any) + if !ok || len(commitments) == 0 { + t.Fatal("the fixture carries no commitments") + } + first, ok := commitments[0].(map[string]any) + if !ok { + t.Fatal("the first commitment is not an object") + } + return first +} + +// attributes reaches the resourceAttributes of a decoded payload. +func attributes(t *testing.T, payload map[string]any) map[string]any { + t.Helper() + resources, ok := commitment(t, payload)["resources"].([]any) + if !ok || len(resources) == 0 { + t.Fatal("the fixture carries no resources") + } + resource, ok := resources[0].(map[string]any) + if !ok { + t.Fatal("the first resource is not an object") + } + found, ok := resource["resourceAttributes"].(map[string]any) + if !ok { + t.Fatal("the first resource carries no resourceAttributes") + } + return found +} + +func TestShippedMappingRefusesWhatItCannotServe(t *testing.T) { + testCases := []struct { + name string + mutate func(t *testing.T, payload map[string]any) + wants string + }{ + { + name: "no fulfillment stop at all", + mutate: func(t *testing.T, payload map[string]any) { + commitment(t, payload)["fulfillment"] = map[string]any{"stops": []any{}} + }, + wants: "Point", + }, + { + name: "a polygon rather than a point", + mutate: func(t *testing.T, payload map[string]any) { + fulfillment := commitment(t, payload)["fulfillment"].(map[string]any) + stop := fulfillment["stops"].([]any)[0].(map[string]any) + stop["location"].(map[string]any)["geo"].(map[string]any)["type"] = "Polygon" + }, + wants: "Point", + }, + { + name: "a facility type nobody governs", + mutate: func(t *testing.T, payload map[string]any) { + attributes(t, payload)["supportedFacilityTypes"] = []any{"TractorShed"} + }, + wants: "facility type", + }, + { + name: "no facility type at all", + mutate: func(t *testing.T, payload map[string]any) { + delete(attributes(t, payload), "supportedFacilityTypes") + }, + wants: "facility type", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + tc.mutate(t, payload) + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not rebuild the payload: %v", err) + } + + sent, _, runErr := runSelect(t, string(body)) + if runErr == nil { + t.Fatal("expected the mapping's precondition to refuse this payload") + } + if !strings.Contains(runErr.Error(), tc.wants) { + t.Errorf("error %q should explain the problem by mentioning %q", runErr, tc.wants) + } + // The refusal has to happen before the call. Reaching POCRA with a + // payload we already know it cannot serve wastes its budget and + // turns our own bad request into its error message. + if sent != nil { + t.Errorf("POCRA was called with %v; a precondition must refuse before the call", sent) + } + }) + } +} + +func TestShippedMappingAnswersWithBeckn(t *testing.T) { + _, answer, err := runSelect(t, selectRequest) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + if got := dig(answer, "context", "action"); got != "on_select" { + t.Errorf("action = %v, want on_select", got) + } + if got := dig(answer, "context", "transactionId"); got != "f9e8d7c6-b5a4-4321-9876-543210fedcba" { + t.Errorf("transactionId = %v, want the one from the request", got) + } + if resources := answerResources(t, answer); len(resources) == 0 { + t.Fatal("the answer carries no resources") + } +} + +// POCRA returns one responses[] entry per answering BPP, and the same facility +// appears in more than one of them. A repeated id in the answer would be a +// broken reference the moment anything resolved it, so they collapse to one. +// +// Ordering is POCRA's own ranking, which arrives only as a distance string on +// each item. It is used to sort and then dropped: the schema pack states that +// query-relative distance is not an intrinsic facility attribute. +func TestShippedMappingDedupesAndOrdersByDistance(t *testing.T) { + _, answer, err := runSelect(t, selectRequest) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + resources := answerResources(t, answer) + + // Three items arrive across two responses; COMMON-55043 appears twice. + if len(resources) != 2 { + t.Fatalf("got %d resources, want 2 -- COMMON-55043 arrives twice and must collapse", len(resources)) + } + + ids := make([]string, 0, len(resources)) + for _, entry := range resources { + id, _ := dig(entry, "id").(string) + ids = append(ids, id) + } + + // 98 Km before 165 Km. Sorting the strings rather than the numbers would put + // "165 Km" first, which nothing downstream could detect. + want := []string{"res:pocra:facility:COMMON-55007", "res:pocra:facility:COMMON-55043"} + for i, expected := range want { + if ids[i] != expected { + t.Errorf("resource %d = %q, want %q -- nearest first", i, ids[i], expected) + } + } + + // Distance must not survive into the answer in any form. + raw, err := json.Marshal(answer) + if err != nil { + t.Fatalf("could not re-encode the answer: %v", err) + } + if strings.Contains(string(raw), "165 Km") || strings.Contains(string(raw), `"distance"`) { + t.Error("the answer carries a distance; the pack says query-relative distance is not a facility attribute") + } +} + +// AgricultureFacility v0.1 Direct mode requires @type, informationMode, +// facilityType and source, plus at least one of location or address. +// +// It is address rather than location, and that is not a shortcut: POCRA returns +// no verified per-facility geometry -- the one gps in its answer is a fixed stub +// unrelated to what was asked for -- and the pack states that an adapter must +// not copy a request coordinate into location unless the provider confirms the +// coordinate belongs to the facility. +func TestShippedMappingFollowsTheAgricultureFacilityPack(t *testing.T) { + _, answer, err := runSelect(t, selectRequest) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + for _, entry := range answerResources(t, answer) { + id, _ := dig(entry, "id").(string) + attrs, ok := dig(entry, "resourceAttributes").(map[string]any) + if !ok { + t.Fatalf("resource %s carries no resourceAttributes", id) + } + + for _, f := range []struct{ key, want string }{ + {"@context", declaredContext}, + {"@type", "openagrinet:AgricultureFacility"}, + {"informationMode", "Direct"}, + {"facilityType", "KrishiVigyanKendra"}, + } { + if attrs[f.key] != f.want { + t.Errorf("%s: %s = %v, want %v", id, f.key, attrs[f.key], f.want) + } + } + + // Direct mode requires source, and anyOf location or address. + if attrs["source"] == nil { + t.Errorf("%s: Direct mode requires source", id) + } + if attrs["address"] == nil { + t.Errorf("%s: no verified geometry is available, so address is required", id) + } + if _, present := attrs["location"]; present { + t.Errorf("%s: carries location; POCRA supplies no verified facility geometry", id) + } + + // Not supplied truthfully by POCRA, so not asserted. Deriving services + // from the facility type, or lastUpdatedAt from fetch time, would be + // inventing data under a governed schema. + for _, absent := range []string{"services", "lastUpdatedAt", "capacity", "website"} { + if _, present := attrs[absent]; present { + t.Errorf("%s: carries %q, which POCRA does not supply", id, absent) + } + } + } +} + +// POCRA sends "Unknown", "N/A" and "000000" where it has no value. Publishing +// them would put junk on the network under a governed schema. +func TestShippedMappingScrubsPocraPlaceholders(t *testing.T) { + _, answer, err := runSelect(t, selectRequest) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + raw, err := json.Marshal(answer) + if err != nil { + t.Fatalf("could not re-encode the answer: %v", err) + } + for _, placeholder := range []string{"Unknown", "N/A", "000000"} { + if strings.Contains(string(raw), placeholder) { + t.Errorf("the answer carries the placeholder %q", placeholder) + } + } + + resources := answerResources(t, answer) + + // COMMON-55007 sorts first and has no usable contact person at all, so it + // must carry no publicContact rather than an empty one. + nearest, ok := dig(resources[0], "resourceAttributes").(map[string]any) + if !ok { + t.Fatal("the nearest resource carries no resourceAttributes") + } + if id := dig(resources[0], "id"); id != "res:pocra:facility:COMMON-55007" { + t.Fatalf("resource 0 = %v, want COMMON-55007", id) + } + if _, present := nearest["publicContact"]; present { + t.Error("COMMON-55007 has no usable contact, so publicContact must be absent") + } + + // COMMON-55043 does have one, carried as an organisational company name -- + // the pack restricts publicContact to contact data approved for catalog + // publication, and POCRA's person field is an office rather than a person. + farther, ok := dig(resources[1], "resourceAttributes").(map[string]any) + if !ok { + t.Fatal("the farther resource carries no resourceAttributes") + } + contact, ok := farther["publicContact"].(map[string]any) + if !ok { + t.Fatal("COMMON-55043 has a contact person and must carry publicContact") + } + if company, _ := contact["company"].(string); !strings.Contains(company, "Sanskriti") { + t.Errorf("publicContact.company = %v, want POCRA's organisational contact", contact["company"]) + } + + // The district survives scrubbing even though region, taluka and village do + // not, so the address is still useful. + address, ok := farther["address"].(map[string]any) + if !ok { + t.Fatal("COMMON-55043 carries no address") + } + if extended, _ := address["extendedAddress"].(string); !strings.Contains(extended, "Nanded") { + t.Errorf("extendedAddress = %v, want the district that survived scrubbing", address["extendedAddress"]) + } + if _, present := address["postalCode"]; present { + t.Error("pinCode was 000000, so postalCode must be absent rather than zeroed") + } + if address["addressCountry"] != "IN" || address["addressRegion"] != "Maharashtra" { + t.Errorf("address = %v, want IN / Maharashtra", address) + } + + // contact.webUrl is POCRA's own site repeated on every item, so it + // identifies the source and not the facility. + source, ok := farther["source"].(map[string]any) + if !ok { + t.Fatal("COMMON-55043 carries no source") + } + if source["sourceId"] != "pocra" { + t.Errorf("source.sourceId = %v, want pocra", source["sourceId"]) + } + if source["sourceUri"] != "https://provider.mahapocra.gov.in" { + t.Errorf("source.sourceUri = %v, want POCRA's own site", source["sourceUri"]) + } +} + +// The request names an abstract search resource and the answer returns the +// concrete facilities that satisfy it, so the offer arrives referencing an id +// that appears nowhere in the answer. Echoing the offer unchanged leaves that +// dangling reference behind. +func TestShippedMappingRewritesTheOffersReferences(t *testing.T) { + _, answer, err := runSelect(t, selectRequest) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + commitments, _ := dig(answer, "message", "contract", "commitments").([]any) + resources := answerResources(t, answer) + offer, ok := dig(commitments[0], "offer").(map[string]any) + if !ok { + t.Fatal("the quoted commitment carries no offer") + } + + returned := make([]string, 0, len(resources)) + for _, entry := range resources { + id, _ := dig(entry, "id").(string) + returned = append(returned, id) + } + + referenced, _ := offer["resourceIds"].([]any) + if len(referenced) != len(returned) { + t.Fatalf("offer.resourceIds has %d entries, want %d -- one per resource returned", + len(referenced), len(returned)) + } + for _, reference := range referenced { + if !slices.Contains(returned, reference.(string)) { + t.Errorf("offer references %v, which is not among the resources returned", reference) + } + if reference == "res:pocra:facility-search" { + t.Error("the offer still references the abstract resource the request asked for") + } + } + + // Only the references are rewritten. Everything else the request offered + // survives. + if offer["id"] != "offer:pocra:common-services" { + t.Errorf("offer id = %v, want the one the request offered", offer["id"]) + } + if dig(offer, "provider", "id") != "pocra" { + t.Errorf("offer provider = %v, want the one the request offered", offer["provider"]) + } + + // A mapping transforms a payload; it does not assert who anyone is. The two + // Uri fields in particular are only whatever the caller sent -- in a deployed + // stack a container-internal address -- so echoing them would republish + // another party's routing details as ours. + beckncontext, _ := dig(answer, "context").(map[string]any) + for _, field := range []string{"bapId", "bapUri", "bppId", "bppUri"} { + if _, present := beckncontext[field]; present { + t.Errorf("response context carries %q; a mapping must not assert identity", field) + } + } + + // The v2 status enum is DRAFT, ACTIVE and CLOSED. QUOTED reads better and is + // refused by base schema validation. + if code := dig(commitments[0], "status", "descriptor", "code"); code != "DRAFT" { + t.Errorf("status = %v, want DRAFT -- QUOTED is not in the spec's enum", code) + } + + for _, entry := range resources { + if _, present := dig(entry, "quantity").(float64); !present { + t.Errorf("resource %v carries no quantity; the spec requires one on every commitment resource", + dig(entry, "id")) + } + } +} + +// warehouseResponse is a verbatim POCRA answer to a warehouse search, captured +// on 4 Sep 2026. It is here because warehouses come from a DIFFERENT BPP than +// the other three facility types, and its items are not the same shape: +// +// - no "category" tag at all, so facilityType cannot be read off the item +// - a "capacity_estimate" tag, which the other three never carry +// - price and rating fields the others do not have +// - a fully populated address rather than "Unknown" and "000000" +// - a real email, and "-" as a phone placeholder rather than "N/A" +// +// The doc's sample covered kvk alone, which is why none of this was visible +// until the mapping was run against the live API. +const warehouseResponse = `{ + "context": { "action": "search", "version": "1.1.0" }, + "responses": [{ + "context": { "action": "on_search", "version": "1.1.0" }, + "message": { "catalog": { + "descriptor": { "name": "Warehouse Services" }, + "providers": [{ + "id": "WAREHOUSE001", + "fulfillments": [{ "id": "f1", "type": "Service", + "categories": [{ "id": "c1", "descriptor": { "code": "GSW", "name": "General Storage Warehouse" } }] }], + "items": [ + { + "id": "WARE-1433", + "descriptor": { "name": "Shrirampur Midc Warehouse", + "short_desc": "Warehouse in Shrirampur, Shrirampur" }, + "address": { "address": "Mswc, Shrirampurc Midc, Plot No. X-33 Khandala", + "district": "Ahmednagar", "region": "Pune", + "taluka": "Shrirampur", "vilage": "Shrirampur", "pinCode": "413720" }, + "contact": { "person": "Warehouse Manager", "email": "shrirampur.wh@mswc.in", + "phone": "-", "webUrl": "https://warehouse.com" }, + "price": { "currency": "INR", "value": "12" }, + "rating": "4.2", + "category_ids": ["c1"], + "fulfillment_ids": ["f1"], + "tags": [{ "list": [ + { "descriptor": { "code": "distance" }, "value": "15.2093 km" }, + { "descriptor": { "code": "capacity_estimate" }, "value": "500 tons" } ] }] + }, + { + "id": "WARE-1406", + "descriptor": { "name": "Shrirampur Warehouse" }, + "address": { "address": "Mswc, Krushi Utpanna Bazar Samiti, Market Yard, Shrirampur", + "district": "Ahmednagar", "region": "Pune", + "taluka": "Shrirampur", "vilage": "Shrirampur", "pinCode": "413709" }, + "contact": { "person": "Warehouse Manager", "email": "shrirampur.wh@mswc.in", + "phone": "02422-222735", "webUrl": "https://warehouse.com" }, + "category_ids": ["c1"], + "fulfillment_ids": ["f1"], + "tags": [{ "list": [ + { "descriptor": { "code": "distance" }, "value": "18.8434 km" }, + { "descriptor": { "code": "capacity_estimate" }, "value": "1200 tons" } ] }] + } + ] + }] + } } + }] +}` + +// warehouseRequest asks for the one facility type whose answer is shaped +// differently from the rest. +func warehouseRequest(t *testing.T) string { + t.Helper() + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["supportedFacilityTypes"] = []any{"Warehouse"} + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not rebuild the payload: %v", err) + } + return string(body) +} + +// runAgainst drives the step over a provider answering with the given body. +func runAgainst(t *testing.T, request, providerBody string) map[string]any { + t.Helper() + + mappings := serveMappings(t) + defer mappings.Close() + + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, providerBody) + })) + defer upstream.Close() + + mapper, closeMapper, err := jsonmapper.New(context.Background(), &jsonmapper.Config{}) + if err != nil { + t.Fatalf("failed to build the mapper: %v", err) + } + defer closeMapper() + + registry := &stubRegistry{plan: &model.ProviderRecord{ + BindingKey: shippedBindingKey, BaseURL: upstream.URL, + Actions: map[string]model.ActionPlan{ + "select": {Method: http.MethodPost, Path: "/search", + Mappings: mappings.URL + "/" + shippedMapping, TimeoutMs: 30000}, + }, + }} + + step, closeStep, err := AgricultureFacility.New(context.Background(), registry, mapper, + &AgricultureFacility.Config{ + BindingKeys: []string{shippedBindingKey}, + // Auth is per provider now; POCRA needs no credential and says so. + AuthByProvider: map[string]*common.AuthProfile{ + strings.Split(shippedBindingKey, "|")[0]: {Scheme: util.AuthSchemeNone}, + }, + }) + if err != nil { + t.Fatalf("failed to build the step: %v", err) + } + defer closeStep() + + stepCtx := &model.StepContext{Context: t.Context(), Body: []byte(request)} + if err := step.Run(stepCtx); err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + var answer map[string]any + if err := json.Unmarshal(stepCtx.ResponseBody, &answer); err != nil { + t.Fatalf("the answer is not JSON: %v\n%s", err, stepCtx.ResponseBody) + } + return answer +} + +// A warehouse item carries no category tag, so facilityType cannot be read off +// the item the way it is for the other three types. Direct mode REQUIRES +// facilityType, so without a fallback every warehouse answer would violate the +// pack -- signed and delivered, because nothing in the adapter validates the +// answer against the pack. +func TestShippedMappingLabelsAWarehouseWithNoCategoryTag(t *testing.T) { + answer := runAgainst(t, warehouseRequest(t), warehouseResponse) + + resources := answerResources(t, answer) + if len(resources) != 2 { + t.Fatalf("got %d resources, want 2", len(resources)) + } + for _, entry := range resources { + id, _ := dig(entry, "id").(string) + attrs, ok := dig(entry, "resourceAttributes").(map[string]any) + if !ok { + t.Fatalf("resource %s carries no resourceAttributes", id) + } + if attrs["facilityType"] != "Warehouse" { + t.Errorf("%s: facilityType = %v, want Warehouse from the requested type", + id, attrs["facilityType"]) + } + } +} + +// The pack has a home for capacity -- value, unit and an optional basis -- and a +// warehouse is the one facility type POCRA reports it for. Dropping it would +// discard the single most useful thing a warehouse search returns. +func TestShippedMappingCarriesWarehouseCapacity(t *testing.T) { + answer := runAgainst(t, warehouseRequest(t), warehouseResponse) + + first, ok := dig(answerResources(t, answer)[0], "resourceAttributes").(map[string]any) + if !ok { + t.Fatal("the nearest resource carries no resourceAttributes") + } + capacity, ok := first["capacity"].(map[string]any) + if !ok { + t.Fatalf("the nearest warehouse carries no capacity; POCRA reported one") + } + if value, _ := capacity["value"].(float64); value != 500 { + t.Errorf("capacity.value = %v, want 500 as a number", capacity["value"]) + } + if capacity["unit"] != "tons" { + t.Errorf("capacity.unit = %v, want tons", capacity["unit"]) + } +} + +// "-" is a placeholder POCRA uses for a phone it does not have, alongside the +// "N/A", "Unknown" and "000000" the other providers use. +func TestShippedMappingScrubsTheWarehousePlaceholders(t *testing.T) { + answer := runAgainst(t, warehouseRequest(t), warehouseResponse) + + raw, err := json.Marshal(answer) + if err != nil { + t.Fatalf("could not re-encode the answer: %v", err) + } + if strings.Contains(string(raw), `"-"`) { + t.Error(`the answer carries "-", which is POCRA's placeholder for an absent phone`) + } + + resources := answerResources(t, answer) + first, _ := dig(resources[0], "resourceAttributes").(map[string]any) + contact, ok := first["publicContact"].(map[string]any) + if !ok { + t.Fatal("the nearest warehouse has a real email and must carry publicContact") + } + if contact["email"] != "shrirampur.wh@mswc.in" { + t.Errorf("publicContact.email = %v, want the address POCRA published", contact["email"]) + } + if _, present := contact["phone"]; present { + t.Error(`publicContact carries a phone, but POCRA sent "-" for it`) + } + + // The second warehouse has a real phone, so it must survive. + second, _ := dig(resources[1], "resourceAttributes").(map[string]any) + secondContact, ok := second["publicContact"].(map[string]any) + if !ok { + t.Fatal("the second warehouse has a real phone and must carry publicContact") + } + if secondContact["phone"] != "02422-222735" { + t.Errorf("publicContact.phone = %v, want the number POCRA published", secondContact["phone"]) + } +} + +// emptyResponse is what POCRA returns when the BPP for a category answers with +// nothing: HTTP 200, and responses[] empty. Observed live against chc on +// 4 Sep 2026, minutes after the same request returned five facilities. +// +// It is not an error. "No facilities near you" is a legitimate answer to a +// search, and the caller has to be able to tell it apart from a failure. +const emptyResponse = `{ + "context": { "action": "search", "version": "1.1.0" }, + "responses": [] +}` + +func TestShippedMappingAnswersAnEmptySearch(t *testing.T) { + answer := runAgainst(t, selectRequest, emptyResponse) + + // The answer still has to be a well-formed on_select that correlates, or + // the caller cannot tell "none found" from "something broke". + if got := dig(answer, "context", "action"); got != "on_select" { + t.Errorf("action = %v, want on_select", got) + } + if got := dig(answer, "context", "transactionId"); got != "f9e8d7c6-b5a4-4321-9876-543210fedcba" { + t.Errorf("transactionId = %v, want the one from the request", got) + } + + commitments, ok := dig(answer, "message", "contract", "commitments").([]any) + if !ok || len(commitments) != 1 { + t.Fatalf("got %d commitments, want 1", len(commitments)) + } + + // Whatever shape it takes, it must not claim facilities it does not have, + // and the offer must not reference resources that are not there. + resources, _ := dig(commitments[0], "resources").([]any) + offer, _ := dig(commitments[0], "offer").(map[string]any) + referenced, _ := offer["resourceIds"].([]any) + if len(resources) != 0 { + t.Errorf("got %d resources for an empty search, want none", len(resources)) + } + if len(referenced) != len(resources) { + t.Errorf("offer references %d resources but the answer carries %d", + len(referenced), len(resources)) + } +} + +// mixedResponse is POCRA answering a chc search with kvk facilities in it too. +// +// This is not hypothetical. POCRA caches per message_id for PT10M and ACCUMULATES +// across searches that share one: on 4 Sep 2026, two searches with the same +// message_id and different transaction_ids returned {kvk:5} and then +// {kvk:5, chc:5}. Beckn requires a fresh messageId per message, but an adapter +// cannot assume every caller is well behaved, and a retry that reuses one is the +// obvious way to trip it. +// +// The warehouse provider is in here too, carrying no category tag on its items -- +// only its fulfillment says GSW. It is the case that makes filtering on the item +// tag alone insufficient: with a tag-or-requested-type fallback, a leaked +// warehouse would be labelled as whatever was asked for and returned as one. +// +// And two providers that are not facilities at all. POCRA fans a search out to +// every BPP on its network, so a live warehouse search came back with 20 +// responses[] holding 182 items: 5 warehouses, 132 mandi price records, 6 +// administrative hierarchy rows and 4 weather station readings. apmcMandi is +// the awkward one -- its items have no tags key at all, so every tag lookup on +// them is a miss rather than a value. +const mixedResponse = `{ + "context": { "action": "search", "version": "1.1.0" }, + "responses": [ + { + "context": { "action": "on_search" }, + "message": { "catalog": { "providers": [{ + "id": "COMMON_PROVIDER_CHC", + "fulfillments": [{ "id": "f1_chc", + "categories": [{ "id": "c_chc", "descriptor": { "code": "chc", "name": "CHC" } }] }], + "items": [{ + "id": "COMMON-185", + "descriptor": { "name": "Custom Hiring Center" }, + "address": { "address": "Gogalgaon, Rahta", "district": "Ahmednagar", + "region": "Unknown", "taluka": "Rahta", "vilage": "Gogalgaon", + "pinCode": "000000" }, + "contact": { "person": "Rajesh Thoke", "email": "N/A", "phone": "9765007429", + "webUrl": "https://provider.mahapocra.gov.in" }, + "tags": [{ "list": [ + { "descriptor": { "code": "distance" }, "value": "14 Km" }, + { "descriptor": { "code": "category" }, "value": "chc" } ] }] + }] + }] } } + }, + { + "context": { "action": "on_search" }, + "message": { "catalog": { "providers": [{ + "id": "COMMON_PROVIDER_KVK", + "fulfillments": [{ "id": "f1_kvk", + "categories": [{ "id": "c_kvk", "descriptor": { "code": "kvk", "name": "KVK" } }] }], + "items": [{ + "id": "COMMON-55031", + "descriptor": { "name": "Krishi Vigyan Kendra, Babhleshwar" }, + "address": { "address": "KVK Babhleshwar", "district": "Ahmednagar", + "region": "Unknown", "taluka": "Unknown", "vilage": "Unknown", + "pinCode": "000000" }, + "contact": { "person": "President", "email": "N/A", "phone": "N/A", + "webUrl": "https://provider.mahapocra.gov.in" }, + "tags": [{ "list": [ + { "descriptor": { "code": "distance" }, "value": "11 Km" }, + { "descriptor": { "code": "category" }, "value": "kvk" } ] }] + }] + }] } } + }, + { + "context": { "action": "on_search" }, + "message": { "catalog": { "providers": [{ + "id": "apmcMandi", + "descriptor": { "name": "Maharashtra Mandi" }, + "items": [{ + "id": "mandi-6001", + "descriptor": { "name": "Tomato" }, + "location_ids": ["loc1"], + "price": { "minimum_value": "6000", "maximum_value": "13000", "estimated_value": "9000" }, + "time": { "label": "2026-09-03" } + }] + }] } } + }, + { + "context": { "action": "on_search" }, + "message": { "catalog": { "providers": [{ + "id": "ADMIN001", + "descriptor": { "name": "Administrative Information" }, + "fulfillments": [{ "id": "f_adm", + "categories": [{ "id": "c_adm", "descriptor": { "code": "ADM", "name": "Administrative Hierarchy" } }] }], + "items": [{ + "id": "ADM-802776", + "descriptor": { "name": "Nashik (M Corp.)" }, + "address": { "address": "Nashik", "district": "Nashik", "region": "Nashik", + "taluka": "Nashik", "vilage": "Nashik (M Corp.)", "pinCode": "422001" }, + "tags": [{ "list": [ + { "descriptor": { "code": "district_code" }, "value": "516" }, + { "descriptor": { "code": "village_code" }, "value": "802776" } ] }] + }] + }] } } + }, + { + "context": { "action": "on_search" }, + "message": { "catalog": { "providers": [{ + "id": "WAREHOUSE001", + "fulfillments": [{ "id": "f1", + "categories": [{ "id": "c1", "descriptor": { "code": "GSW", "name": "General Storage Warehouse" } }] }], + "items": [{ + "id": "WARE-1433", + "descriptor": { "name": "Shrirampur Midc Warehouse" }, + "address": { "address": "Mswc Shrirampur", "district": "Ahmednagar", + "region": "Pune", "taluka": "Shrirampur", "vilage": "Shrirampur", + "pinCode": "413720" }, + "contact": { "person": "Warehouse Manager", "email": "shrirampur.wh@mswc.in", + "phone": "-", "webUrl": "https://warehouse.com" }, + "tags": [{ "list": [ + { "descriptor": { "code": "distance" }, "value": "15.2093 km" }, + { "descriptor": { "code": "capacity_estimate" }, "value": "500 tons" } ] }] + }] + }] } } + } + ] +}` + +// requestFor builds a select for one governed facility type. +func requestFor(t *testing.T, facilityType string) string { + t.Helper() + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["supportedFacilityTypes"] = []any{facilityType} + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not rebuild the payload: %v", err) + } + return string(body) +} + +// A request for one facility type must be answered with that type and nothing +// else, whatever POCRA chose to include. +func TestShippedMappingReturnsOnlyTheRequestedFacilityType(t *testing.T) { + for _, tc := range []struct { + facilityType string + wantIDs []string + }{ + {"CustomHiringCentre", []string{"res:pocra:facility:COMMON-185"}}, + {"KrishiVigyanKendra", []string{"res:pocra:facility:COMMON-55031"}}, + {"Warehouse", []string{"res:pocra:facility:WARE-1433"}}, + } { + t.Run(tc.facilityType, func(t *testing.T) { + answer := runAgainst(t, requestFor(t, tc.facilityType), mixedResponse) + resources := answerResources(t, answer) + + got := make([]string, 0, len(resources)) + for _, entry := range resources { + id, _ := dig(entry, "id").(string) + got = append(got, id) + + attrs, ok := dig(entry, "resourceAttributes").(map[string]any) + if !ok { + t.Fatalf("resource %s carries no resourceAttributes", id) + } + if attrs["facilityType"] != tc.facilityType { + t.Errorf("%s: facilityType = %v, want %s -- a leaked facility must not be relabelled", + id, attrs["facilityType"], tc.facilityType) + } + } + if !slices.Equal(got, tc.wantIDs) { + t.Errorf("returned %v, want exactly %v", got, tc.wantIDs) + } + + // Nothing from a provider that is not a facility may reach the + // answer -- not a mandi price, not an administrative code. + raw, err := json.Marshal(answer) + if err != nil { + t.Fatalf("could not re-encode the answer: %v", err) + } + for _, leak := range []string{"mandi-6001", "estimated_value", "ADM-802776", + "district_code", "802776", "Maharashtra Mandi"} { + if strings.Contains(string(raw), leak) { + t.Errorf("the answer leaked %q from a provider that is not a facility", leak) + } + } + + // The offer must not reference what was filtered out either. + commitments, _ := dig(answer, "message", "contract", "commitments").([]any) + offer, _ := dig(commitments[0], "offer").(map[string]any) + referenced, _ := offer["resourceIds"].([]any) + if len(referenced) != len(got) { + t.Errorf("offer references %d resources but the answer carries %d", + len(referenced), len(got)) + } + }) + } +} + +// The answer echoes whatever @context the request declared, rather than +// restating a pack URL of its own. +// +// Hardcoding it meant the mapping had to know which identifier is current, and +// could contradict what the caller actually sent. Both forms below are in use: +// the published pack URL, which is what resolves today and what the fixture +// declares, and the schemas.openagrinet.global identifier the packs name in +// their own x-jsonld -- which has no DNS record, so a caller cannot be assumed +// to send either one. +// +// The pack does not constrain @context -- it appears only under x-jsonld, which +// is annotation metadata a JSON Schema validator ignores -- so conformance +// cannot catch a wrong one. This test is the only thing that does. +func TestShippedMappingEchoesTheCallersContext(t *testing.T) { + for _, declared := range []string{ + declaredContext, + "https://schemas.openagrinet.global/schema/AgricultureFacility/v0.1/context.jsonld", + } { + t.Run(declared, func(t *testing.T) { + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["@context"] = declared + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not rebuild the payload: %v", err) + } + + _, answer, runErr := runSelect(t, string(body)) + if runErr != nil { + t.Fatalf("Run() returned an unexpected error: %v", runErr) + } + for _, entry := range answerResources(t, answer) { + id, _ := dig(entry, "id").(string) + got := dig(entry, "resourceAttributes", "@context") + if got != declared { + t.Errorf("%s: @context = %v, want the one the request declared (%s)", + id, got, declared) + } + } + }) + } +} + +// -------------------------------------------------------------------------- +// Fan-out: one payload, one upstream call per facility type it asks for. +// +// POCRA's search takes exactly ONE category code -- a comma-separated pair +// answers 200 with no providers, and a category array is refused with "Schema +// validation failed", both verified against the live API. So a payload naming +// two types cannot be served by one call. +// +// Before fan-out existed the request half sent supportedFacilityTypes[0] and +// the response half filtered to that same entry, so a two-type search returned +// the first type and silently dropped the rest. Nothing in the answer said a +// type had been asked for and lost, which is what made it worth finding. +// -------------------------------------------------------------------------- + +// pocraByCategory is a fake POCRA that answers each category with its own +// captured response, and records what it was asked. +// +// Routing on the category code is the whole point: a fake that returns one +// canned body whatever it is asked cannot tell a fan-out that sent the right +// codes from one that sent the same code twice. +type pocraByCategory struct { + mu sync.Mutex + byCode map[string]string + codes []string + messages []string +} + +func (p *pocraByCategory) ServeHTTP(w http.ResponseWriter, r *http.Request) { + var sent map[string]any + if err := json.NewDecoder(r.Body).Decode(&sent); err != nil { + http.Error(w, "not JSON", http.StatusBadRequest) + return + } + code, _ := dig(sent, "message", "intent", "category", "descriptor", "code").(string) + message, _ := dig(sent, "context", "message_id").(string) + + p.mu.Lock() + p.codes = append(p.codes, code) + p.messages = append(p.messages, message) + body, known := p.byCode[code] + p.mu.Unlock() + + if !known { + // A code with no fixture is a fan-out that sent something unasked for, + // and answering it with anything would hide that. + http.Error(w, "unexpected category "+code, http.StatusBadRequest) + return + } + fmt.Fprint(w, body) +} + +// asked returns the category codes POCRA was sent, sorted so an assertion does +// not depend on which concurrent call arrived first. +func (p *pocraByCategory) asked() []string { + p.mu.Lock() + defer p.mu.Unlock() + sorted := append([]string(nil), p.codes...) + sort.Strings(sorted) + return sorted +} + +// runSplitSearch runs the shipped mapping against a fake POCRA that answers per +// category, and returns the answer with the fake for inspection. +func runSplitSearch(t *testing.T, types []string, byCode map[string]string) (map[string]any, *pocraByCategory) { + t.Helper() + + mappings := serveMappings(t) + defer mappings.Close() + + pocra := &pocraByCategory{byCode: byCode} + upstream := httptest.NewServer(pocra) + defer upstream.Close() + + mapper, closeMapper, err := jsonmapper.New(context.Background(), &jsonmapper.Config{}) + if err != nil { + t.Fatalf("failed to build the mapper: %v", err) + } + defer closeMapper() + + registry := &stubRegistry{plan: &model.ProviderRecord{ + BindingKey: shippedBindingKey, BaseURL: upstream.URL, + Actions: map[string]model.ActionPlan{ + "select": {Method: http.MethodPost, Path: "/search", + Mappings: mappings.URL + "/" + shippedMapping, TimeoutMs: 30000}, + }, + }} + + step, closeStep, err := AgricultureFacility.New(context.Background(), registry, mapper, + &AgricultureFacility.Config{ + BindingKeys: []string{shippedBindingKey}, + // Auth is per provider now; POCRA needs no credential and says so. + AuthByProvider: map[string]*common.AuthProfile{ + strings.Split(shippedBindingKey, "|")[0]: {Scheme: util.AuthSchemeNone}, + }, + }) + if err != nil { + t.Fatalf("failed to build the step: %v", err) + } + defer closeStep() + + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["supportedFacilityTypes"] = toAny(types) + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not rebuild the payload: %v", err) + } + + stepCtx := &model.StepContext{Context: t.Context(), Body: body} + if err := step.Run(stepCtx); err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + var answer map[string]any + if err := json.Unmarshal(stepCtx.ResponseBody, &answer); err != nil { + t.Fatalf("the answer is not JSON: %v\n%s", err, stepCtx.ResponseBody) + } + return answer, pocra +} + +func toAny(values []string) []any { + out := make([]any, len(values)) + for index, value := range values { + out[index] = value + } + return out +} + +// typesIn counts the answer's resources by facilityType. +func typesIn(t *testing.T, answer map[string]any) map[string]int { + t.Helper() + counts := map[string]int{} + for _, entry := range answerResources(t, answer) { + got, _ := dig(entry, "resourceAttributes", "facilityType").(string) + counts[got]++ + } + return counts +} + +// The whole point: two types asked for, two types answered. +// +// This is the case that was broken. It returned KrishiVigyanKendra alone, and +// the Warehouse the caller also asked for was dropped without a word. +func TestATwoTypeSearchIsAnsweredWithBothTypes(t *testing.T) { + answer, pocra := runSplitSearch(t, + []string{"KrishiVigyanKendra", "Warehouse"}, + map[string]string{"kvk": providerResponse, "warehouse": warehouseResponse}) + + if want := []string{"kvk", "warehouse"}; !slices.Equal(pocra.asked(), want) { + t.Errorf("POCRA was asked for %v, want one call per type %v", pocra.asked(), want) + } + got := typesIn(t, answer) + if got["KrishiVigyanKendra"] == 0 || got["Warehouse"] == 0 { + t.Errorf("answer carries %v, want both requested types present", got) + } + for facilityType := range got { + if facilityType != "KrishiVigyanKendra" && facilityType != "Warehouse" { + t.Errorf("answer carries %q, which was not asked for", facilityType) + } + } +} + +// All four, because they do not all state their type the same way -- three +// carry a category tag and a COMMON_PROVIDER_ id, a warehouse carries neither. +func TestEveryGovernedTypeCanBeAskedForAtOnce(t *testing.T) { + answer, pocra := runSplitSearch(t, + []string{"KrishiVigyanKendra", "CustomHiringCentre", "SoilTestingFacility", "Warehouse"}, + map[string]string{ + "kvk": providerResponse, + "chc": mixedResponse, + "soil_lab": mixedResponse, + "warehouse": warehouseResponse, + }) + + if want := []string{"chc", "kvk", "soil_lab", "warehouse"}; !slices.Equal(pocra.asked(), want) { + t.Errorf("POCRA was asked for %v, want %v", pocra.asked(), want) + } + got := typesIn(t, answer) + // soil_lab is routed to a fixture that carries none, so three of the four + // have facilities to return. The assertion is that nothing UNASKED appears + // and that the types with data all do. + for _, want := range []string{"KrishiVigyanKendra", "CustomHiringCentre", "Warehouse"} { + if got[want] == 0 { + t.Errorf("answer carries %v, want %s present", got, want) + } + } + if got["SoilTestingFacility"] != 0 { + t.Errorf("answer carries %d SoilTestingFacility, but the fixture has none", + got["SoilTestingFacility"]) + } +} + +// A leaked facility is still dropped when fanning out. +// +// mixedResponse is POCRA answering a chc search with kvk, mandi, administrative +// and warehouse providers in it too. Asking for chc and warehouse must return +// the chc facilities and the warehouse ones -- not the kvk that leaked into the +// chc answer, and not the mandi or administrative providers, which are not +// facilities at all. +func TestASplitSearchStillDropsWhatWasNotAskedFor(t *testing.T) { + answer, _ := runSplitSearch(t, + []string{"CustomHiringCentre", "Warehouse"}, + map[string]string{"chc": mixedResponse, "warehouse": warehouseResponse}) + + got := typesIn(t, answer) + if got["KrishiVigyanKendra"] != 0 { + t.Errorf("the kvk that leaked into the chc answer was returned: %v", got) + } + for facilityType, count := range got { + if facilityType != "CustomHiringCentre" && facilityType != "Warehouse" { + t.Errorf("answer carries %d of %q, which was not asked for", count, facilityType) + } + } + if got["CustomHiringCentre"] == 0 || got["Warehouse"] == 0 { + t.Errorf("answer carries %v, want both requested types", got) + } +} + +// Each fan-out call carries its OWN message_id, and none of them is the +// caller's. +// +// POCRA keeps a message_id's answers for ten minutes and returns the union of +// everything asked for under it -- verified against the live API, where a kvk +// search under a reused id came back carrying an earlier chc search's +// facilities. Sending one id across the fan-out would make every call return +// every other call's facilities, so this is what stops the adapter causing the +// leak it also filters. +func TestEachSearchCallCarriesItsOwnRequestId(t *testing.T) { + _, pocra := runSplitSearch(t, + []string{"KrishiVigyanKendra", "Warehouse"}, + map[string]string{"kvk": providerResponse, "warehouse": warehouseResponse}) + + if len(pocra.messages) != 2 { + t.Fatalf("POCRA saw %d calls, want 2", len(pocra.messages)) + } + if pocra.messages[0] == pocra.messages[1] { + t.Errorf("both calls used message_id %q, so POCRA would blend their answers", + pocra.messages[0]) + } + for _, message := range pocra.messages { + // POCRA's schema refuses a message_id that is not a UUID: a derived one + // like "-kvk" is answered with 400 Schema validation failed. + if _, err := uuid.Parse(message); err != nil { + t.Errorf("message_id %q is not a UUID, which POCRA refuses: %v", message, err) + } + if message == callerMessageID { + t.Errorf("a call reused the caller's messageId %q", message) + } + } +} + +// An ungoverned type anywhere in the list is refused, not just in first place. +// +// The precondition used to read supportedFacilityTypes[0], so a payload naming +// a governed type first and nonsense second passed the check and then fanned +// out to a call POCRA would refuse. +func TestAnUngovernedTypeIsRefusedWhereverItAppears(t *testing.T) { + for _, types := range [][]string{ + {"NotAFacility"}, + {"KrishiVigyanKendra", "NotAFacility"}, + {"NotAFacility", "KrishiVigyanKendra"}, + } { + t.Run(strings.Join(types, "+"), func(t *testing.T) { + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["supportedFacilityTypes"] = toAny(types) + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not rebuild the payload: %v", err) + } + + _, _, runErr := runSelect(t, string(body)) + if runErr == nil { + t.Fatal("the payload was served, want it refused for naming an ungoverned type") + } + if !strings.Contains(runErr.Error(), "governed facility type") { + t.Errorf("error = %v, want the mapping's own explanation", runErr) + } + }) + } +} + +// -------------------------------------------------------------------------- +// A placeholder value must not crash the whole search. +// +// $number() throws on anything it cannot parse, and the throw is not local to +// the item carrying the bad value -- it propagates out of the whole response +// mapping. One placeholder distance or capacity_estimate on one item then +// failed a search that also carried good facilities, which is worse than +// dropping the one bad field: it drops everything. +// -------------------------------------------------------------------------- + +// pocraSingleWith builds a minimal, valid POCRA envelope around one item's +// extra tags, so a test can vary just the field it is checking. +func pocraSingleWith(providerID, categoryCode, itemID, extraTags string) string { + return `{ "responses": [ { + "context": { "action": "on_search", "version": "1.1.0" }, + "message": { "catalog": { "descriptor": { "name": "x" }, + "providers": [{ + "id": "` + providerID + `", + "fulfillments": [{ "id": "f1", "categories": [{ "descriptor": { "code": "` + categoryCode + `" } }] }], + "items": [ + { "id": "` + itemID + `", "descriptor": {"name":"a"}, "address": {}, "contact": {}, + "fulfillment_ids": ["f1"], "category_ids": ["c1"], + "tags": [{ "list": [` + extraTags + `] }] } + ] + }] + } } + } ] }` +} + +// A placeholder distance is dropped from ranking, not fatal to the search. +// +// POCRA reports "165 Km" when it knows the distance and "Unknown" -- among the +// same placeholders this file already strips elsewhere -- when it does not. +// TestReadingTreatsAnAbsentDistanceAsUnranked (below, existing) covers the +// field being missing outright; this covers it being PRESENT but unparseable, +// which used to throw D3030 rather than return the resource unranked. +func TestAPlaceholderDistanceDoesNotFailTheSearch(t *testing.T) { + body := `{ "responses": [ { + "context": { "action": "on_search", "version": "1.1.0" }, + "message": { "catalog": { "descriptor": { "name": "x" }, + "providers": [{ + "id": "COMMON_PROVIDER_KVK", + "fulfillments": [{ "id": "f1", "categories": [{ "descriptor": { "code": "kvk" } }] }], + "items": [ + { "id": "NEAR", "descriptor": {"name":"a"}, "address": {}, "contact": {}, + "fulfillment_ids": ["f1"], "category_ids": ["c1"], + "tags": [{ "list": [{ "descriptor": {"code":"distance"}, "value": "12 Km" }] }] }, + { "id": "PLACEHOLDER", "descriptor": {"name":"b"}, "address": {}, "contact": {}, + "fulfillment_ids": ["f1"], "category_ids": ["c1"], + "tags": [{ "list": [{ "descriptor": {"code":"distance"}, "value": "Unknown" }] }] } + ] + }] + } } + } ] }` + + answer := runAgainst(t, requestFor(t, "KrishiVigyanKendra"), body) + resources := answerResources(t, answer) + if len(resources) != 2 { + t.Fatalf("got %d resources, want 2 -- a placeholder distance must not drop the item, "+ + "only its ranking", len(resources)) + } + // The good reading sorts first; the placeholder, like an absent distance, + // sorts last. + if got := dig(resources[0], "id"); got != "res:pocra:facility:NEAR" { + t.Errorf("resources[0] = %v, want NEAR (the one with a real distance) first", got) + } + if got := dig(resources[1], "id"); got != "res:pocra:facility:PLACEHOLDER" { + t.Errorf("resources[1] = %v, want PLACEHOLDER last, same as an absent distance", got) + } +} + +// A non-numeric distance with no space -- "12Km" rather than "12 Km" -- is the +// same failure by a different route: $substringBefore returns the WHOLE string +// when there is no separator, so "12Km" reached $number unparsed. +func TestADistanceWithNoSpaceDoesNotFailTheSearch(t *testing.T) { + body := pocraSingleWith("COMMON_PROVIDER_KVK", "kvk", "NOSPACE", + `{ "descriptor": {"code":"distance"}, "value": "12Km" }`) + answer := runAgainst(t, requestFor(t, "KrishiVigyanKendra"), body) + if len(answerResources(t, answer)) != 1 { + t.Fatalf("got %d resources, want 1 -- a malformed distance must not fail the search", + len(answerResources(t, answer))) + } +} + +// A placeholder capacity_estimate is omitted, not fatal, on the one facility +// type that reports it. +func TestAPlaceholderCapacityDoesNotFailTheSearch(t *testing.T) { + body := pocraSingleWith("WAREHOUSE001", "GSW", "WARE-PLACEHOLDER", + `{ "descriptor": {"code":"capacity_estimate"}, "value": "Unknown" }`) + answer := runAgainst(t, requestFor(t, "Warehouse"), body) + resources := answerResources(t, answer) + if len(resources) != 1 { + t.Fatalf("got %d resources, want 1 -- a placeholder capacity must not fail the search", + len(resources)) + } + if capacity := dig(resources[0], "resourceAttributes", "capacity"); capacity != nil { + t.Errorf("capacity = %v, want omitted for a placeholder value", capacity) + } +} + +// A real capacity still parses -- the guard added for the placeholder must not +// have broken the value it is meant to let through. +func TestARealCapacityStillParses(t *testing.T) { + body := pocraSingleWith("WAREHOUSE001", "GSW", "WARE-REAL", + `{ "descriptor": {"code":"capacity_estimate"}, "value": "500 tons" }`) + answer := runAgainst(t, requestFor(t, "Warehouse"), body) + resources := answerResources(t, answer) + if len(resources) != 1 { + t.Fatalf("got %d resources, want 1", len(resources)) + } + capacity, ok := dig(resources[0], "resourceAttributes", "capacity").(map[string]any) + if !ok { + t.Fatalf("capacity = %v, want a parsed {value, unit}", + dig(resources[0], "resourceAttributes", "capacity")) + } + if capacity["value"] != float64(500) || capacity["unit"] != "tons" { + t.Errorf("capacity = %v, want {value: 500, unit: \"tons\"}", capacity) + } +} + +// A provider naming more than one fulfillment category must not crash the +// search either. +// +// $lookup throws T0410 on anything but a scalar key, and +// provider.fulfillments.categories.descriptor.code is a SEQUENCE the moment a +// provider has more than one fulfillment or a fulfillment has more than one +// category. Every shipped fixture has exactly one of each, so nothing else in +// this suite reaches the path. +// +// The provider id here deliberately does not match COMMON_PROVIDER_*, so type +// resolution falls all the way through to the fulfillment-category source this +// is testing -- the id and item-tag sources are tried first and would mask the +// bug otherwise. +func TestAProviderWithTwoFulfillmentCategoriesDoesNotFailTheSearch(t *testing.T) { + body := `{ "responses": [ { + "context": { "action": "on_search", "version": "1.1.0" }, + "message": { "catalog": { "descriptor": { "name": "x" }, + "providers": [{ + "id": "NOT_A_COMMON_PROVIDER", + "fulfillments": [ + { "id": "f1", "categories": [{ "descriptor": { "code": "kvk" } }] }, + { "id": "f2", "categories": [{ "descriptor": { "code": "chc" } }] } + ], + "items": [ + { "id": "MIXED-1", "descriptor": {"name":"a"}, "address": {}, "contact": {}, + "fulfillment_ids": ["f1","f2"], "category_ids": ["c1"], + "tags": [{ "list": [{ "descriptor": {"code":"distance"}, "value": "12 Km" }] }] } + ] + }] + } } + } ] }` + answer := runAgainst(t, requestFor(t, "KrishiVigyanKendra"), body) + if len(answerResources(t, answer)) != 1 { + t.Fatalf("got %d resources, want 1 -- a provider with two fulfillment categories "+ + "must not fail the search", len(answerResources(t, answer))) + } +} + +// An item carrying more than one category tag must not crash the search +// either -- same T0410 hazard as the fulfillment-categories case above, but +// for $item.tags.list[descriptor.code = "category"].value. +// +// providerID is deliberately not COMMON_PROVIDER_* and categoryCode is +// deliberately ungoverned, so type resolution falls through to the item's own +// tag -- the two sources tried first would otherwise mask the bug. +func TestADuplicateCategoryTagDoesNotFailTheSearch(t *testing.T) { + body := pocraSingleWith("NOT_A_COMMON_PROVIDER", "not_governed", "DUP-CAT-1", + `{ "descriptor": {"code":"category"}, "value": "kvk" }, + { "descriptor": {"code":"category"}, "value": "chc" }`) + answer := runAgainst(t, requestFor(t, "KrishiVigyanKendra"), body) + resources := answerResources(t, answer) + if len(resources) != 1 { + t.Fatalf("got %d resources, want 1 -- a duplicated category tag must not fail the search", + len(resources)) + } + if got := dig(resources[0], "resourceAttributes", "facilityType"); got != "KrishiVigyanKendra" { + t.Errorf("facilityType = %v, want the first tag that resolves", got) + } +} + +// A duplicated distance tag must not crash the search -- same T0410 hazard as +// the placeholder-distance case above, but from two VALID values rather than +// one placeholder. +func TestADuplicateDistanceTagDoesNotFailTheSearch(t *testing.T) { + body := pocraSingleWith("COMMON_PROVIDER_KVK", "kvk", "DUP-DIST-1", + `{ "descriptor": {"code":"distance"}, "value": "12 Km" }, + { "descriptor": {"code":"distance"}, "value": "20 Km" }`) + answer := runAgainst(t, requestFor(t, "KrishiVigyanKendra"), body) + resources := answerResources(t, answer) + if len(resources) != 1 { + t.Fatalf("got %d resources, want 1 -- a duplicated distance tag must not fail the search", + len(resources)) + } +} + +// A duplicated capacity_estimate tag must not crash the search, and the value +// taken is the first, same as distance above. +func TestADuplicateCapacityTagDoesNotFailTheSearch(t *testing.T) { + body := pocraSingleWith("WAREHOUSE001", "GSW", "DUP-CAP-1", + `{ "descriptor": {"code":"capacity_estimate"}, "value": "500 tons" }, + { "descriptor": {"code":"capacity_estimate"}, "value": "800 tons" }`) + answer := runAgainst(t, requestFor(t, "Warehouse"), body) + resources := answerResources(t, answer) + if len(resources) != 1 { + t.Fatalf("got %d resources, want 1 -- a duplicated capacity_estimate tag must not fail the search", + len(resources)) + } + capacity, ok := dig(resources[0], "resourceAttributes", "capacity").(map[string]any) + if !ok { + t.Fatalf("capacity = %v, want a parsed {value, unit}", + dig(resources[0], "resourceAttributes", "capacity")) + } + if capacity["value"] != float64(500) || capacity["unit"] != "tons" { + t.Errorf("capacity = %v, want the FIRST tag's {value: 500, unit: \"tons\"}", capacity) + } +} + +// supportedFacilityTypes repeating a type is refused rather than absorbed -- +// it would otherwise pass both governed-type checks and make one duplicate +// upstream call per repeat for nothing, up to the fan-out ceiling. +func TestDuplicateFacilityTypesAreRefused(t *testing.T) { + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["supportedFacilityTypes"] = toAny([]string{"Warehouse", "Warehouse"}) + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not rebuild the payload: %v", err) + } + + _, _, runErr := runSelect(t, string(body)) + if runErr == nil { + t.Fatal("the payload was served, want it refused for repeating a facility type") + } + if !strings.Contains(runErr.Error(), "not repeat") { + t.Errorf("error = %v, want the mapping's own explanation", runErr) + } +} diff --git a/pkg/plugin/implementation/AgricultureFacility/payload.go b/pkg/plugin/implementation/AgricultureFacility/payload.go new file mode 100644 index 00000000..3f85ffba --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/payload.go @@ -0,0 +1,134 @@ +// payload.go is the one place in this repo that reads a field out of a Beckn +// payload in Go rather than in a mapping. +// +// It exists because the facility types have to be known BEFORE any mapping +// runs: they decide how many single-type payloads one inbound payload becomes, +// and each of those is then mapped and called on its own. Reading them here is +// what keeps splitting out of jsonmapper, out of pkg/plugin/definition and out +// of internal/upstream -- see search.go. +// +// The cost, stated plainly because it is the one real cost of that: this path +// is compiled in. The mapping's required: checks read the SAME path in +// JSONata, so a Beckn payload shape change touches both, and only one of them +// is a config edit. Keep the two in step; a mismatch means a payload the +// checks accept and this refuses, which reads as an adapter bug rather than as +// a bad request. +// +// Separated from the package clause by a blank line on purpose: the package's +// own doc comment is in AgricultureFacility.go, and this is a note about one +// file. + +package AgricultureFacility + +import ( + "fmt" + "slices" + + "github.com/beckn-one/beckn-onix/pkg/model" +) + +// facilityTypesFrom returns the facility types a payload asks for, in the +// order the payload wrote them -- one upstream call each. +// +// Every failure is a bad request rather than an adapter fault: the payload is +// the thing that is wrong, and the caller is the only one who can fix it. The +// mapping's own required: checks refuse most of these first, with better +// messages; these are what is left if a check is relaxed or a payload reaches +// here another way, and they exist so that path is a refusal rather than a +// panic on a nil map. +func facilityTypesFrom(beckn any) ([]string, error) { + document, ok := beckn.(map[string]any) + if !ok { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload is %T, not an object, so it names no commitment to search from", beckn)) + } + + commitments, _ := dig(document, "message", "contract", "commitments").([]any) + if len(commitments) == 0 { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload carries no commitment to read a facility search from")) + } + commitment, ok := commitments[0].(map[string]any) + if !ok { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload's first commitment is %T, not an object", commitments[0])) + } + + resources, _ := commitment["resources"].([]any) + if len(resources) == 0 { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload's commitment carries no resource to read a facility search from")) + } + resource, ok := resources[0].(map[string]any) + if !ok { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload's first resource is %T, not an object", resources[0])) + } + + declared := dig(resource, "resourceAttributes", "supportedFacilityTypes") + + // A bare string is one value: the same instruction as a one-element list, + // which is what a caller writing a single type may well send. + var declaredList []any + switch typed := declared.(type) { + case nil: + declaredList = nil + case []any: + declaredList = typed + default: + declaredList = []any{typed} + } + if len(declaredList) == 0 { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload names no facility type in supportedFacilityTypes, "+ + "so there is nothing to ask the provider for")) + } + + // Each value becomes a category code in the request half's $codes lookup, + // so a non-string is a request this capability cannot build -- refused + // here rather than sent as a null category POCRA answers with everything. + values := make([]string, 0, len(declaredList)) + for _, value := range declaredList { + text, ok := value.(string) + if !ok { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: supportedFacilityTypes contains %v (%T), which is not a facility type", + value, value)) + } + values = append(values, text) + } + + // A repeated type is refused rather than absorbed. It would otherwise pass + // every governed-type check and make one duplicate upstream call per + // repeat for nothing, up to MaxFacilityTypes. + // + // Checked here rather than left to the mapping, which states the same rule + // in its own required: block: the split happens before either half of the + // mapping runs, and each part it produces names exactly one type, so the + // mapping's version can no longer see a repeat. The wording matches so a + // caller cannot tell which guard answered. + for index, value := range values { + if slices.Contains(values[:index], value) { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: supportedFacilityTypes must not repeat a facility type, and %q appears twice", + value)) + } + } + return values, nil +} + +// dig walks a chain of object keys, returning nil the moment one is absent or +// is not an object. Written out rather than reached for from a library because +// a mistyped key must read as an absent field, which is a bad request, and not +// as a panic. +func dig(document map[string]any, keys ...string) any { + var current any = document + for _, key := range keys { + object, ok := current.(map[string]any) + if !ok { + return nil + } + current = object[key] + } + return current +} diff --git a/pkg/plugin/implementation/AgricultureFacility/payload_test.go b/pkg/plugin/implementation/AgricultureFacility/payload_test.go new file mode 100644 index 00000000..c58701d6 --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/payload_test.go @@ -0,0 +1,133 @@ +package AgricultureFacility + +// payload_test.go covers the one path read this package does in Go rather +// than in a mapping. Table-driven because the interesting cases are all +// malformed payloads, and each one has to say something a caller can act on. +// +// An INTERNAL test package, unlike every other test file here: what it tests +// is unexported, and exporting a payload path read so a test can reach it +// would publish it as something another package may depend on. + +import ( + "encoding/json" + "strings" + "testing" +) + +// decode is what Step.Run works from: the payload, already decoded into +// map[string]any / []any by encoding/json. +func decode(t *testing.T, body string) any { + t.Helper() + var beckn any + if err := json.Unmarshal([]byte(body), &beckn); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + return beckn +} + +// A payload naming three types yields three values, in the order the payload +// wrote them -- the order the answers come back in, which is the only thing +// making the result reproducible. +func TestFacilityTypesFromReadsThemInPayloadOrder(t *testing.T) { + t.Parallel() + + beckn := decode(t, `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ + "supportedFacilityTypes":["KrishiVigyanKendra","Warehouse","SoilTestingFacility"]}}]}]}}}`) + + values, err := facilityTypesFrom(beckn) + if err != nil { + t.Fatalf("facilityTypesFrom() returned an unexpected error: %v", err) + } + want := []string{"KrishiVigyanKendra", "Warehouse", "SoilTestingFacility"} + if len(values) != len(want) { + t.Fatalf("values = %v, want %v", values, want) + } + for index := range want { + if values[index] != want[index] { + t.Errorf("values[%d] = %v, want %q -- payload order, not any other", index, values[index], want[index]) + } + } +} + +// One type is one value and one call, not a special case. +func TestFacilityTypesFromAcceptsASingleType(t *testing.T) { + t.Parallel() + + beckn := decode(t, `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ + "supportedFacilityTypes":["Warehouse"]}}]}]}}}`) + + values, err := facilityTypesFrom(beckn) + if err != nil { + t.Fatalf("facilityTypesFrom() returned an unexpected error: %v", err) + } + if len(values) != 1 || values[0] != "Warehouse" { + t.Errorf("values = %v, want [Warehouse]", values) + } +} + +// A payload that carries the field as a bare string rather than a list is +// accepted as one value. The mapping's own required checks wrap it in [] to +// compare it against the governed list, so a caller that satisfies those +// checks may still have written a scalar here. +func TestFacilityTypesFromAcceptsABareString(t *testing.T) { + t.Parallel() + + beckn := decode(t, `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ + "supportedFacilityTypes":"KrishiVigyanKendra"}}]}]}}}`) + + values, err := facilityTypesFrom(beckn) + if err != nil { + t.Fatalf("facilityTypesFrom() returned an unexpected error: %v", err) + } + if len(values) != 1 || values[0] != "KrishiVigyanKendra" { + t.Errorf("values = %v, want [KrishiVigyanKendra]", values) + } +} + +// Every shape that is not a payload this capability can serve is refused with +// a message naming what was missing -- not a nil-map panic, and not an empty +// list that would silently become zero calls. +func TestFacilityTypesFromRefusesWhatItCannotRead(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name, body, wants string + }{ + {"an empty object", `{}`, "commitment"}, + {"no commitments", `{"message":{"contract":{"commitments":[]}}}`, "commitment"}, + {"no resources", `{"message":{"contract":{"commitments":[{"resources":[]}]}}}`, "resource"}, + {"no resourceAttributes", `{"message":{"contract":{"commitments":[{"resources":[{}]}]}}}`, "no facility type"}, + {"an empty list", `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ + "supportedFacilityTypes":[]}}]}]}}}`, "no facility type"}, + {"a repeated type", `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ + "supportedFacilityTypes":["Warehouse","Warehouse"]}}]}]}}}`, "not repeat"}, + {"a number instead of a type", `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ + "supportedFacilityTypes":[42]}}]}]}}}`, "not a facility type"}, + {"a nested list", `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ + "supportedFacilityTypes":[["Warehouse"]]}}]}]}}}`, "not a facility type"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + values, err := facilityTypesFrom(decode(t, tc.body)) + if err == nil { + t.Fatalf("facilityTypesFrom() returned %v, want %s refused", values, tc.name) + } + if !strings.Contains(err.Error(), tc.wants) { + t.Errorf("error = %v, want it to mention %q", err, tc.wants) + } + }) + } +} + +// A payload that is not an object at all -- the shape a decoder handed +// something odd would produce -- is refused rather than panicking. +func TestFacilityTypesFromRefusesANonObjectPayload(t *testing.T) { + t.Parallel() + + for _, beckn := range []any{nil, "a string", 42.0, []any{"a", "list"}} { + if _, err := facilityTypesFrom(beckn); err == nil { + t.Errorf("facilityTypesFrom(%v) was accepted, want it refused", beckn) + } + } +} diff --git a/pkg/plugin/implementation/AgricultureFacility/prerequisites.go b/pkg/plugin/implementation/AgricultureFacility/prerequisites.go new file mode 100644 index 00000000..ee307863 --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/prerequisites.go @@ -0,0 +1,17 @@ +package AgricultureFacility + +import "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" + +// prerequisites is what an agriculture facility capability needs that its +// payload does not carry, keyed by binding key. +// +// Empty. A facility search names the point to search from and the facility type +// it wants, and POCRA's search takes exactly those, so there is nothing to +// resolve before the call. The mapping reads both straight off the payload. +// +// An entry would be needed only for real I/O: a district name to resolve to a +// code, a token to exchange, an address to geocode. Each of those is a +// different upstream than the one this was written against, so prefer keeping +// the payload explicit over adding an entry here -- and see the note in +// WeatherObservation/prerequisites.go before doing so. +var prerequisites = common.Prerequisites{} diff --git a/pkg/plugin/implementation/AgricultureFacility/schemacache_test.go b/pkg/plugin/implementation/AgricultureFacility/schemacache_test.go new file mode 100644 index 00000000..d4421487 --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/schemacache_test.go @@ -0,0 +1,309 @@ +package AgricultureFacility_test + +// schemacache_test.go fetches the AgricultureFacility v0.1 schema pack from +// where it is actually published, and caches it under testdata/schema-cache. +// +// This package used to carry the nine schemas the pack reaches as one vendored +// 700-line multi-document YAML file, with a hand-written table naming the URI +// each document had to be registered under. Two problems with that. It was a +// transcription of the pack that could disagree with the pack and only a manual +// refresh would notice. And the base URI the table registered the OAN packs +// under -- https://schemas.openagrinet.global/... -- has no DNS record at all, +// so the one address the tests presented as the schema's identity was an +// address nothing serves. +// +// Now the validator resolves the pack's $refs itself and this loader answers +// them: once over the network, then from disk. So the base URIs are the real, +// dereferenceable ones, +// +// https://raw.githubusercontent.com/OpenAgriNet/network-specs/schema-packs-v0.1/schema/AgricultureFacility/v0.1 +// +// the relative refs the pack writes ("../../AgricultureResource/v0.1/...") +// resolve against them with no table to keep in step, and the beckn.io +// schemas -- which redirect to schema.nfh.global, and which the two sides +// disagree about the naming of -- are simply loaded under whichever name the +// ref that reached them was written with. +// +// The cache is not committed; see testdata/.gitignore. A cold run needs the +// network, and every run after it is offline and deterministic. With an empty +// cache AND no network the schema tests SKIP rather than fail, because they +// cannot be performed at all on that machine -- what they cover that does not +// need the pack is covered offline in mappings_test.go. +// +// To refresh against upstream, empty the cache and run the tests: +// +// rm -rf pkg/plugin/implementation/AgricultureFacility/testdata/schema-cache +// go test ./pkg/plugin/implementation/AgricultureFacility/ + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + neturl "net/url" + "os" + "path" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/santhosh-tekuri/jsonschema/v6" + "gopkg.in/yaml.v3" +) + +// The published pack. Pinned to the schema-packs-v0.1 ref rather than to a +// moving default branch: a suite whose verdict depends on the day it ran is not +// a conformance suite. +// +// This is a BRANCH name, not a commit -- it moves as OpenAgriNet/network-specs +// moves, deliberately, so this suite always tests against what schema-packs-v0.1 +// currently publishes rather than a snapshot that quietly falls behind. +// +// The cost of that: the branch moves under this suite, so a change upstream +// lands as a red build here on the next COLD cache -- CI always is, a +// developer's machine usually is not. Two have happened so far, and they are +// worth reading as the two shapes this takes. +// +// A constraint DROPPED. Commit b76c9ad8a5, titled "docs #5: generate composed +// schema reference pages," silently removed the `not: anyOf: [...]` clause +// that made an OnDemand resource reject facilityType/location/address/ +// services/capacity/publicContact/website/source/lastUpdatedAt. That was the +// constraint this plugin's design leans on (see the package doc and README's +// "Where the query lives"), and nothing here could restore it: the test that +// asserted the forbid now asserts only that the fixture is a valid OnDemand +// resource, and the convention is kept as a choice. See +// dev_docs/schema-onDemand-forbid-removed.md. +// +// A constraint ADDED. AgricultureResource v0.1 later made subjectCategories +// REQUIRED (it was optional), added "Facility" to its enum, and +// AgricultureFacility now requires the list to contain "Facility". That one +// this repo could and did satisfy: the mapping states +// subjectCategories: ["Facility"] on every facility it answers with, and the +// request fixture carries it too. See the mapping's own comment beside the +// field. +// +// The rule for the next one: a red cold-cache run here is a question about +// which of those two it is. If the pack asks for something this adapter can +// state, state it. If the pack dropped something this adapter relies on, that +// is not ours to silence -- say so in the test and in dev_docs. +const ( + packBase = "https://raw.githubusercontent.com/OpenAgriNet/network-specs/schema-packs-v0.1/schema" + + facilityPackURL = packBase + "/AgricultureFacility/v0.1/attributes.yaml" + resourcePackURL = packBase + "/AgricultureResource/v0.1/attributes.yaml" + + // The pack's own examples, published alongside it. + packExampleBase = packBase + "/AgricultureFacility/v0.1/examples" +) + +// schemaCacheDir mirrors each fetched URL as host/path, so what is on disk says +// where it came from without an index to consult. +const schemaCacheDir = "testdata/schema-cache" + +// fetchTimeout is per document. Generous, because a cold cache pulls a dozen +// of them. +const fetchTimeout = 30 * time.Second + +// maxSchemaSize caps what is read from a response. The largest of these +// documents is a few kilobytes; anything near this is not a schema. +const maxSchemaSize = 4 << 20 + +// packLoader answers the validator's $ref lookups, from the cache when it can +// and from the network when it must. +// +// It records what it loaded so a test can assert the schema was compiled out of +// the pack rather than out of whatever else happened to be cached, and records +// the first fetch failure so an offline machine can be told apart from a broken +// schema. +type packLoader struct { + mu sync.Mutex + loaded map[string]bool + fetchErr error +} + +func newPackLoader() *packLoader { + return &packLoader{loaded: map[string]bool{}} +} + +// Load satisfies jsonschema.URLLoader. +func (l *packLoader) Load(url string) (any, error) { + raw, err := l.text(url) + if err != nil { + return nil, err + } + return decodeSchema(url, raw) +} + +// text returns a document's bytes verbatim, which is what the governed-field +// check needs -- it reads the pack as YAML text rather than as a compiled +// schema. +func (l *packLoader) text(url string) ([]byte, error) { + cached, err := cachePath(url) + if err != nil { + return nil, err + } + if raw, err := os.ReadFile(cached); err == nil { + l.markLoaded(url) + return raw, nil + } + + raw, err := fetchSchema(url) + if err != nil { + l.mu.Lock() + if l.fetchErr == nil { + l.fetchErr = err + } + l.mu.Unlock() + return nil, err + } + if err := writeCache(cached, raw); err != nil { + return nil, err + } + l.markLoaded(url) + return raw, nil +} + +func (l *packLoader) markLoaded(url string) { + l.mu.Lock() + defer l.mu.Unlock() + l.loaded[url] = true +} + +func (l *packLoader) sawLoaded(url string) bool { + l.mu.Lock() + defer l.mu.Unlock() + return l.loaded[url] +} + +// skipIfOffline turns "the pack could not be reached and was not cached" into a +// skip, and leaves every other failure to the caller -- including a reachable +// server that answered with something other than 200, which is not offline, +// it is a finding. +// +// The distinction is the point. A schema that no longer compiles is a finding; a +// laptop on a train is not. A 404 for a pack that moved or was deleted is a +// finding too, not a train. +func (l *packLoader) skipIfOffline(t *testing.T, cause error) { + t.Helper() + + l.mu.Lock() + fetchErr := l.fetchErr + l.mu.Unlock() + if fetchErr == nil { + return + } + var status httpStatusError + if errors.As(fetchErr, &status) { + return + } + t.Skipf("the schema pack is neither cached under %s nor reachable, so conformance "+ + "cannot be checked here -- run once with network access to prime the cache.\n"+ + "fetch: %v\nfailure: %v", schemaCacheDir, fetchErr, cause) +} + +// decodeSchema converts one fetched document into the shape the validator +// takes. +// +// The packs are published as YAML and the beckn examples as JSON. YAML parses +// both, since JSON is a subset of it, and the round trip through JSON is a +// format conversion and nothing more: no key is renamed and no value is +// touched. It is there so numbers and keys arrive as the validator's own types +// rather than as yaml's. +func decodeSchema(url string, raw []byte) (any, error) { + var parsed any + if err := yaml.Unmarshal(raw, &parsed); err != nil { + return nil, fmt.Errorf("%s is not YAML or JSON: %w", url, err) + } + asJSON, err := json.Marshal(parsed) + if err != nil { + return nil, fmt.Errorf("%s could not be converted to JSON: %w", url, err) + } + return jsonschema.UnmarshalJSON(strings.NewReader(string(asJSON))) +} + +// cachePath maps a URL to its place in the cache, and refuses anything that +// would write outside it. +func cachePath(rawURL string) (string, error) { + parsed, err := neturl.Parse(rawURL) + if err != nil { + return "", fmt.Errorf("could not parse %s: %w", rawURL, err) + } + if parsed.Scheme != "https" { + return "", fmt.Errorf("refusing to load %s: the pack is fetched over https only", rawURL) + } + if parsed.Host == "" { + return "", fmt.Errorf("refusing to load %s: no host", rawURL) + } + clean := path.Clean("/" + parsed.Path) + if strings.Contains(clean, "..") || strings.ContainsAny(parsed.Host, `/\`) { + return "", fmt.Errorf("refusing to cache %s: the path escapes %s", rawURL, schemaCacheDir) + } + return filepath.Join(schemaCacheDir, parsed.Host, filepath.FromSlash(clean)), nil +} + +// httpStatusError marks a fetch that reached the server and got back +// something other than 200 -- a reachable pack that 404s or 500s, not an +// unreachable one. skipIfOffline uses the type to tell the two apart. +type httpStatusError struct { + url string + status string +} + +func (e httpStatusError) Error() string { + return fmt.Sprintf("GET %s: %s", e.url, e.status) +} + +// fetchSchema retrieves one document. Redirects are followed -- schema.beckn.io +// 301s to schema.nfh.global -- but the URL the ref was written with is what the +// document is cached and registered under, which is what makes the refs +// resolve. +func fetchSchema(url string) ([]byte, error) { + client := &http.Client{Timeout: fetchTimeout} + response, err := client.Get(url) + if err != nil { + return nil, fmt.Errorf("GET %s: %w", url, err) + } + defer response.Body.Close() + + if response.StatusCode != http.StatusOK { + return nil, httpStatusError{url: url, status: response.Status} + } + raw, err := io.ReadAll(io.LimitReader(response.Body, maxSchemaSize+1)) + if err != nil { + return nil, fmt.Errorf("reading %s: %w", url, err) + } + if len(raw) > maxSchemaSize { + return nil, fmt.Errorf("%s is larger than %d bytes, which no schema in this pack is", + url, maxSchemaSize) + } + return raw, nil +} + +// writeCache writes through a temporary file in the same directory, so an +// interrupted run leaves no half-written document behind for the next one to +// parse. +func writeCache(cached string, raw []byte) error { + if err := os.MkdirAll(filepath.Dir(cached), 0o755); err != nil { + return fmt.Errorf("could not create %s: %w", filepath.Dir(cached), err) + } + temp, err := os.CreateTemp(filepath.Dir(cached), "."+filepath.Base(cached)+".*") + if err != nil { + return fmt.Errorf("could not stage %s: %w", cached, err) + } + defer os.Remove(temp.Name()) + + if _, err := temp.Write(raw); err != nil { + temp.Close() + return fmt.Errorf("could not write %s: %w", temp.Name(), err) + } + if err := temp.Close(); err != nil { + return fmt.Errorf("could not close %s: %w", temp.Name(), err) + } + if err := os.Rename(temp.Name(), cached); err != nil { + return fmt.Errorf("could not move %s into place: %w", cached, err) + } + return nil +} diff --git a/pkg/plugin/implementation/AgricultureFacility/search.go b/pkg/plugin/implementation/AgricultureFacility/search.go new file mode 100644 index 00000000..ddcd9b41 --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/search.go @@ -0,0 +1,398 @@ +// search.go is the whole of what a multi-type facility search means, and it +// lives here because every part of it is a fact about POCRA. +// +// POCRA's search takes exactly ONE category code: a comma-separated pair +// answers 200 with no providers at all, and a category array is refused +// outright, both verified against the live API. So a payload asking for three +// facility types cannot be served by one call, however the mapping is written. +// +// What this file does about that: read the types out of the payload, split one +// inbound payload into one single-type payload per type, run the ordinary +// one-payload-one-call step over each of them concurrently, and merge the +// answers back into one. +// +// Nothing below this file knows any of that. internal/upstream serves one +// payload with one call and has no notion of splitting; jsonmapper compiles +// the two halves every mapping has and no third thing; internal/concurrent +// runs N of anything, bounded and ordered, and has never heard of Beckn. The +// mapping this runs is written for a single-type payload, which is what it is +// always handed. +// +// Separated from the package clause by a blank line on purpose: the package's +// own doc comment is in AgricultureFacility.go, and this is a note about one +// file. + +package AgricultureFacility + +import ( + "context" + "encoding/json" + "fmt" + "slices" + + "github.com/google/uuid" + + "github.com/beckn-one/beckn-onix/pkg/log" + "github.com/beckn-one/beckn-onix/pkg/model" + "github.com/beckn-one/beckn-onix/pkg/plugin/definition" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/concurrent" +) + +const ( + // MaxFacilityTypes bounds how many upstream calls one inbound payload may + // become. + // + // Splitting is amplification: one request in, N out, each with its own + // retry budget. The count comes from the PAYLOAD, so without a ceiling a + // caller decides how much work this adapter and the provider do -- and the + // provider is a government API that takes eight seconds to answer. + // + // Refused rather than clamped, unlike the registry's budgets. A clamped + // timeout still answers the question asked; a clamped search silently + // drops facility types from the answer, which is the exact defect this + // facility exists to fix. + MaxFacilityTypes = 8 + + // DefaultSearchConcurrency is how many of a split search's calls are in + // flight at once when a deployment does not say. + // + // One -- sequential. POCRA's aggregator waits a fixed window for its BPPs + // to reply and returns whatever arrived, so a slow BPP that misses the + // window is reported as no results rather than as an error. Concurrency + // makes that more likely, and its failure mode is silent data loss -- the + // exact defect this facility exists to fix. + // + // A provider that tolerates concurrency can say so per deployment, which + // is a config change rather than a code one. The cost of the safe default + // is latency: N calls take N times as long, and the operator can see that + // in the log line each call writes. + DefaultSearchConcurrency = 1 +) + +// Step serves the agriculture facility capability by splitting a multi-type +// search across the ordinary one-call step and merging what comes back. +// +// A step rather than a hook into internal/upstream: what a payload splits +// across, how many calls that is, what is too many and how the answers +// recombine are all facts about POCRA, so they belong to a thing that knows +// POCRA. internal/upstream stays a step that serves one payload with one +// call, which is what every other capability needs of it. +type Step struct { + // inner serves one payload with one call: the ordinary upstream step, + // built by this package's New and never handed a payload naming more than + // one facility type. + inner definition.Step + + // paths and bindingKeys answer "is this payload mine?", the same question + // and the same way inner answers it. Asked here first because a payload + // that is NOT this capability's must reach inner untouched -- it passes + // through, as it does for every capability it does not serve -- while one + // that IS must be split before inner sees it. + paths common.Paths + bindingKeys []string + + // concurrency is how many of a split search's calls run at once. + concurrency int +} + +// Run serves one inbound payload. +// +// A payload naming one facility type is one call, which is the common case. A +// payload naming several becomes one call per type, run concurrently up to +// this step's limit, and their answers are merged into one. A payload that is +// not this capability's is handed to inner unchanged, which passes it through. +func (s *Step) Run(ctx *model.StepContext) error { + if !s.mine(ctx.Body) { + return s.inner.Run(ctx) + } + + var beckn any + if err := json.Unmarshal(ctx.Body, &beckn); err != nil { + return model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload is not JSON: %w", err)) + } + + types, err := facilityTypesFrom(beckn) + if err != nil { + return err + } + if len(types) > MaxFacilityTypes { + return model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: this payload asks for %d facility types and the ceiling is %d; "+ + "split it across more than one request", len(types), MaxFacilityTypes)) + } + + // Every type gets its own payload, even when there is only one of them: + // the fresh message id each part carries is what stops POCRA blending the + // answers, and a search of one still wants its own id rather than the + // caller's. See splitByType. + parts, err := splitByType(ctx.Body, types) + if err != nil { + return err + } + + log.Debugf(ctx, "agriculture facility: serving %d facility type(s) as %d call(s), %d at a time", + len(types), len(parts), s.concurrency) + + // Bounded, ordered and fail-fast, which is internal/concurrent's whole + // job. One failure fails the request: a partial answer is the defect this + // was built to fix wearing a different hat -- the caller asked for four + // facility types, would receive three, and nothing in the payload would + // say that the fourth was asked for and lost. + answers, err := concurrent.Map(ctx, parts, s.concurrency, + func(callCtx context.Context, part []byte) ([]byte, error) { + // Its own StepContext: inner reads Body and writes ResponseBody, + // so the parts must not share either. callCtx rather than ctx so a + // sibling's failure cancels this call too. + partCtx := *ctx + partCtx.Context = callCtx + partCtx.Body = part + partCtx.ResponseBody = nil + + if err := s.runPart(&partCtx); err != nil { + return nil, err + } + return partCtx.ResponseBody, nil + }) + if err != nil { + return err + } + + merged, err := mergeAnswers(answers, messageIDOf(beckn)) + if err != nil { + return err + } + ctx.ResponseBody = merged + return nil +} + +// runPart runs the one-call step over one part and insists it answered. +// +// An empty ResponseBody means inner passed the payload through rather than +// serving it, which for a part built from a payload this step already +// recognised can only be a mismatch between this step's binding keys and +// inner's. Reported rather than merged, because merging nothing would answer +// a four-type search with three types and no error. +func (s *Step) runPart(partCtx *model.StepContext) error { + if err := s.inner.Run(partCtx); err != nil { + return err + } + if len(partCtx.ResponseBody) == 0 { + return fmt.Errorf( + "agriculture facility: the upstream step served no answer for one part of this search, " + + "which means it does not serve this capability -- check that both are configured " + + "with the same binding keys") + } + return nil +} + +// mine reports whether this payload names one of the capabilities this step +// serves. A payload it cannot read is not refused here: inner refuses it, with +// the message it has always used. +func (s *Step) mine(body []byte) bool { + binding, err := common.BindingFrom(s.paths, body) + if err != nil { + return false + } + return slices.Contains(s.bindingKeys, binding.Key()) +} + +// splitByType turns one payload into one payload per facility type. +// +// Each part names exactly one type, so the mapping's request half reads a +// single value rather than choosing among several -- choosing the first is +// what made a multi-type search answer with one type and silently drop the +// rest. +// +// Each part also carries a FRESH context.messageId. POCRA keeps a message_id's +// answers for ten minutes and returns the union of everything asked for under +// it, so two parts sharing an id would each come back carrying the other's +// facilities. The mapping copies this straight through to POCRA's message_id, +// whose schema refuses anything that is not a UUID. +// +// Re-decoded per part rather than deep-copied: the parts are mutated +// independently and a shared nested map would have them overwrite each other's +// type. A payload is a few kilobytes and this happens once per request. +func splitByType(body []byte, types []string) ([][]byte, error) { + parts := make([][]byte, 0, len(types)) + for _, facilityType := range types { + var part map[string]any + if err := json.Unmarshal(body, &part); err != nil { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload is not a JSON object: %w", err)) + } + + attributes, ok := dig(part, "message", "contract", "commitments").([]any) + if !ok || len(attributes) == 0 { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload carries no commitment to split")) + } + commitment, ok := attributes[0].(map[string]any) + if !ok { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload's first commitment is %T, not an object", attributes[0])) + } + resources, ok := commitment["resources"].([]any) + if !ok || len(resources) == 0 { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload's commitment carries no resource to split")) + } + resource, ok := resources[0].(map[string]any) + if !ok { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload's first resource is %T, not an object", resources[0])) + } + resourceAttributes, ok := resource["resourceAttributes"].(map[string]any) + if !ok { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload's first resource carries no resourceAttributes")) + } + + // A list of one, not a bare string: the required: checks and the + // request half both read this through [] and a scalar would work, but + // a part that does not look like the payload it came from is a trap + // for whoever reads one in a log. + resourceAttributes["supportedFacilityTypes"] = []any{facilityType} + + becknContext, ok := part["context"].(map[string]any) + if !ok { + return nil, model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload carries no context to stamp a message id on")) + } + becknContext["messageId"] = uuid.NewString() + + encoded, err := json.Marshal(part) + if err != nil { + return nil, fmt.Errorf("agriculture facility: could not rebuild the payload for %s: %w", + facilityType, err) + } + parts = append(parts, encoded) + } + return parts, nil +} + +// mergeAnswers combines one answer per facility type into the single answer the +// caller gets. +// +// The first answer is the shape of the result -- context, status and offer are +// the same in all of them, because all the parts came from one payload. What +// differs is the resources, which are concatenated in the order the payload +// named the types, deduplicated by id, and pointed at by the offer. +// +// The caller's own messageId is restored: each part carried a generated one so +// POCRA would not blend them, and the answer has to correlate with the request +// the caller actually sent. +// +// ORDERING, worth knowing: within one facility type the mapping ranks by +// POCRA's distance, and that ranking survives here. ACROSS types the result is +// type-blocked -- every KrishiVigyanKendra, then every Warehouse -- rather +// than globally nearest-first, because distance is not an intrinsic facility +// attribute and the mapping drops it before this sees the answers. A globally +// ranked multi-type search would need the mapping to publish a distance this +// could sort on, which the schema pack says it must not. +func mergeAnswers(answers [][]byte, callerMessageID string) ([]byte, error) { + if len(answers) == 0 { + return nil, fmt.Errorf("agriculture facility: nothing to merge; this search made no calls") + } + + var merged map[string]any + if err := json.Unmarshal(answers[0], &merged); err != nil { + return nil, fmt.Errorf("agriculture facility: the answer for the first facility type is not JSON: %w", err) + } + commitment, err := commitmentOf(merged) + if err != nil { + return nil, err + } + + resources := make([]any, 0) + seen := map[string]bool{} + for index, answer := range answers { + var document map[string]any + if err := json.Unmarshal(answer, &document); err != nil { + return nil, fmt.Errorf( + "agriculture facility: the answer for facility type %d is not JSON: %w", index, err) + } + part, err := commitmentOf(document) + if err != nil { + return nil, err + } + found, _ := part["resources"].([]any) + for _, entry := range found { + resource, ok := entry.(map[string]any) + if !ok { + continue + } + id, _ := resource["id"].(string) + // Deduplicated by id because a repeated id in the answer is a + // broken reference the moment anything resolves it. The mapping + // already collapses POCRA's own repeats within one call; this + // catches a facility that answered for two types. + if id != "" && seen[id] { + continue + } + seen[id] = true + resources = append(resources, resource) + } + } + commitment["resources"] = resources + + // The offer points at what the answer actually carries. Leaving the first + // part's resourceIds would name only its own type's facilities. + if offer, ok := commitment["offer"].(map[string]any); ok { + ids := make([]any, 0, len(resources)) + for _, entry := range resources { + if resource, ok := entry.(map[string]any); ok { + ids = append(ids, resource["id"]) + } + } + offer["resourceIds"] = ids + } + + if becknContext, ok := merged["context"].(map[string]any); ok && callerMessageID != "" { + becknContext["messageId"] = callerMessageID + } + + encoded, err := json.Marshal(merged) + if err != nil { + return nil, fmt.Errorf("agriculture facility: could not encode the merged answer: %w", err) + } + return encoded, nil +} + +// commitmentOf reaches the one commitment an answer carries. +func commitmentOf(document map[string]any) (map[string]any, error) { + commitments, _ := dig(document, "message", "contract", "commitments").([]any) + if len(commitments) == 0 { + return nil, fmt.Errorf("agriculture facility: an answer carries no commitment to merge") + } + commitment, ok := commitments[0].(map[string]any) + if !ok { + return nil, fmt.Errorf("agriculture facility: an answer's commitment is %T, not an object", commitments[0]) + } + return commitment, nil +} + +// messageIDOf reads the caller's own message id, for the merged answer to +// correlate with. Absent is not an error: the mapping copies whatever the +// payload carried, so an absent one stays absent, as it did before splitting +// existed. +func messageIDOf(beckn any) string { + document, ok := beckn.(map[string]any) + if !ok { + return "" + } + id, _ := dig(document, "context", "messageId").(string) + return id +} + +// searchConcurrency resolves the deployment's setting, clamped to +// MaxFacilityTypes and defaulting to DefaultSearchConcurrency when left unset +// or non-positive. +// +// The arithmetic is internal/concurrent's, because every caller of it needs +// the same clamp and a zero limit there means UNBOUNDED. The three numbers are +// this package's, because each one is a fact about POCRA. +func searchConcurrency(configured int) int { + return concurrent.Bound(configured, DefaultSearchConcurrency, MaxFacilityTypes) +} diff --git a/pkg/plugin/implementation/AgricultureFacility/search_test.go b/pkg/plugin/implementation/AgricultureFacility/search_test.go new file mode 100644 index 00000000..33bcb9be --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/search_test.go @@ -0,0 +1,249 @@ +package AgricultureFacility_test + +// search_test.go covers the POLICY this package owns: how many of a +// multi-type search's calls run at once, in what order, and what happens when +// one fails. mappings_test.go already covers that splitting happens at all +// and produces the right facilities (TestATwoTypeSearchIsAnsweredWithBothTypes +// and neighbours) and that each call carries its own identity +// (TestEachSearchCallCarriesItsOwnRequestId). + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/beckn-one/beckn-onix/pkg/model" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/AgricultureFacility" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common/util" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/jsonmapper" +) + +// runSearchTimed is runSplitSearch's sibling: it hands the caller a raw HTTP +// handler (so a test can measure timing or count in-flight calls) and an +// explicit Config (so a test can set SearchConcurrency), rather than a canned +// per-category response map. +func runSearchTimed(t *testing.T, types []string, cfg *AgricultureFacility.Config, handler http.HandlerFunc) error { + t.Helper() + + mappings := serveMappings(t) + defer mappings.Close() + + upstream := httptest.NewServer(handler) + defer upstream.Close() + + mapper, closeMapper, err := jsonmapper.New(context.Background(), &jsonmapper.Config{}) + if err != nil { + t.Fatalf("failed to build the mapper: %v", err) + } + defer closeMapper() + + registry := &stubRegistry{plan: &model.ProviderRecord{ + BindingKey: shippedBindingKey, BaseURL: upstream.URL, + Actions: map[string]model.ActionPlan{ + "select": {Method: http.MethodPost, Path: "/search", + Mappings: mappings.URL + "/" + shippedMapping, TimeoutMs: 30000}, + }, + }} + + cfg.BindingKeys = []string{shippedBindingKey} + // Auth is per provider, and a served provider without a block is refused at + // startup. POCRA needs no credential and says so. + if cfg.AuthByProvider == nil { + cfg.AuthByProvider = map[string]*common.AuthProfile{ + strings.Split(shippedBindingKey, "|")[0]: {Scheme: util.AuthSchemeNone}, + } + } + step, closeStep, err := AgricultureFacility.New(context.Background(), registry, mapper, cfg) + if err != nil { + t.Fatalf("failed to build the step: %v", err) + } + defer closeStep() + + var payload map[string]any + if err := json.Unmarshal([]byte(selectRequest), &payload); err != nil { + t.Fatalf("the fixture is not JSON: %v", err) + } + attributes(t, payload)["supportedFacilityTypes"] = toAny(types) + body, err := json.Marshal(payload) + if err != nil { + t.Fatalf("could not rebuild the payload: %v", err) + } + + return step.Run(&model.StepContext{Context: t.Context(), Body: body}) +} + +// A search's calls are sequential unless the deployment raised the limit. +// +// The default is not a performance choice. A provider that answers one +// question at a time is often not built to be asked several at once, and +// POCRA's failure mode when pushed is a 200 with an empty catalog -- +// indistinguishable from having no results, so the loss is silent. Parallel +// is opt-in per deployment for that reason, and this pins the default so it +// cannot drift. +func TestSearchCallsAreSequentialByDefault(t *testing.T) { + t.Parallel() + + var mu sync.Mutex + var inFlight, peak int + handler := func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + inFlight++ + if inFlight > peak { + peak = inFlight + } + mu.Unlock() + + // Long enough that overlapping calls would be caught. Without a wait + // each call could finish before the next begins and a parallel step + // would look sequential. + time.Sleep(40 * time.Millisecond) + + mu.Lock() + inFlight-- + mu.Unlock() + fmt.Fprint(w, providerResponse) + } + + types := []string{"KrishiVigyanKendra", "CustomHiringCentre", "Warehouse"} + if err := runSearchTimed(t, types, &AgricultureFacility.Config{}, handler); err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if peak != 1 { + t.Errorf("%d calls were in flight at once, want 1 by default", peak) + } +} + +// A deployment that raises the limit gets calls in parallel, bounded by it. +func TestSearchHonoursTheConfiguredConcurrency(t *testing.T) { + t.Parallel() + + var mu sync.Mutex + var inFlight, peak int + handler := func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + inFlight++ + if inFlight > peak { + peak = inFlight + } + mu.Unlock() + time.Sleep(60 * time.Millisecond) + mu.Lock() + inFlight-- + mu.Unlock() + fmt.Fprint(w, providerResponse) + } + + types := []string{"KrishiVigyanKendra", "CustomHiringCentre", "Warehouse", "SoilTestingFacility"} + cfg := &AgricultureFacility.Config{SearchConcurrency: 2} + if err := runSearchTimed(t, types, cfg, handler); err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + mu.Lock() + defer mu.Unlock() + if peak != 2 { + t.Errorf("peak concurrency was %d, want the configured 2", peak) + } +} + +// A call not yet issued when an earlier one fails is skipped, not made and +// discarded. Sequential (the default) makes this deterministic: the first +// type's failure cancels the shared context before the second type's call is +// ever issued. +func TestSearchSkipsCallsNotYetIssuedAfterAFailure(t *testing.T) { + t.Parallel() + + var mu sync.Mutex + var calls int + handler := func(w http.ResponseWriter, r *http.Request) { + mu.Lock() + calls++ + mu.Unlock() + http.Error(w, "no", http.StatusInternalServerError) + } + + types := []string{"KrishiVigyanKendra", "CustomHiringCentre", "Warehouse"} + err := runSearchTimed(t, types, &AgricultureFacility.Config{}, handler) + if err == nil { + t.Fatal("Run() served a partial answer, want it refused") + } + + mu.Lock() + defer mu.Unlock() + // Sequential (limit 1): the first type fails and cancels the shared + // context before the loop ever reaches the second or third type's call. + if calls != 1 { + t.Errorf("the provider was called %d times, want 1 -- later facility types "+ + "must be skipped once an earlier one fails", calls) + } +} + +// A payload asking for more calls than the ceiling is refused, not clamped. +// +// Uses a repeated governed type to exceed MaxFacilityTypes, which the +// mapping's own duplicate-type required check ALSO refuses -- both guards +// agree the payload is bad, so the assertion that matters is that the provider +// was never called, not which guard fired first. +func TestSearchRefusesOverTheCeiling(t *testing.T) { + t.Parallel() + + handler := func(w http.ResponseWriter, r *http.Request) { + t.Error("the provider was called for a search that should have been refused") + } + + types := make([]string, AgricultureFacility.MaxFacilityTypes+1) + for i := range types { + types[i] = "KrishiVigyanKendra" + } + err := runSearchTimed(t, types, &AgricultureFacility.Config{}, handler) + if err == nil { + t.Fatal("a search over the ceiling was served, want it refused") + } +} + +// A payload naming one type is one call. +func TestSearchOfOneTypeIsOneCall(t *testing.T) { + t.Parallel() + + var calls int32 + handler := func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&calls, 1) + fmt.Fprint(w, providerResponse) + } + + if err := runSearchTimed(t, []string{"KrishiVigyanKendra"}, &AgricultureFacility.Config{}, handler); err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + if got := atomic.LoadInt32(&calls); got != 1 { + t.Errorf("the provider saw %d calls, want 1 for a one-type search", got) + } +} + +// A payload naming no types at all is refused before the provider is called. +// +// The mapping's own required checks refuse this first, with a better message. +// This asserts the Go read refuses it too, because a payload with nothing to +// split across would otherwise become zero calls and an empty answer -- the +// silent-loss failure this whole facility exists to prevent. +func TestSearchRefusesAPayloadNamingNoTypes(t *testing.T) { + t.Parallel() + + handler := func(w http.ResponseWriter, r *http.Request) { + t.Error("the provider was called for a payload naming no facility types") + } + + if err := runSearchTimed(t, []string{}, &AgricultureFacility.Config{}, handler); err == nil { + t.Fatal("a payload naming no facility types was served, want it refused") + } +} diff --git a/pkg/plugin/implementation/AgricultureFacility/testdata/.gitignore b/pkg/plugin/implementation/AgricultureFacility/testdata/.gitignore new file mode 100644 index 00000000..e216875f --- /dev/null +++ b/pkg/plugin/implementation/AgricultureFacility/testdata/.gitignore @@ -0,0 +1,4 @@ +# The schema pack is fetched from where it is published and cached here, rather +# than vendored into the repo -- see schemacache_test.go. The cache is a local +# build artifact: delete it to refresh against upstream. +schema-cache/ diff --git a/pkg/plugin/implementation/internal/common/binding.go b/pkg/plugin/implementation/internal/common/binding.go index 7f0759ce..8d657d32 100644 --- a/pkg/plugin/implementation/internal/common/binding.go +++ b/pkg/plugin/implementation/internal/common/binding.go @@ -31,16 +31,19 @@ func (b Binding) Key() string { return b.ParticipantID + separator + b.CapabilityCode } -// bindingFrom derives the capability binding a payload is asking for. +// BindingFrom derives the capability binding a payload is asking for. // // Returns errNoBinding when the payload names no provider or type -- the // ordinary case for a request this step is not meant to serve. // +// EXPORTED for a domain package that must answer "is this payload mine?" the +// same way serve does, rather than reading the payload a second way. +// // More than one distinct provider or type is refused, not resolved to the // first: the two halves index ONE registry row for ONE call, so a payload // spanning several is asking for something this design cannot express, and // guessing would silently serve part of it. -func bindingFrom(paths Paths, body []byte) (Binding, error) { +func BindingFrom(paths Paths, body []byte) (Binding, error) { var payload any if err := json.Unmarshal(body, &payload); err != nil { return Binding{}, fmt.Errorf("payload could not be read: %w", err) diff --git a/pkg/plugin/implementation/internal/common/binding_test.go b/pkg/plugin/implementation/internal/common/binding_test.go index a7507f2a..d0195221 100644 --- a/pkg/plugin/implementation/internal/common/binding_test.go +++ b/pkg/plugin/implementation/internal/common/binding_test.go @@ -43,9 +43,9 @@ const realSelectPayload = `{ func TestFromReadsARealSelectPayload(t *testing.T) { t.Parallel() - got, err := bindingFrom(BecknV2, []byte(realSelectPayload)) + got, err := BindingFrom(BecknV2, []byte(realSelectPayload)) if err != nil { - t.Fatalf("bindingFrom() returned an unexpected error: %v", err) + t.Fatalf("BindingFrom() returned an unexpected error: %v", err) } if got.ParticipantID != "mausamgram" { t.Errorf("participant = %q, want mausamgram", got.ParticipantID) @@ -80,7 +80,7 @@ func TestFromReportsAPayloadWithNoBinding(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - if _, err := bindingFrom(BecknV2, []byte(tc.body)); !errors.Is(err, errNoBinding) { + if _, err := BindingFrom(BecknV2, []byte(tc.body)); !errors.Is(err, errNoBinding) { t.Errorf("expected errNoBinding, got %v", err) } }) @@ -119,7 +119,7 @@ func TestFromRefusesAnAmbiguousPayload(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - _, err := bindingFrom(BecknV2, []byte(tc.body)) + _, err := BindingFrom(BecknV2, []byte(tc.body)) if err == nil { t.Fatal("expected an ambiguous payload to be refused") } @@ -136,7 +136,7 @@ func TestFromRefusesAnAmbiguousPayload(t *testing.T) { func TestFromReportsAnUnreadablePayload(t *testing.T) { t.Parallel() - _, err := bindingFrom(BecknV2, []byte(`{"message":`)) + _, err := BindingFrom(BecknV2, []byte(`{"message":`)) if err == nil { t.Fatal("expected unreadable JSON to be reported") } @@ -161,9 +161,9 @@ func TestBecknV2IsTheDefault(t *testing.T) { if BecknV2.ProviderID == "" || BecknV2.CapabilityCode == "" { t.Fatal("the default paths must be set") } - got, err := bindingFrom(BecknV2, []byte(realSelectPayload)) + got, err := BindingFrom(BecknV2, []byte(realSelectPayload)) if err != nil { - t.Fatalf("bindingFrom() returned an unexpected error: %v", err) + t.Fatalf("BindingFrom() returned an unexpected error: %v", err) } if got.ParticipantID != "mausamgram" || got.CapabilityCode != "openagrinet:WeatherObservation" { t.Errorf("binding = %+v, want the Beckn v2 convention's answer", got) @@ -176,12 +176,12 @@ func TestFromReadsAnOverriddenPath(t *testing.T) { t.Parallel() body := `{"who":{"provider":"agmarknet"},"what":[{"type":"openagrinet:MandiPrice"}]}` - got, err := bindingFrom(Paths{ + got, err := BindingFrom(Paths{ ProviderID: "who.provider", CapabilityCode: "what[].type", }, []byte(body)) if err != nil { - t.Fatalf("bindingFrom() returned an unexpected error: %v", err) + t.Fatalf("BindingFrom() returned an unexpected error: %v", err) } if got.Key() != "agmarknet|openagrinet:MandiPrice" { t.Errorf("binding key = %q, want it read from the overridden paths", got.Key()) @@ -193,7 +193,7 @@ func TestFromReadsAnOverriddenPath(t *testing.T) { func TestFromReportsNoBindingWhenAPathMatchesNothing(t *testing.T) { t.Parallel() - _, err := bindingFrom(Paths{ProviderID: "nowhere.at.all", CapabilityCode: "what[].type"}, + _, err := BindingFrom(Paths{ProviderID: "nowhere.at.all", CapabilityCode: "what[].type"}, []byte(`{"what":[{"type":"x"}]}`)) if !errors.Is(err, errNoBinding) { t.Errorf("expected errNoBinding, got %v", err) @@ -205,7 +205,7 @@ func TestFromReportsNoBindingWhenAPathMatchesNothing(t *testing.T) { func TestFromStillRefusesSeveralValuesUnderAnOverride(t *testing.T) { t.Parallel() - _, err := bindingFrom(Paths{ProviderID: "who[].provider", CapabilityCode: "what[].type"}, + _, err := BindingFrom(Paths{ProviderID: "who[].provider", CapabilityCode: "what[].type"}, []byte(`{"who":[{"provider":"a"},{"provider":"b"}],"what":[{"type":"x"}]}`)) if err == nil || errors.Is(err, errNoBinding) { t.Errorf("expected a refusal naming both providers, got %v", err) @@ -303,7 +303,7 @@ func TestFromRefusesSeveralCommitments(t *testing.T) { "resources":[{"resourceAttributes":{"@type":"openagrinet:WeatherObservation"}}]} ]}}}` - _, err := bindingFrom(BecknV2, []byte(body)) + _, err := BindingFrom(BecknV2, []byte(body)) if err == nil { t.Fatal("expected two commitments to be refused rather than halved") } @@ -319,7 +319,7 @@ func TestFromRefusesSeveralCommitments(t *testing.T) { {"offer":{"provider":{"id":"mausamgram"}}, "resources":[{"resourceAttributes":{"@type":"openagrinet:WeatherObservation"}}]} ]}}}` - binding, err := bindingFrom(BecknV2, []byte(single)) + binding, err := BindingFrom(BecknV2, []byte(single)) if err != nil { t.Fatalf("one commitment must still resolve: %v", err) } @@ -370,7 +370,7 @@ func TestFromRefusesSeveralCommitmentsEvenWhenOneDoesNotResolve(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - _, err := bindingFrom(BecknV2, []byte(tt.payload)) + _, err := BindingFrom(BecknV2, []byte(tt.payload)) if err == nil { t.Fatal("two commitments must be refused, not half answered") } @@ -394,7 +394,7 @@ func TestFromStillPassesThroughASingleUnresolvableCommitment(t *testing.T) { {"offer":{"provider":{}}, "resources":[{"resourceAttributes":{"@type":"openagrinet:WeatherObservation"}}]} ]}}}` - _, err := bindingFrom(BecknV2, []byte(payload)) + _, err := BindingFrom(BecknV2, []byte(payload)) if !errors.Is(err, errNoBinding) { t.Errorf("err = %v, want errNoBinding so the payload passes through", err) } diff --git a/pkg/plugin/implementation/internal/common/common.go b/pkg/plugin/implementation/internal/common/common.go index bbf7ff98..ccfcd565 100644 --- a/pkg/plugin/implementation/internal/common/common.go +++ b/pkg/plugin/implementation/internal/common/common.go @@ -106,7 +106,7 @@ func New(ctx context.Context, registry definition.ProviderRecordLookup, mapper d return nil, nil, err } - paths, err := bindingPaths(cfg) + paths, err := BindingPaths(cfg) if err != nil { return nil, nil, err } @@ -135,11 +135,16 @@ func New(ctx context.Context, registry definition.ProviderRecordLookup, mapper d return step, closer, nil } -// bindingPaths resolves where this step reads a binding key from. +// BindingPaths resolves where this step reads a binding key from. // // Both halves or neither: one overridden and one defaulted would match nothing, // silently, on every request. -func bindingPaths(cfg *Config) (Paths, error) { +// +// EXPORTED for a domain package that has to answer "is this payload mine?" the +// same way this step does -- AgricultureFacility, whose search.go fans one +// request out over several capability types. The alternative is reading the +// same two config fields a second time, which is how the two drift apart. +func BindingPaths(cfg *Config) (Paths, error) { if cfg.ProviderIDAt == "" && cfg.CapabilityCodeAt == "" { return BecknV2, nil } diff --git a/pkg/plugin/implementation/internal/common/serve.go b/pkg/plugin/implementation/internal/common/serve.go index c902fe1f..6d34eca7 100644 --- a/pkg/plugin/implementation/internal/common/serve.go +++ b/pkg/plugin/implementation/internal/common/serve.go @@ -22,12 +22,12 @@ import ( // recognises its own work, so adding a provider is one config entry rather than // a routing-table change. func (s *Step) Run(ctx *model.StepContext) error { - binding, err := bindingFrom(s.paths, ctx.Body) + binding, err := BindingFrom(s.paths, ctx.Body) if errors.Is(err, errNoBinding) { return nil } if err != nil { - // Everything bindingFrom refuses is about the payload -- unreadable + // Everything BindingFrom refuses is about the payload -- unreadable // JSON, or more than one call named. Unclassified it becomes a 500, // which blames this adapter and hides the reason from the caller. return model.NewBadReqErr("", err) diff --git a/pkg/plugin/implementation/internal/concurrent/concurrent.go b/pkg/plugin/implementation/internal/concurrent/concurrent.go new file mode 100644 index 00000000..7bd9b231 --- /dev/null +++ b/pkg/plugin/implementation/internal/concurrent/concurrent.go @@ -0,0 +1,116 @@ +// Package concurrent is one generic engine: run a known number of +// independent calls, bounded, with the first failure cancelling the rest. +// +// It knows nothing about Beckn, upstream providers, or any specific +// capability. Every policy question -- how many is too many, what identity +// each call carries, what a failure should say -- belongs to whoever calls +// this, not to this package: see +// pkg/plugin/implementation/AgricultureFacility/search.go for the one caller +// that exists today. +// +// pkg/plugin/implementation/catalogpublisher's own runConcurrent looks +// similar but is NOT this: it collects every call's error rather than +// cancelling on the first one, because its caller needs to inspect all of +// them to decide whether any is worth retrying. That is a different +// concurrency shape, kept separate rather than forced into one function +// with a mode flag. +package concurrent + +import ( + "context" + "fmt" + + "golang.org/x/sync/errgroup" +) + +// Run calls work(ctx, i) for every i in [0, n), at most limit at a time, and +// returns each call's result at its own index -- in the order Run was asked +// for them, not the order they finished, so a caller can rely on results[i] +// meaning "the i-th thing I asked for" regardless of which one answered +// first. +// +// The first error any call returns cancels the ctx handed to every other +// call, in flight or not yet started: a call not yet issued when an earlier +// one already failed is skipped rather than made and discarded, and an +// in-flight call that itself respects ctx cancellation (as an +// http.NewRequestWithContext-built request does) is aborted rather than run +// to completion for an answer nobody will use. Run itself returns that first +// error, wrapped by nothing -- whatever work returned is what the caller +// gets, so the caller's own wrapping (naming which of the n things failed, +// what identity it carried) survives unchanged. What each call's own input or +// identity looks like is entirely work's business -- Run passes only an +// index, so it never has to know or care what a caller maps that index onto. +// +// n == 0 calls work zero times and returns (nil, nil). limit is passed +// straight to errgroup.Group.SetLimit -- see its own doc for what a +// non-positive value means. +func Run[T any](ctx context.Context, n, limit int, work func(ctx context.Context, index int) (T, error)) ([]T, error) { + if n == 0 { + return nil, nil + } + + group, groupCtx := errgroup.WithContext(ctx) + group.SetLimit(limit) + + results := make([]T, n) + for index := range n { + group.Go(func() error { + if groupCtx.Err() != nil { + return nil + } + result, err := work(groupCtx, index) + if err != nil { + return fmt.Errorf("%w", err) + } + results[index] = result + return nil + }) + } + + if err := group.Wait(); err != nil { + return nil, err + } + return results, nil +} + +// Map is Run over a list of VALUES rather than a count: one call per value, +// results in the order the values were given. +// +// The same bounded, ordered, fail-fast guarantees as Run -- it is Run, with +// the index-to-value step done here instead of in every caller. Prefer it +// whenever the work is "one call per thing in this list". +// +// values is []V and results are []T, so a caller keeps whatever types it +// already has; nothing here inspects either. +// +// What Map does NOT decide, on purpose: how many values are too many, and +// what to tell a caller who asked for too many. A ceiling is a fact about +// whatever is being called and its refusal is that domain's error to write, +// so it belongs to the package that knows the domain. Use Bound below for the +// other half of the arithmetic, defaulting and clamping a configured +// concurrency. +func Map[V, T any](ctx context.Context, values []V, limit int, + call func(ctx context.Context, value V) (T, error)) ([]T, error) { + + return Run(ctx, len(values), limit, func(ctx context.Context, index int) (T, error) { + return call(ctx, values[index]) + }) +} + +// Bound resolves a configured concurrency: fallback when it is unset or +// non-positive, ceiling when it is over. +// +// Here rather than in each caller because every caller of Run or Map needs +// the same three lines, and getting them wrong is quiet: a zero limit means +// UNBOUNDED in errgroup, so a deployment that leaves the setting out would get +// every call at once from a mistake that reads like a default. +func Bound(configured, fallback, ceiling int) int { + limit := configured + if limit <= 0 { + limit = fallback + } + if limit > ceiling { + limit = ceiling + } + return limit +} diff --git a/pkg/plugin/implementation/internal/concurrent/concurrent_test.go b/pkg/plugin/implementation/internal/concurrent/concurrent_test.go new file mode 100644 index 00000000..a35d4c4f --- /dev/null +++ b/pkg/plugin/implementation/internal/concurrent/concurrent_test.go @@ -0,0 +1,288 @@ +package concurrent + +import ( + "context" + "errors" + "fmt" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestRunReturnsResultsInOrderNotArrivalOrder(t *testing.T) { + t.Parallel() + + // index 0 is the slowest, so an arrival-ordered result would put it + // last -- this proves the result is indexed, not appended. + delays := []time.Duration{30 * time.Millisecond, 10 * time.Millisecond, 0} + results, err := Run(context.Background(), len(delays), len(delays), + func(ctx context.Context, i int) (int, error) { + time.Sleep(delays[i]) + return i, nil + }) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + for i, got := range results { + if got != i { + t.Errorf("results[%d] = %d, want %d", i, got, i) + } + } +} + +func TestRunIsSequentialWhenLimitIsOne(t *testing.T) { + t.Parallel() + + var mu sync.Mutex + var inFlight, peak int + _, err := Run(context.Background(), 4, 1, func(ctx context.Context, i int) (struct{}, error) { + mu.Lock() + inFlight++ + if inFlight > peak { + peak = inFlight + } + mu.Unlock() + + time.Sleep(10 * time.Millisecond) + + mu.Lock() + inFlight-- + mu.Unlock() + return struct{}{}, nil + }) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + if peak != 1 { + t.Errorf("peak concurrency = %d, want 1", peak) + } +} + +func TestRunHonoursALimitAboveOne(t *testing.T) { + t.Parallel() + + var mu sync.Mutex + var inFlight, peak int + _, err := Run(context.Background(), 6, 2, func(ctx context.Context, i int) (struct{}, error) { + mu.Lock() + inFlight++ + if inFlight > peak { + peak = inFlight + } + mu.Unlock() + + time.Sleep(15 * time.Millisecond) + + mu.Lock() + inFlight-- + mu.Unlock() + return struct{}{}, nil + }) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + if peak != 2 { + t.Errorf("peak concurrency = %d, want the configured 2", peak) + } +} + +func TestRunReturnsTheCallsOwnError(t *testing.T) { + t.Parallel() + + sentinel := errors.New("boom") + _, err := Run(context.Background(), 3, 3, func(ctx context.Context, i int) (int, error) { + if i == 1 { + return 0, fmt.Errorf("call %d failed: %w", i, sentinel) + } + return i, nil + }) + if err == nil || !errors.Is(err, sentinel) { + t.Fatalf("error = %v, want it to wrap the call's own error", err) + } +} + +func TestRunSkipsCallsNotYetIssuedAfterAFailureWhenSequential(t *testing.T) { + t.Parallel() + + var called int32 + _, err := Run(context.Background(), 3, 1, func(ctx context.Context, i int) (struct{}, error) { + atomic.AddInt32(&called, 1) + if i == 0 { + return struct{}{}, errors.New("first call fails") + } + return struct{}{}, nil + }) + if err == nil { + t.Fatal("Run() returned no error, want the first call's failure") + } + // Sequential (limit 1): index 0 fails and cancels the shared context + // before index 1's call is ever issued, so only index 0 was called. + if got := atomic.LoadInt32(&called); got != 1 { + t.Errorf("work was called %d times, want 1 -- calls not yet issued "+ + "when an earlier one fails must be skipped, not made and discarded", got) + } +} + +func TestRunOfZeroReturnsNoResultsAndNoError(t *testing.T) { + t.Parallel() + + results, err := Run(context.Background(), 0, 4, func(ctx context.Context, i int) (int, error) { + t.Error("work was called for n == 0") + return 0, nil + }) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + if len(results) != 0 { + t.Errorf("results = %v, want empty", results) + } +} + +func TestRunOfOneRunsExactlyOnce(t *testing.T) { + t.Parallel() + + var called int32 + results, err := Run(context.Background(), 1, 1, func(ctx context.Context, i int) (string, error) { + atomic.AddInt32(&called, 1) + return "ok", nil + }) + if err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + if got := atomic.LoadInt32(&called); got != 1 { + t.Errorf("work was called %d times, want 1", got) + } + if len(results) != 1 || results[0] != "ok" { + t.Errorf("results = %v, want [\"ok\"]", results) + } +} + +func TestRunCancelsTheContextItHandsToInFlightWork(t *testing.T) { + t.Parallel() + + var slowCtxDone int32 + // inFlight is closed by index 1 once it is genuinely running, and index 0 + // waits for that before failing. Without the handshake this test is a + // race: index 1's call may not have started when index 0 fails, and Run + // then SKIPS it rather than cancelling it -- correct behaviour, and what + // TestRunSkipsCallsNotYetIssuedAfterAFailureWhenSequential is for, but + // not what this test is about. + inFlight := make(chan struct{}) + _, err := Run(context.Background(), 2, 2, func(ctx context.Context, i int) (struct{}, error) { + if i == 0 { + <-inFlight + return struct{}{}, errors.New("fails once the other call is in flight") + } + close(inFlight) + // In flight when index 0 fails: wait for our own ctx to be cancelled, + // or give up after a bound that would mean it never was. A bound + // rather than a bare receive so a broken Run fails this test instead + // of hanging it. + select { + case <-ctx.Done(): + atomic.AddInt32(&slowCtxDone, 1) + case <-time.After(2 * time.Second): + } + return struct{}{}, nil + }) + if err == nil { + t.Fatal("Run() returned no error, want index 0's failure") + } + if atomic.LoadInt32(&slowCtxDone) != 1 { + t.Error("the in-flight call's context was not cancelled after the other call failed") + } +} + +// --- Map -------------------------------------------------------------------- + +// Map hands each value to its own call and returns the answers in the order +// the values were given, whatever order the calls finished in. +func TestMapCallsOncePerValueInOrder(t *testing.T) { + t.Parallel() + + values := []string{"kvk", "warehouse", "soil_lab"} + results, err := Map(context.Background(), values, 3, func(ctx context.Context, value string) (string, error) { + // The first value sleeps longest, so arrival order is the reverse of + // the order asked for. A caller relying on results[i] meaning "the + // answer for values[i]" has to hold regardless. + if value == "kvk" { + time.Sleep(40 * time.Millisecond) + } + return "answered:" + value, nil + }) + if err != nil { + t.Fatalf("Map() returned an unexpected error: %v", err) + } + want := []string{"answered:kvk", "answered:warehouse", "answered:soil_lab"} + if len(results) != len(want) { + t.Fatalf("results = %v, want %v", results, want) + } + for index := range want { + if results[index] != want[index] { + t.Errorf("results[%d] = %q, want %q -- value order, not arrival order", + index, results[index], want[index]) + } + } +} + +// An empty list makes no calls and is not an error: a caller deciding that an +// empty list is a bad request says so itself, in its own words. +func TestMapOfNoValuesMakesNoCalls(t *testing.T) { + t.Parallel() + + results, err := Map(context.Background(), nil, 2, func(ctx context.Context, value string) (int, error) { + t.Error("a call was made for an empty value list") + return 0, nil + }) + if err != nil { + t.Fatalf("Map() returned an unexpected error: %v", err) + } + if len(results) != 0 { + t.Errorf("results = %v, want empty", results) + } +} + +// One value's failure fails the whole call, and the value's own error is what +// comes back -- the caller named which value it was and why. +func TestMapFailsWhenOneValueFails(t *testing.T) { + t.Parallel() + + sentinel := errors.New("warehouse is unreachable") + _, err := Map(context.Background(), []string{"kvk", "warehouse"}, 1, + func(ctx context.Context, value string) (string, error) { + if value == "warehouse" { + return "", sentinel + } + return "ok", nil + }) + if !errors.Is(err, sentinel) { + t.Errorf("error = %v, want the failing value's own error", err) + } +} + +// --- Bound ------------------------------------------------------------------ + +// Bound is the arithmetic every caller of Run and Map needs: a configured +// limit, a fallback when it is unset, a ceiling when it is over. Zero must +// become the fallback and not reach errgroup, where it means UNBOUNDED. +func TestBoundDefaultsAndClamps(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + configured, fallback, ceiling, want int + }{ + {"unset takes the fallback", 0, 1, 8, 1}, + {"negative takes the fallback", -4, 2, 8, 2}, + {"within range is honoured", 4, 1, 8, 4}, + {"exactly the ceiling is honoured", 8, 1, 8, 8}, + {"over the ceiling is clamped", 64, 1, 8, 8}, + {"a fallback over the ceiling is clamped too", 0, 16, 8, 8}, + } { + if got := Bound(tc.configured, tc.fallback, tc.ceiling); got != tc.want { + t.Errorf("%s: Bound(%d, %d, %d) = %d, want %d", + tc.name, tc.configured, tc.fallback, tc.ceiling, got, tc.want) + } + } +} From 45f16e54aff4f10ac9fba1057187aa21ff0029a1 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Thu, 10 Sep 2026 00:50:49 +0530 Subject: [PATCH 2/8] refactor(AgricultureFacility): read the facility types by path, not by hand [#91] facilityTypesFrom walked the document in Go -- dig(message, contract, commitments), then [0], then resources, then [0], then resourceAttributes -- with a type assertion and an error message at each level. That compiles the payload's shape into the binary: a spec change needs a rebuild, and eleven lines re-check what a walker already checks. The shape is the network's convention, not this adapter's, so it is now a PATH, defaulting to the Beckn v2 location and overridable per deployment -- exactly as common.Config's providerIdAt and capabilityCodeAt already are: facilityTypesAt: message.contract.commitments[].resources[]. resourceAttributes.supportedFacilityTypes[] The reading is common's walker, the same one that finds a binding key, so the traversal and its absent-field handling live in one place. A test proves the point: a payload carrying the types at search.facilities[].kinds[] is read correctly with nothing but a config path. WHAT THIS NEARLY BROKE, caught by their own tests. ValuesAt drops a non-string leaf, which is right for a binding key and wrong here: supportedFacilityTypes ["KrishiVigyanKendra", 42] would have searched for one type and reported success, a partial answer with nothing recording the loss -- the same class of bug the fan-out work existed to fix. common now exposes LeavesAt alongside ValuesAt: one walker, two views, type policy left to the caller. The plugin takes the leaves and refuses a non-string with the message it always used. An empty supportedFacilityTypes needed care too: it reads as zero leaves like an absent one, and the bare-string fallback would then hand back the empty list itself, reported as "[] is not a facility type" -- true and useless. A leaf that is itself a list is skipped, so an empty list keeps the "names no facility type" message. common's own tests pass untouched, so binding-key extraction is unchanged. 69 packages green, vet clean, no races. dig() remains for now: search.go still uses it to WRITE into a payload when splitting by type, which is a different primitive from reading and not something common offers. --- .../AgricultureFacility.go | 16 ++- .../AgricultureFacility/cmd/plugin.go | 3 + .../AgricultureFacility/payload.go | 103 ++++++++++-------- .../AgricultureFacility/payload_test.go | 41 ++++++- .../AgricultureFacility/search.go | 6 +- .../implementation/internal/common/binding.go | 4 +- .../internal/common/binding_test.go | 6 +- .../implementation/internal/common/paths.go | 47 ++++++-- 8 files changed, 153 insertions(+), 73 deletions(-) diff --git a/pkg/plugin/implementation/AgricultureFacility/AgricultureFacility.go b/pkg/plugin/implementation/AgricultureFacility/AgricultureFacility.go index 17f1e8c8..59a35d61 100644 --- a/pkg/plugin/implementation/AgricultureFacility/AgricultureFacility.go +++ b/pkg/plugin/implementation/AgricultureFacility/AgricultureFacility.go @@ -49,6 +49,13 @@ type Config struct { // schemes are defined and validated -- see common.AuthProfile. AuthByProvider map[string]*common.AuthProfile `yaml:"-" json:"-"` + // FacilityTypesAt is where the payload carries the facility types a search + // asks for. Absent means DefaultFacilityTypesAt, which is the Beckn v2 + // convention and what every deployment should be using -- the override is + // for tracking a spec change without waiting for a release, exactly as + // common.Config's providerIdAt and capabilityCodeAt are. + FacilityTypesAt string `yaml:"facilityTypesAt" json:"facilityTypesAt"` + // SearchConcurrency is how many of a multi-type search's calls may be in // flight at once. See search.go's DefaultSearchConcurrency and // MaxFacilityTypes for what absent and too-large mean. @@ -96,9 +103,10 @@ func New(ctx context.Context, registry definition.ProviderRecordLookup, mapper d } return &Step{ - inner: one, - paths: paths, - bindingKeys: cfg.BindingKeys, - concurrency: searchConcurrency(cfg.SearchConcurrency), + inner: one, + paths: paths, + bindingKeys: cfg.BindingKeys, + facilityTypesAt: cfg.FacilityTypesAt, + concurrency: searchConcurrency(cfg.SearchConcurrency), }, closer, nil } diff --git a/pkg/plugin/implementation/AgricultureFacility/cmd/plugin.go b/pkg/plugin/implementation/AgricultureFacility/cmd/plugin.go index 25a13ef3..d2ecb2be 100644 --- a/pkg/plugin/implementation/AgricultureFacility/cmd/plugin.go +++ b/pkg/plugin/implementation/AgricultureFacility/cmd/plugin.go @@ -35,6 +35,9 @@ func (p agriFacilityProvider) parseConfig(config map[string]string) (*Agricultur // this is a default rather than something to set. ProviderIDAt: config["providerIdAt"], CapabilityCodeAt: config["capabilityCodeAt"], + // Absent means the Beckn v2 convention. See + // AgricultureFacility.DefaultFacilityTypesAt. + FacilityTypesAt: config["facilityTypesAt"], } // One credential profile per provider, read from the flattened diff --git a/pkg/plugin/implementation/AgricultureFacility/payload.go b/pkg/plugin/implementation/AgricultureFacility/payload.go index 3f85ffba..89eb36f0 100644 --- a/pkg/plugin/implementation/AgricultureFacility/payload.go +++ b/pkg/plugin/implementation/AgricultureFacility/payload.go @@ -23,77 +23,83 @@ package AgricultureFacility import ( "fmt" "slices" + "strings" "github.com/beckn-one/beckn-onix/pkg/model" + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" ) +// DefaultFacilityTypesAt is where a Beckn v2 select carries the facility types +// a search asks for. +// +// A PATH rather than a walk written out in Go, for the same reason +// common.Config's providerIdAt and capabilityCodeAt are paths: the payload's +// shape is the network's convention, not this adapter's. Compiled in, a spec +// change needs a rebuild; as a path, it needs a config edit. The default is +// what every deployment should be using, and facilityTypesAt exists so a spec +// change can be tracked without waiting for a release. +const DefaultFacilityTypesAt = "message.contract.commitments[].resources[]." + + "resourceAttributes.supportedFacilityTypes[]" + // facilityTypesFrom returns the facility types a payload asks for, in the // order the payload wrote them -- one upstream call each. // +// The reading is common.ValuesAt's, the same walker that finds a binding key, +// so the type assertions and the absent-field handling are in one place rather +// than repeated here. It returns only the strings the path reaches: a missing +// field, an object where a list belongs, or a number among the types all +// arrive as "no value at that path", which is a bad request either way. +// // Every failure is a bad request rather than an adapter fault: the payload is // the thing that is wrong, and the caller is the only one who can fix it. The // mapping's own required: checks refuse most of these first, with better -// messages; these are what is left if a check is relaxed or a payload reaches -// here another way, and they exist so that path is a refusal rather than a -// panic on a nil map. -func facilityTypesFrom(beckn any) ([]string, error) { - document, ok := beckn.(map[string]any) - if !ok { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload is %T, not an object, so it names no commitment to search from", beckn)) - } - - commitments, _ := dig(document, "message", "contract", "commitments").([]any) - if len(commitments) == 0 { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload carries no commitment to read a facility search from")) - } - commitment, ok := commitments[0].(map[string]any) - if !ok { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload's first commitment is %T, not an object", commitments[0])) +// messages; these are what is left if a check is relaxed. +func facilityTypesFrom(beckn any, path string) ([]string, error) { + if path == "" { + path = DefaultFacilityTypesAt } - resources, _ := commitment["resources"].([]any) - if len(resources) == 0 { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload's commitment carries no resource to read a facility search from")) - } - resource, ok := resources[0].(map[string]any) - if !ok { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload's first resource is %T, not an object", resources[0])) - } - - declared := dig(resource, "resourceAttributes", "supportedFacilityTypes") + // LeavesAt, not ValuesAt: a non-string here is the caller's mistake and has + // to be reported. ValuesAt would drop it, and dropping one entry of a + // hand-written list means searching for the rest and reporting success -- + // a partial answer with nothing recording what was lost. + leaves := common.LeavesAt(beckn, path) // A bare string is one value: the same instruction as a one-element list, - // which is what a caller writing a single type may well send. - var declaredList []any - switch typed := declared.(type) { - case nil: - declaredList = nil - case []any: - declaredList = typed - default: - declaredList = []any{typed} + // which is what a caller writing a single type may well send. The path's + // "[]" suffix wants a list, so the scalar form is a second read rather than + // a looser walker -- keeping "[]" meaning exactly "a list" everywhere. + // + // A list that is PRESENT and empty reads as zero leaves too, and the second + // read would then hand back the empty list itself as one value -- reported + // as "[] is not a facility type", which is true and useless. Skipping a + // leaf that is itself a list keeps that case on the "names no facility + // type" message, which is what an empty list means. + if len(leaves) == 0 { + for _, leaf := range common.LeavesAt(beckn, strings.TrimSuffix(path, arrayMarker)) { + if _, isList := leaf.([]any); isList { + continue + } + leaves = append(leaves, leaf) + } } - if len(declaredList) == 0 { + + if len(leaves) == 0 { return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload names no facility type in supportedFacilityTypes, "+ - "so there is nothing to ask the provider for")) + "agriculture facility: the payload names no facility type at %s, "+ + "so there is nothing to ask the provider for", path)) } // Each value becomes a category code in the request half's $codes lookup, // so a non-string is a request this capability cannot build -- refused // here rather than sent as a null category POCRA answers with everything. - values := make([]string, 0, len(declaredList)) - for _, value := range declaredList { - text, ok := value.(string) + values := make([]string, 0, len(leaves)) + for _, leaf := range leaves { + text, ok := leaf.(string) if !ok { return nil, model.NewBadReqErr("", fmt.Errorf( "agriculture facility: supportedFacilityTypes contains %v (%T), which is not a facility type", - value, value)) + leaf, leaf)) } values = append(values, text) } @@ -117,6 +123,9 @@ func facilityTypesFrom(beckn any) ([]string, error) { return values, nil } +// arrayMarker is common.ValuesAt's "look in each element" suffix. +const arrayMarker = "[]" + // dig walks a chain of object keys, returning nil the moment one is absent or // is not an object. Written out rather than reached for from a library because // a mistyped key must read as an absent field, which is a bad request, and not diff --git a/pkg/plugin/implementation/AgricultureFacility/payload_test.go b/pkg/plugin/implementation/AgricultureFacility/payload_test.go index c58701d6..7465d78d 100644 --- a/pkg/plugin/implementation/AgricultureFacility/payload_test.go +++ b/pkg/plugin/implementation/AgricultureFacility/payload_test.go @@ -10,8 +10,11 @@ package AgricultureFacility import ( "encoding/json" + "reflect" "strings" "testing" + + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" ) // decode is what Step.Run works from: the payload, already decoded into @@ -34,7 +37,7 @@ func TestFacilityTypesFromReadsThemInPayloadOrder(t *testing.T) { beckn := decode(t, `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ "supportedFacilityTypes":["KrishiVigyanKendra","Warehouse","SoilTestingFacility"]}}]}]}}}`) - values, err := facilityTypesFrom(beckn) + values, err := facilityTypesFrom(beckn, "") if err != nil { t.Fatalf("facilityTypesFrom() returned an unexpected error: %v", err) } @@ -56,7 +59,7 @@ func TestFacilityTypesFromAcceptsASingleType(t *testing.T) { beckn := decode(t, `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ "supportedFacilityTypes":["Warehouse"]}}]}]}}}`) - values, err := facilityTypesFrom(beckn) + values, err := facilityTypesFrom(beckn, "") if err != nil { t.Fatalf("facilityTypesFrom() returned an unexpected error: %v", err) } @@ -75,7 +78,7 @@ func TestFacilityTypesFromAcceptsABareString(t *testing.T) { beckn := decode(t, `{"message":{"contract":{"commitments":[{"resources":[{"resourceAttributes":{ "supportedFacilityTypes":"KrishiVigyanKendra"}}]}]}}}`) - values, err := facilityTypesFrom(beckn) + values, err := facilityTypesFrom(beckn, "") if err != nil { t.Fatalf("facilityTypesFrom() returned an unexpected error: %v", err) } @@ -109,7 +112,7 @@ func TestFacilityTypesFromRefusesWhatItCannotRead(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - values, err := facilityTypesFrom(decode(t, tc.body)) + values, err := facilityTypesFrom(decode(t, tc.body), "") if err == nil { t.Fatalf("facilityTypesFrom() returned %v, want %s refused", values, tc.name) } @@ -126,8 +129,36 @@ func TestFacilityTypesFromRefusesANonObjectPayload(t *testing.T) { t.Parallel() for _, beckn := range []any{nil, "a string", 42.0, []any{"a", "list"}} { - if _, err := facilityTypesFrom(beckn); err == nil { + if _, err := facilityTypesFrom(beckn, ""); err == nil { t.Errorf("facilityTypesFrom(%v) was accepted, want it refused", beckn) } } } + +// THE POINT OF THE PATH. A payload that carries the types somewhere else is a +// config edit, not a rebuild: the same reading works against a different shape +// when facilityTypesAt says where to look. +// +// This is what the hand-written walk could not do. It knew +// message.contract.commitments[0].resources[0].resourceAttributes as Go code, +// so a spec change meant editing and shipping a binary. +func TestFacilityTypesFollowAConfiguredPath(t *testing.T) { + t.Parallel() + + // A shape the default path would find nothing in. + body := `{"search":{"facilities":[ + {"kinds":["Warehouse","KrishiVigyanKendra"]}]}}` + + if got := common.LeavesAt(decode(t, body), DefaultFacilityTypesAt); len(got) != 0 { + t.Fatalf("the default path found %v in a payload it should not read", got) + } + + values, err := facilityTypesFrom(decode(t, body), "search.facilities[].kinds[]") + if err != nil { + t.Fatalf("facilityTypesFrom() with a configured path: %v", err) + } + want := []string{"Warehouse", "KrishiVigyanKendra"} + if !reflect.DeepEqual(values, want) { + t.Errorf("values = %v, want %v", values, want) + } +} diff --git a/pkg/plugin/implementation/AgricultureFacility/search.go b/pkg/plugin/implementation/AgricultureFacility/search.go index ddcd9b41..452ea2c3 100644 --- a/pkg/plugin/implementation/AgricultureFacility/search.go +++ b/pkg/plugin/implementation/AgricultureFacility/search.go @@ -92,6 +92,10 @@ type Step struct { paths common.Paths bindingKeys []string + // facilityTypesAt is where the payload carries the types to search for. + // A path rather than a walk in Go, so the payload's shape stays config. + facilityTypesAt string + // concurrency is how many of a split search's calls run at once. concurrency int } @@ -113,7 +117,7 @@ func (s *Step) Run(ctx *model.StepContext) error { "agriculture facility: the payload is not JSON: %w", err)) } - types, err := facilityTypesFrom(beckn) + types, err := facilityTypesFrom(beckn, s.facilityTypesAt) if err != nil { return err } diff --git a/pkg/plugin/implementation/internal/common/binding.go b/pkg/plugin/implementation/internal/common/binding.go index 8d657d32..4a325306 100644 --- a/pkg/plugin/implementation/internal/common/binding.go +++ b/pkg/plugin/implementation/internal/common/binding.go @@ -64,8 +64,8 @@ func BindingFrom(paths Paths, body []byte) (Binding, error) { commitments) } - providers := distinct(valuesAt(payload, paths.ProviderID)) - types := distinct(valuesAt(payload, paths.CapabilityCode)) + providers := distinct(ValuesAt(payload, paths.ProviderID)) + types := distinct(ValuesAt(payload, paths.CapabilityCode)) if len(providers) == 0 || len(types) == 0 { return Binding{}, errNoBinding diff --git a/pkg/plugin/implementation/internal/common/binding_test.go b/pkg/plugin/implementation/internal/common/binding_test.go index d0195221..11552cce 100644 --- a/pkg/plugin/implementation/internal/common/binding_test.go +++ b/pkg/plugin/implementation/internal/common/binding_test.go @@ -248,13 +248,13 @@ func TestPathWalk(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got := valuesAt(doc, tc.path) + got := ValuesAt(doc, tc.path) if len(got) != len(tc.want) { - t.Fatalf("valuesAt(%q) = %v, want %v", tc.path, got, tc.want) + t.Fatalf("ValuesAt(%q) = %v, want %v", tc.path, got, tc.want) } for i, want := range tc.want { if got[i] != want { - t.Errorf("valuesAt(%q)[%d] = %q, want %q", tc.path, i, got[i], want) + t.Errorf("ValuesAt(%q)[%d] = %q, want %q", tc.path, i, got[i], want) } } }) diff --git a/pkg/plugin/implementation/internal/common/paths.go b/pkg/plugin/implementation/internal/common/paths.go index af1ded59..96491903 100644 --- a/pkg/plugin/implementation/internal/common/paths.go +++ b/pkg/plugin/implementation/internal/common/paths.go @@ -45,13 +45,28 @@ func (p Paths) Validate() error { return nil } -// valuesAt collects every string the path reaches. +// ValuesAt collects every string the path reaches. // // The grammar is two things: segments separated by ".", and a "[]" suffix // meaning "look in each element". No wildcards, filters or indices -- each is // another way to write something subtly wrong in config nobody reviews. -func valuesAt(node any, path string) []string { - return walk(node, strings.Split(path, ".")) +// +// EXPORTED for a domain package that has to read its own field out of a +// payload -- AgricultureFacility reads supportedFacilityTypes this way. The +// alternative is hand-walking the document in Go, which compiles the payload's +// shape into the binary: a spec change then needs a rebuild rather than a +// config edit, and the traversal has to re-check every type assertion the +// walker already checks. +func ValuesAt(node any, path string) []string { + var found []string + for _, leaf := range walk(node, strings.Split(path, ".")) { + // Only strings are binding-key material; anything else means the path + // landed somewhere unintended. + if value, ok := leaf.(string); ok { + found = append(found, value) + } + } + return found } // countAt reports how many elements the first array segment holds, whether or @@ -83,14 +98,24 @@ func countAt(node any, path string) int { return 0 } -func walk(node any, segments []string) []string { +// LeavesAt collects every value the path reaches, whatever its type. +// +// ValuesAt's sibling, and the one a caller wants when a non-string at the leaf +// is a MISTAKE to report rather than a leaf to skip. ValuesAt drops it, which +// is right for a binding key -- a number where a capability code belongs means +// the path landed somewhere unintended, and there is nothing to say about it +// beyond "no binding here". It is wrong for a list the caller wrote by hand: +// dropping one entry of supportedFacilityTypes would search for the rest and +// report success, which is a partial answer with nothing recording the loss. +func LeavesAt(node any, path string) []any { + return walk(node, strings.Split(path, ".")) +} + +// walk collects the leaves a path reaches. Type policy is the caller's: this +// returns what is there, ValuesAt keeps the strings, LeavesAt keeps everything. +func walk(node any, segments []string) []any { if len(segments) == 0 { - // Only strings are binding-key material; anything else means the path - // landed somewhere unintended. - if value, ok := node.(string); ok { - return []string{value} - } - return nil + return []any{node} } segment := segments[0] @@ -114,7 +139,7 @@ func walk(node any, segments []string) []string { if !ok { return nil } - var found []string + var found []any for _, element := range elements { found = append(found, walk(element, rest)...) } From dcfeb03037923b2778dc2d7dd10c6afcc3cd8fb4 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Thu, 10 Sep 2026 01:35:23 +0530 Subject: [PATCH 3/8] refactor(AgricultureFacility): the mapping derives its own request id [#91] The plugin stamped a fresh UUID on every part it split out, so POCRA would not blend the answers. That put the reason in Go and the consequence in the mapping, which then had a comment explaining what the Go code had done for it. The mapping derives the id itself now, from the caller's messageId with its last hex digit replaced by a slot the facility type owns. The TYPE picks the slot rather than a call index, because every payload reaching the request half names exactly one type -- so no index has to be passed, and a retry reuses its own slot instead of taking a fresh one. WHY THE CALLER'S OWN ID CANNOT BE SENT, verified live against POCRA: its cache is keyed on message_id, holds for PT10M, and IGNORES LOCATION. The same id asked twice 350km apart returned the first location's facility alongside the second's, which no type filter can catch because the type matches. Reusing it would answer a farmer in Nagpur with a facility near Ahmednagar. Replacing the last character leaves a valid v4 UUID -- it sits in the node field, not the version or variant nibbles -- which POCRA's schema requires. Derived rather than random because JSONata here has no $uuid. That deletes, in order: the UUID stamping, the merged answer's messageId restore (parts never lose the caller's id now), messageIDOf, the uuid import, and dig -- the last hand-written traversal in this package's production code. splitByType writes through the SAME configured path the types were read from, so the read and the write cannot disagree about where they live. setAt and containersAt are local to this package, not added to common: common's step reads payloads and never rewrites one, and a write raises questions a read does not. They create nothing, so a path that does not resolve is a refusal rather than a payload invented to fit. Their own TestEachSearchCallCarriesItsOwnRequestId already asserted the exact property this change has to keep -- two calls, distinct ids, both valid UUIDs, neither the caller's. Negative control: giving two types the same slot fails it with "both calls used message_id ... so POCRA would blend their answers". 69 packages green, vet clean, no races. --- .../pocra/agriculture-facility.select.yaml | 40 ++++++--- .../AgricultureFacility/payload.go | 80 +++++++++++++++--- .../AgricultureFacility/search.go | 81 +++++-------------- 3 files changed, 116 insertions(+), 85 deletions(-) diff --git a/config/mappings/pocra/agriculture-facility.select.yaml b/config/mappings/pocra/agriculture-facility.select.yaml index 44aee0fe..9d98f3b3 100644 --- a/config/mappings/pocra/agriculture-facility.select.yaml +++ b/config/mappings/pocra/agriculture-facility.select.yaml @@ -116,6 +116,33 @@ request: | "Warehouse": "warehouse", "SoilTestingFacility": "soil_lab" }; + /* THIS CALL'S OWN message_id, derived from the caller's by replacing its + last hex digit with a slot this facility type owns. + + POCRA caches per message_id for PT10M and returns the UNION of + everything asked under it, so two calls sharing an id each come back + carrying the other's facilities. Worse, that cache IGNORES LOCATION: + verified live, the same id asked twice 350km apart returned the first + location's facility alongside the second's, which no type filter can + catch because the type matches. So the caller's own id cannot be sent + as it stands. + + The last character of a v4 UUID is in the node field, not the version + or variant nibbles, so replacing it leaves a valid UUID -- which POCRA's + schema requires. Derived rather than random because this expression has + no random source: JSONata here has no $uuid. + + The TYPE picks the slot, not a call index, because each payload reaching + here names exactly one type. That also makes a retry reuse its own slot + rather than take a fresh one. */ + $slots := { + "CustomHiringCentre": "0", + "KrishiVigyanKendra": "1", + "Warehouse": "2", + "SoilTestingFacility": "3" + }; + $callerId := beckn.context.messageId; + $messageId := $substring($callerId, 0, 35) & $lookup($slots, $requested); { "context": { "domain": "advisory:mh-vistaar", @@ -124,16 +151,9 @@ request: | "version": "1.1.0", "bap_id": "bap.mahapocra.gov.in", "bap_uri": "https://middleware.mahapocra.gov.in/bap/", - /* This call's own id. POCRA keeps a message_id's answers for ten - minutes and returns the UNION of everything asked for under it, so - two calls sharing an id would each return the other's facilities - too. - - Read straight off the payload: the plugin stamps a fresh UUID on - context.messageId of every single-type payload it splits out, for - exactly this reason, so each call already carries its own. POCRA's - schema refuses a message_id that is not a UUID. */ - "message_id": beckn.context.messageId, + /* Derived above, one slot per facility type. NOT the caller's own id: + POCRA's cache is keyed on it and ignores location. */ + "message_id": $messageId, "transaction_id": beckn.context.transactionId, "timestamp": $string($floor($millis() / 1000)) }, diff --git a/pkg/plugin/implementation/AgricultureFacility/payload.go b/pkg/plugin/implementation/AgricultureFacility/payload.go index 89eb36f0..d913a5c9 100644 --- a/pkg/plugin/implementation/AgricultureFacility/payload.go +++ b/pkg/plugin/implementation/AgricultureFacility/payload.go @@ -41,6 +41,14 @@ import ( const DefaultFacilityTypesAt = "message.contract.commitments[].resources[]." + "resourceAttributes.supportedFacilityTypes[]" +// CommitmentsAt is where a Beckn contract carries its commitments -- the level +// mergeAnswers appends one answer's resources onto another's. +// +// Not configurable, unlike DefaultFacilityTypesAt: this reads the answers this +// package's own mapping produced, not a payload a caller wrote, so it moves +// only when that mapping does and the two are edited together. +const CommitmentsAt = "message.contract.commitments[]" + // facilityTypesFrom returns the facility types a payload asks for, in the // order the payload wrote them -- one upstream call each. // @@ -126,18 +134,64 @@ func facilityTypesFrom(beckn any, path string) ([]string, error) { // arrayMarker is common.ValuesAt's "look in each element" suffix. const arrayMarker = "[]" -// dig walks a chain of object keys, returning nil the moment one is absent or -// is not an object. Written out rather than reached for from a library because -// a mistyped key must read as an absent field, which is a bad request, and not -// as a panic. -func dig(document map[string]any, keys ...string) any { - var current any = document - for _, key := range keys { - object, ok := current.(map[string]any) - if !ok { - return nil - } - current = object[key] +// setAt writes value at the leaf the path names, creating nothing. +// +// The mirror of common.ValuesAt, and the same grammar, so ONE configured path +// -- facilityTypesAt -- both reads the types out of a payload and narrows them +// in a part. Two paths that had to agree would be a way for them to disagree. +// +// Local to this package rather than in common: common's step reads a payload +// and never rewrites one, and a write raises questions a read does not -- +// whether to create the intermediates, what an array segment means when +// assigning. This answers both narrowly. It creates nothing, so a path that +// does not already resolve is a refusal rather than a payload invented to fit, +// and an array segment writes to EVERY element, which is what narrowing a +// search to one type means when a payload carries several commitments. +// +// Reported as a bad request: the payload is what is wrong, and the caller is +// the only one who can fix it. +func setAt(document any, path string, value any) error { + segments := strings.Split(path, ".") + targets := containersAt(document, segments[:len(segments)-1]) + if len(targets) == 0 { + return model.NewBadReqErr("", fmt.Errorf( + "agriculture facility: the payload has nothing at %s to narrow to one facility type", + strings.Join(segments[:len(segments)-1], "."))) + } + leaf := strings.TrimSuffix(segments[len(segments)-1], arrayMarker) + for _, target := range targets { + target[leaf] = value + } + return nil +} + +// containersAt collects every object the path reaches, so setAt has somewhere +// to write. Absent or wrongly-typed reaches nothing, which setAt reports. +func containersAt(node any, segments []string) []map[string]any { + object, ok := node.(map[string]any) + if !ok { + return nil + } + if len(segments) == 0 { + return []map[string]any{object} + } + + segment := segments[0] + child, present := object[strings.TrimSuffix(segment, arrayMarker)] + if !present { + return nil + } + if !strings.HasSuffix(segment, arrayMarker) { + return containersAt(child, segments[1:]) + } + + elements, ok := child.([]any) + if !ok { + return nil + } + var found []map[string]any + for _, element := range elements { + found = append(found, containersAt(element, segments[1:])...) } - return current + return found } diff --git a/pkg/plugin/implementation/AgricultureFacility/search.go b/pkg/plugin/implementation/AgricultureFacility/search.go index 452ea2c3..39a7d11d 100644 --- a/pkg/plugin/implementation/AgricultureFacility/search.go +++ b/pkg/plugin/implementation/AgricultureFacility/search.go @@ -29,8 +29,7 @@ import ( "encoding/json" "fmt" "slices" - - "github.com/google/uuid" + "strings" "github.com/beckn-one/beckn-onix/pkg/log" "github.com/beckn-one/beckn-onix/pkg/model" @@ -131,7 +130,7 @@ func (s *Step) Run(ctx *model.StepContext) error { // the fresh message id each part carries is what stops POCRA blending the // answers, and a search of one still wants its own id rather than the // caller's. See splitByType. - parts, err := splitByType(ctx.Body, types) + parts, err := splitByType(ctx.Body, types, s.facilityTypesAt) if err != nil { return err } @@ -163,7 +162,7 @@ func (s *Step) Run(ctx *model.StepContext) error { return err } - merged, err := mergeAnswers(answers, messageIDOf(beckn)) + merged, err := mergeAnswers(answers) if err != nil { return err } @@ -218,7 +217,11 @@ func (s *Step) mine(body []byte) bool { // Re-decoded per part rather than deep-copied: the parts are mutated // independently and a shared nested map would have them overwrite each other's // type. A payload is a few kilobytes and this happens once per request. -func splitByType(body []byte, types []string) ([][]byte, error) { +func splitByType(body []byte, types []string, path string) ([][]byte, error) { + if path == "" { + path = DefaultFacilityTypesAt + } + parts := make([][]byte, 0, len(types)) for _, facilityType := range types { var part map[string]any @@ -227,44 +230,19 @@ func splitByType(body []byte, types []string) ([][]byte, error) { "agriculture facility: the payload is not a JSON object: %w", err)) } - attributes, ok := dig(part, "message", "contract", "commitments").([]any) - if !ok || len(attributes) == 0 { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload carries no commitment to split")) - } - commitment, ok := attributes[0].(map[string]any) - if !ok { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload's first commitment is %T, not an object", attributes[0])) - } - resources, ok := commitment["resources"].([]any) - if !ok || len(resources) == 0 { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload's commitment carries no resource to split")) - } - resource, ok := resources[0].(map[string]any) - if !ok { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload's first resource is %T, not an object", resources[0])) - } - resourceAttributes, ok := resource["resourceAttributes"].(map[string]any) - if !ok { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload's first resource carries no resourceAttributes")) - } - // A list of one, not a bare string: the required: checks and the // request half both read this through [] and a scalar would work, but // a part that does not look like the payload it came from is a trap // for whoever reads one in a log. - resourceAttributes["supportedFacilityTypes"] = []any{facilityType} - - becknContext, ok := part["context"].(map[string]any) - if !ok { - return nil, model.NewBadReqErr("", fmt.Errorf( - "agriculture facility: the payload carries no context to stamp a message id on")) + // + // Written at the SAME path the types were read from, so the two cannot + // disagree about where they live. No message id is stamped here: the + // request half derives POCRA's own from the caller's id and this + // part's type, which leaves the caller's id on the part where a Beckn + // answer wants it. + if err := setAt(part, path, []any{facilityType}); err != nil { + return nil, err } - becknContext["messageId"] = uuid.NewString() encoded, err := json.Marshal(part) if err != nil { @@ -295,7 +273,7 @@ func splitByType(body []byte, types []string) ([][]byte, error) { // attribute and the mapping drops it before this sees the answers. A globally // ranked multi-type search would need the mapping to publish a distance this // could sort on, which the schema pack says it must not. -func mergeAnswers(answers [][]byte, callerMessageID string) ([]byte, error) { +func mergeAnswers(answers [][]byte) ([]byte, error) { if len(answers) == 0 { return nil, fmt.Errorf("agriculture facility: nothing to merge; this search made no calls") } @@ -353,10 +331,6 @@ func mergeAnswers(answers [][]byte, callerMessageID string) ([]byte, error) { offer["resourceIds"] = ids } - if becknContext, ok := merged["context"].(map[string]any); ok && callerMessageID != "" { - becknContext["messageId"] = callerMessageID - } - encoded, err := json.Marshal(merged) if err != nil { return nil, fmt.Errorf("agriculture facility: could not encode the merged answer: %w", err) @@ -366,28 +340,11 @@ func mergeAnswers(answers [][]byte, callerMessageID string) ([]byte, error) { // commitmentOf reaches the one commitment an answer carries. func commitmentOf(document map[string]any) (map[string]any, error) { - commitments, _ := dig(document, "message", "contract", "commitments").([]any) + commitments := containersAt(document, strings.Split(CommitmentsAt, ".")) if len(commitments) == 0 { return nil, fmt.Errorf("agriculture facility: an answer carries no commitment to merge") } - commitment, ok := commitments[0].(map[string]any) - if !ok { - return nil, fmt.Errorf("agriculture facility: an answer's commitment is %T, not an object", commitments[0]) - } - return commitment, nil -} - -// messageIDOf reads the caller's own message id, for the merged answer to -// correlate with. Absent is not an error: the mapping copies whatever the -// payload carried, so an absent one stays absent, as it did before splitting -// existed. -func messageIDOf(beckn any) string { - document, ok := beckn.(map[string]any) - if !ok { - return "" - } - id, _ := dig(document, "context", "messageId").(string) - return id + return commitments[0], nil } // searchConcurrency resolves the deployment's setting, clamped to From 1cb385e8c47c9e259ff70f4945a63b0923163d19 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Thu, 10 Sep 2026 02:07:18 +0530 Subject: [PATCH 4/8] refactor(AgricultureFacility): bound the fan-out with errgroup, not a package [#91] internal/concurrent was 116 lines of generic bounded fail-fast machinery plus 288 lines of tests to run one loop, with a single caller, at a configured concurrency of 1. golang.org/x/sync is already a dependency and errgroup does the same job here in a method that reads alongside the code it serves. The clamp moves into searchConcurrency and gains a reason: SetLimit reads a non-positive limit as UNBOUNDED, so a misread setting would fan out over every facility type at once against a provider that rate-limits into silent empty answers. concurrent.Bound treated zero the same way; the comment now says why it matters at the point where it is enforced. Order is kept by indexing the answers rather than appending, so each slot is written by exactly one goroutine and the merge does not depend on which call finished first. No behaviour change: the five search tests that pin sequential-by-default, the configured limit, fail-fast skipping calls not yet issued, one-type-one-call and the ceiling all pass unmodified, under -race. --- .../AgricultureFacility/search.go | 90 ++++-- .../internal/concurrent/concurrent.go | 116 ------- .../internal/concurrent/concurrent_test.go | 288 ------------------ 3 files changed, 58 insertions(+), 436 deletions(-) delete mode 100644 pkg/plugin/implementation/internal/concurrent/concurrent.go delete mode 100644 pkg/plugin/implementation/internal/concurrent/concurrent_test.go diff --git a/pkg/plugin/implementation/AgricultureFacility/search.go b/pkg/plugin/implementation/AgricultureFacility/search.go index 39a7d11d..3cfbea88 100644 --- a/pkg/plugin/implementation/AgricultureFacility/search.go +++ b/pkg/plugin/implementation/AgricultureFacility/search.go @@ -11,12 +11,11 @@ // one-payload-one-call step over each of them concurrently, and merge the // answers back into one. // -// Nothing below this file knows any of that. internal/upstream serves one -// payload with one call and has no notion of splitting; jsonmapper compiles -// the two halves every mapping has and no third thing; internal/concurrent -// runs N of anything, bounded and ordered, and has never heard of Beckn. The -// mapping this runs is written for a single-type payload, which is what it is -// always handed. +// Nothing below this file knows any of that. internal/common serves one +// payload with one call and has no notion of splitting, and jsonmapper +// compiles the two halves every mapping has and no third thing. The mapping +// this runs is written for a single-type payload, which is what it is always +// handed. // // Separated from the package clause by a blank line on purpose: the package's // own doc comment is in AgricultureFacility.go, and this is a note about one @@ -25,17 +24,17 @@ package AgricultureFacility import ( - "context" "encoding/json" "fmt" "slices" "strings" + "golang.org/x/sync/errgroup" + "github.com/beckn-one/beckn-onix/pkg/log" "github.com/beckn-one/beckn-onix/pkg/model" "github.com/beckn-one/beckn-onix/pkg/plugin/definition" "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common" - "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/concurrent" ) const ( @@ -138,26 +137,7 @@ func (s *Step) Run(ctx *model.StepContext) error { log.Debugf(ctx, "agriculture facility: serving %d facility type(s) as %d call(s), %d at a time", len(types), len(parts), s.concurrency) - // Bounded, ordered and fail-fast, which is internal/concurrent's whole - // job. One failure fails the request: a partial answer is the defect this - // was built to fix wearing a different hat -- the caller asked for four - // facility types, would receive three, and nothing in the payload would - // say that the fourth was asked for and lost. - answers, err := concurrent.Map(ctx, parts, s.concurrency, - func(callCtx context.Context, part []byte) ([]byte, error) { - // Its own StepContext: inner reads Body and writes ResponseBody, - // so the parts must not share either. callCtx rather than ctx so a - // sibling's failure cancels this call too. - partCtx := *ctx - partCtx.Context = callCtx - partCtx.Body = part - partCtx.ResponseBody = nil - - if err := s.runPart(&partCtx); err != nil { - return nil, err - } - return partCtx.ResponseBody, nil - }) + answers, err := s.callEach(ctx, parts) if err != nil { return err } @@ -177,6 +157,46 @@ func (s *Step) Run(ctx *model.StepContext) error { // recognised can only be a mismatch between this step's binding keys and // inner's. Reported rather than merged, because merging nothing would answer // a four-type search with three types and no error. +// callEach runs one upstream call per part, at most s.concurrency at a time, +// and returns the answers in the parts' order. +// +// Fail-fast, and that is the point: one failure fails the whole request. A +// partial answer is the defect this split was built to fix wearing a different +// hat -- the caller asked for four facility types, would receive three, and +// nothing in the payload would say the fourth was asked for and lost. +func (s *Step) callEach(ctx *model.StepContext, parts [][]byte) ([][]byte, error) { + // Indexed rather than appended, so the answers keep the parts' order + // whichever call finishes first. Each slot is written by exactly one + // goroutine, which is what makes this safe without a lock. + answers := make([][]byte, len(parts)) + + group, callCtx := errgroup.WithContext(ctx.Context) + group.SetLimit(s.concurrency) + + for index, part := range parts { + group.Go(func() error { + // Its own StepContext: inner reads Body and writes ResponseBody, + // so the parts must not share either. callCtx rather than + // ctx.Context so a sibling's failure cancels this call too. + partCtx := *ctx + partCtx.Context = callCtx + partCtx.Body = part + partCtx.ResponseBody = nil + + if err := s.runPart(&partCtx); err != nil { + return err + } + answers[index] = partCtx.ResponseBody + return nil + }) + } + + if err := group.Wait(); err != nil { + return nil, err + } + return answers, nil +} + func (s *Step) runPart(partCtx *model.StepContext) error { if err := s.inner.Run(partCtx); err != nil { return err @@ -351,9 +371,15 @@ func commitmentOf(document map[string]any) (map[string]any, error) { // MaxFacilityTypes and defaulting to DefaultSearchConcurrency when left unset // or non-positive. // -// The arithmetic is internal/concurrent's, because every caller of it needs -// the same clamp and a zero limit there means UNBOUNDED. The three numbers are -// this package's, because each one is a fact about POCRA. +// The clamp is not decoration: errgroup.SetLimit takes a non-positive limit to +// mean UNBOUNDED, so a misread setting would fan out over every facility type +// at once against a provider that rate-limits into silent empty answers. func searchConcurrency(configured int) int { - return concurrent.Bound(configured, DefaultSearchConcurrency, MaxFacilityTypes) + if configured <= 0 { + return DefaultSearchConcurrency + } + if configured > MaxFacilityTypes { + return MaxFacilityTypes + } + return configured } diff --git a/pkg/plugin/implementation/internal/concurrent/concurrent.go b/pkg/plugin/implementation/internal/concurrent/concurrent.go deleted file mode 100644 index 7bd9b231..00000000 --- a/pkg/plugin/implementation/internal/concurrent/concurrent.go +++ /dev/null @@ -1,116 +0,0 @@ -// Package concurrent is one generic engine: run a known number of -// independent calls, bounded, with the first failure cancelling the rest. -// -// It knows nothing about Beckn, upstream providers, or any specific -// capability. Every policy question -- how many is too many, what identity -// each call carries, what a failure should say -- belongs to whoever calls -// this, not to this package: see -// pkg/plugin/implementation/AgricultureFacility/search.go for the one caller -// that exists today. -// -// pkg/plugin/implementation/catalogpublisher's own runConcurrent looks -// similar but is NOT this: it collects every call's error rather than -// cancelling on the first one, because its caller needs to inspect all of -// them to decide whether any is worth retrying. That is a different -// concurrency shape, kept separate rather than forced into one function -// with a mode flag. -package concurrent - -import ( - "context" - "fmt" - - "golang.org/x/sync/errgroup" -) - -// Run calls work(ctx, i) for every i in [0, n), at most limit at a time, and -// returns each call's result at its own index -- in the order Run was asked -// for them, not the order they finished, so a caller can rely on results[i] -// meaning "the i-th thing I asked for" regardless of which one answered -// first. -// -// The first error any call returns cancels the ctx handed to every other -// call, in flight or not yet started: a call not yet issued when an earlier -// one already failed is skipped rather than made and discarded, and an -// in-flight call that itself respects ctx cancellation (as an -// http.NewRequestWithContext-built request does) is aborted rather than run -// to completion for an answer nobody will use. Run itself returns that first -// error, wrapped by nothing -- whatever work returned is what the caller -// gets, so the caller's own wrapping (naming which of the n things failed, -// what identity it carried) survives unchanged. What each call's own input or -// identity looks like is entirely work's business -- Run passes only an -// index, so it never has to know or care what a caller maps that index onto. -// -// n == 0 calls work zero times and returns (nil, nil). limit is passed -// straight to errgroup.Group.SetLimit -- see its own doc for what a -// non-positive value means. -func Run[T any](ctx context.Context, n, limit int, work func(ctx context.Context, index int) (T, error)) ([]T, error) { - if n == 0 { - return nil, nil - } - - group, groupCtx := errgroup.WithContext(ctx) - group.SetLimit(limit) - - results := make([]T, n) - for index := range n { - group.Go(func() error { - if groupCtx.Err() != nil { - return nil - } - result, err := work(groupCtx, index) - if err != nil { - return fmt.Errorf("%w", err) - } - results[index] = result - return nil - }) - } - - if err := group.Wait(); err != nil { - return nil, err - } - return results, nil -} - -// Map is Run over a list of VALUES rather than a count: one call per value, -// results in the order the values were given. -// -// The same bounded, ordered, fail-fast guarantees as Run -- it is Run, with -// the index-to-value step done here instead of in every caller. Prefer it -// whenever the work is "one call per thing in this list". -// -// values is []V and results are []T, so a caller keeps whatever types it -// already has; nothing here inspects either. -// -// What Map does NOT decide, on purpose: how many values are too many, and -// what to tell a caller who asked for too many. A ceiling is a fact about -// whatever is being called and its refusal is that domain's error to write, -// so it belongs to the package that knows the domain. Use Bound below for the -// other half of the arithmetic, defaulting and clamping a configured -// concurrency. -func Map[V, T any](ctx context.Context, values []V, limit int, - call func(ctx context.Context, value V) (T, error)) ([]T, error) { - - return Run(ctx, len(values), limit, func(ctx context.Context, index int) (T, error) { - return call(ctx, values[index]) - }) -} - -// Bound resolves a configured concurrency: fallback when it is unset or -// non-positive, ceiling when it is over. -// -// Here rather than in each caller because every caller of Run or Map needs -// the same three lines, and getting them wrong is quiet: a zero limit means -// UNBOUNDED in errgroup, so a deployment that leaves the setting out would get -// every call at once from a mistake that reads like a default. -func Bound(configured, fallback, ceiling int) int { - limit := configured - if limit <= 0 { - limit = fallback - } - if limit > ceiling { - limit = ceiling - } - return limit -} diff --git a/pkg/plugin/implementation/internal/concurrent/concurrent_test.go b/pkg/plugin/implementation/internal/concurrent/concurrent_test.go deleted file mode 100644 index a35d4c4f..00000000 --- a/pkg/plugin/implementation/internal/concurrent/concurrent_test.go +++ /dev/null @@ -1,288 +0,0 @@ -package concurrent - -import ( - "context" - "errors" - "fmt" - "sync" - "sync/atomic" - "testing" - "time" -) - -func TestRunReturnsResultsInOrderNotArrivalOrder(t *testing.T) { - t.Parallel() - - // index 0 is the slowest, so an arrival-ordered result would put it - // last -- this proves the result is indexed, not appended. - delays := []time.Duration{30 * time.Millisecond, 10 * time.Millisecond, 0} - results, err := Run(context.Background(), len(delays), len(delays), - func(ctx context.Context, i int) (int, error) { - time.Sleep(delays[i]) - return i, nil - }) - if err != nil { - t.Fatalf("Run() returned an unexpected error: %v", err) - } - for i, got := range results { - if got != i { - t.Errorf("results[%d] = %d, want %d", i, got, i) - } - } -} - -func TestRunIsSequentialWhenLimitIsOne(t *testing.T) { - t.Parallel() - - var mu sync.Mutex - var inFlight, peak int - _, err := Run(context.Background(), 4, 1, func(ctx context.Context, i int) (struct{}, error) { - mu.Lock() - inFlight++ - if inFlight > peak { - peak = inFlight - } - mu.Unlock() - - time.Sleep(10 * time.Millisecond) - - mu.Lock() - inFlight-- - mu.Unlock() - return struct{}{}, nil - }) - if err != nil { - t.Fatalf("Run() returned an unexpected error: %v", err) - } - if peak != 1 { - t.Errorf("peak concurrency = %d, want 1", peak) - } -} - -func TestRunHonoursALimitAboveOne(t *testing.T) { - t.Parallel() - - var mu sync.Mutex - var inFlight, peak int - _, err := Run(context.Background(), 6, 2, func(ctx context.Context, i int) (struct{}, error) { - mu.Lock() - inFlight++ - if inFlight > peak { - peak = inFlight - } - mu.Unlock() - - time.Sleep(15 * time.Millisecond) - - mu.Lock() - inFlight-- - mu.Unlock() - return struct{}{}, nil - }) - if err != nil { - t.Fatalf("Run() returned an unexpected error: %v", err) - } - if peak != 2 { - t.Errorf("peak concurrency = %d, want the configured 2", peak) - } -} - -func TestRunReturnsTheCallsOwnError(t *testing.T) { - t.Parallel() - - sentinel := errors.New("boom") - _, err := Run(context.Background(), 3, 3, func(ctx context.Context, i int) (int, error) { - if i == 1 { - return 0, fmt.Errorf("call %d failed: %w", i, sentinel) - } - return i, nil - }) - if err == nil || !errors.Is(err, sentinel) { - t.Fatalf("error = %v, want it to wrap the call's own error", err) - } -} - -func TestRunSkipsCallsNotYetIssuedAfterAFailureWhenSequential(t *testing.T) { - t.Parallel() - - var called int32 - _, err := Run(context.Background(), 3, 1, func(ctx context.Context, i int) (struct{}, error) { - atomic.AddInt32(&called, 1) - if i == 0 { - return struct{}{}, errors.New("first call fails") - } - return struct{}{}, nil - }) - if err == nil { - t.Fatal("Run() returned no error, want the first call's failure") - } - // Sequential (limit 1): index 0 fails and cancels the shared context - // before index 1's call is ever issued, so only index 0 was called. - if got := atomic.LoadInt32(&called); got != 1 { - t.Errorf("work was called %d times, want 1 -- calls not yet issued "+ - "when an earlier one fails must be skipped, not made and discarded", got) - } -} - -func TestRunOfZeroReturnsNoResultsAndNoError(t *testing.T) { - t.Parallel() - - results, err := Run(context.Background(), 0, 4, func(ctx context.Context, i int) (int, error) { - t.Error("work was called for n == 0") - return 0, nil - }) - if err != nil { - t.Fatalf("Run() returned an unexpected error: %v", err) - } - if len(results) != 0 { - t.Errorf("results = %v, want empty", results) - } -} - -func TestRunOfOneRunsExactlyOnce(t *testing.T) { - t.Parallel() - - var called int32 - results, err := Run(context.Background(), 1, 1, func(ctx context.Context, i int) (string, error) { - atomic.AddInt32(&called, 1) - return "ok", nil - }) - if err != nil { - t.Fatalf("Run() returned an unexpected error: %v", err) - } - if got := atomic.LoadInt32(&called); got != 1 { - t.Errorf("work was called %d times, want 1", got) - } - if len(results) != 1 || results[0] != "ok" { - t.Errorf("results = %v, want [\"ok\"]", results) - } -} - -func TestRunCancelsTheContextItHandsToInFlightWork(t *testing.T) { - t.Parallel() - - var slowCtxDone int32 - // inFlight is closed by index 1 once it is genuinely running, and index 0 - // waits for that before failing. Without the handshake this test is a - // race: index 1's call may not have started when index 0 fails, and Run - // then SKIPS it rather than cancelling it -- correct behaviour, and what - // TestRunSkipsCallsNotYetIssuedAfterAFailureWhenSequential is for, but - // not what this test is about. - inFlight := make(chan struct{}) - _, err := Run(context.Background(), 2, 2, func(ctx context.Context, i int) (struct{}, error) { - if i == 0 { - <-inFlight - return struct{}{}, errors.New("fails once the other call is in flight") - } - close(inFlight) - // In flight when index 0 fails: wait for our own ctx to be cancelled, - // or give up after a bound that would mean it never was. A bound - // rather than a bare receive so a broken Run fails this test instead - // of hanging it. - select { - case <-ctx.Done(): - atomic.AddInt32(&slowCtxDone, 1) - case <-time.After(2 * time.Second): - } - return struct{}{}, nil - }) - if err == nil { - t.Fatal("Run() returned no error, want index 0's failure") - } - if atomic.LoadInt32(&slowCtxDone) != 1 { - t.Error("the in-flight call's context was not cancelled after the other call failed") - } -} - -// --- Map -------------------------------------------------------------------- - -// Map hands each value to its own call and returns the answers in the order -// the values were given, whatever order the calls finished in. -func TestMapCallsOncePerValueInOrder(t *testing.T) { - t.Parallel() - - values := []string{"kvk", "warehouse", "soil_lab"} - results, err := Map(context.Background(), values, 3, func(ctx context.Context, value string) (string, error) { - // The first value sleeps longest, so arrival order is the reverse of - // the order asked for. A caller relying on results[i] meaning "the - // answer for values[i]" has to hold regardless. - if value == "kvk" { - time.Sleep(40 * time.Millisecond) - } - return "answered:" + value, nil - }) - if err != nil { - t.Fatalf("Map() returned an unexpected error: %v", err) - } - want := []string{"answered:kvk", "answered:warehouse", "answered:soil_lab"} - if len(results) != len(want) { - t.Fatalf("results = %v, want %v", results, want) - } - for index := range want { - if results[index] != want[index] { - t.Errorf("results[%d] = %q, want %q -- value order, not arrival order", - index, results[index], want[index]) - } - } -} - -// An empty list makes no calls and is not an error: a caller deciding that an -// empty list is a bad request says so itself, in its own words. -func TestMapOfNoValuesMakesNoCalls(t *testing.T) { - t.Parallel() - - results, err := Map(context.Background(), nil, 2, func(ctx context.Context, value string) (int, error) { - t.Error("a call was made for an empty value list") - return 0, nil - }) - if err != nil { - t.Fatalf("Map() returned an unexpected error: %v", err) - } - if len(results) != 0 { - t.Errorf("results = %v, want empty", results) - } -} - -// One value's failure fails the whole call, and the value's own error is what -// comes back -- the caller named which value it was and why. -func TestMapFailsWhenOneValueFails(t *testing.T) { - t.Parallel() - - sentinel := errors.New("warehouse is unreachable") - _, err := Map(context.Background(), []string{"kvk", "warehouse"}, 1, - func(ctx context.Context, value string) (string, error) { - if value == "warehouse" { - return "", sentinel - } - return "ok", nil - }) - if !errors.Is(err, sentinel) { - t.Errorf("error = %v, want the failing value's own error", err) - } -} - -// --- Bound ------------------------------------------------------------------ - -// Bound is the arithmetic every caller of Run and Map needs: a configured -// limit, a fallback when it is unset, a ceiling when it is over. Zero must -// become the fallback and not reach errgroup, where it means UNBOUNDED. -func TestBoundDefaultsAndClamps(t *testing.T) { - t.Parallel() - - for _, tc := range []struct { - name string - configured, fallback, ceiling, want int - }{ - {"unset takes the fallback", 0, 1, 8, 1}, - {"negative takes the fallback", -4, 2, 8, 2}, - {"within range is honoured", 4, 1, 8, 4}, - {"exactly the ceiling is honoured", 8, 1, 8, 8}, - {"over the ceiling is clamped", 64, 1, 8, 8}, - {"a fallback over the ceiling is clamped too", 0, 16, 8, 8}, - } { - if got := Bound(tc.configured, tc.fallback, tc.ceiling); got != tc.want { - t.Errorf("%s: Bound(%d, %d, %d) = %d, want %d", - tc.name, tc.configured, tc.fallback, tc.ceiling, got, tc.want) - } - } -} From 803d81c6d81e51b42357ad6edbb6282ed3618235 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Thu, 10 Sep 2026 02:07:34 +0530 Subject: [PATCH 5/8] chore: stop ignoring /docs/, which would hide real documentation [#91] Both lines were added by the facility plugin's branch and ignore nothing: no docs/ or dev_docs/ directory exists and neither has a tracked file. /docs/ is worse than unnecessary -- it is the conventional place for documentation, so the first person to add any there would find it silently uncommittable, with a rule in an unrelated feature branch to explain it. testdata/.gitignore stays. It ignores schema-cache/, which does exist, and is how schemacache_test.go keeps the fetched schema pack out of the repo instead of vendoring a copy. --- .gitignore | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index 1134973c..6292bb58 100644 --- a/.gitignore +++ b/.gitignore @@ -176,8 +176,4 @@ test_request.json .claude # catalogPublish handler's local output root (config/local-beckn-one-bap.yaml) -/catalog/ - -# Local, non-shipped developer notes (review summaries, scratch docs) -/dev_docs/ -/docs/ \ No newline at end of file +/catalog/ \ No newline at end of file From 99d08740e300aa5024a1b0d9b3b9e05c6db4f401 Mon Sep 17 00:00:00 2001 From: Kelvin Date: Thu, 10 Sep 2026 11:13:32 +0530 Subject: [PATCH 6/8] chore: add __MACOSX/ to .gitignore to prevent macOS system files from being tracked --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 6292bb58..0e87cdd2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ # Logs .DS_Store +__MACOSX/ logs .idea *.log From 233793ff8ea8d5e4aea99f5e0cba9b1bdfc7de25 Mon Sep 17 00:00:00 2001 From: Kelvin Date: Thu, 10 Sep 2026 12:07:46 +0530 Subject: [PATCH 7/8] chore: add /catalog/, /docs/, and /dev_docs/ to .gitignore --- .gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 0e87cdd2..f91664b1 100644 --- a/.gitignore +++ b/.gitignore @@ -177,4 +177,6 @@ test_request.json .claude # catalogPublish handler's local output root (config/local-beckn-one-bap.yaml) -/catalog/ \ No newline at end of file +/catalog/ +/docs/ +/dev_docs/ \ No newline at end of file From 7b056ad44340d1fbef0d1559f548f332db03b2e3 Mon Sep 17 00:00:00 2001 From: ameersohel45 Date: Thu, 10 Sep 2026 12:32:47 +0530 Subject: [PATCH 8/8] feat(common): add a tokenQuery auth scheme for providers that issue their own [#91] For an upstream that mints short-lived tokens from its own endpoint rather than an OAuth2 one. The live Agmarknet Vistaar API is the case: POST a JSON body of credentials, get {"token":""} back, send it as a QUERY PARAMETER. Neither existing scheme can serve that, and bending one would have been wrong in five separate ways: oauth2 posts form-encoded client_id/client_secret, reads access_token and expires_in, sets Authorization: Bearer tokenQuery posts JSON under CONFIGURED keys, reads a CONFIGURED key, sets a query parameter -- and the response carries NO expiry The field names are configured rather than fixed because access_name and password are one provider's spelling, not a standard. WHY tokenTtl IS REQUIRED. The response says nothing about how long the token lives, so there is nothing to read and guessing would be inventing a lifetime. An operator states what they believe instead. A ttl at or below the refresh skew is refused: every token would already be expired on arrival, making two round trips per request and hitting the token endpoint at the request rate. AND WHY A WRONG tokenTtl IS NOT AN OUTAGE. Because the lifetime is an estimate, a too-generous one leaves a dead token cached -- and without help every call would fail until it lapsed. A 401 or 403 from the provider now drops the held token, so the next call exchanges a fresh one. One request pays; the next recovers. This applies to oauth2 too, where an issuer can revoke a token before its expires_in runs out. REDACTION follows the placement, not the scheme it borrows from. A query string is the exposed spot -- proxies log it and Go's transport errors quote the whole URL -- so the token's URL-ESCAPED form is covered as well, or one containing + or = survives redaction. The identifier is left readable, as a client id is: it identifies, it does not authenticate. The HTTP half of the exchange is now shared by both schemes in postForToken. The interesting part there is the retry decision -- unreachable, unreadable, 5xx and 429 retry; a 4xx is configuration -- and two copies of that would drift. VERIFIED AGAINST THE LIVE PROVIDER, not just in unit tests: exchanged a real 36-character UUID from the actual token endpoint, placed it as ?token=, and confirmed the log line reads token=REDACTED. That check read the endpoint and credentials from the environment and is not committed -- no IP and no credential is written down anywhere. Twenty test cases, and the self-heal one carries a negative control: with the forgetToken call removed it fails with "the rejected token was reused instead of re-exchanged", so it is testing the behaviour rather than passing by accident. Full suite green under -race. --- .../implementation/internal/common/auth.go | 84 +++- .../implementation/internal/common/http.go | 13 + .../implementation/internal/common/redact.go | 19 + .../implementation/internal/common/token.go | 156 ++++++-- .../internal/common/tokenquery_test.go | 366 ++++++++++++++++++ .../internal/common/util/constant.go | 26 +- 6 files changed, 626 insertions(+), 38 deletions(-) create mode 100644 pkg/plugin/implementation/internal/common/tokenquery_test.go diff --git a/pkg/plugin/implementation/internal/common/auth.go b/pkg/plugin/implementation/internal/common/auth.go index e53eb4e6..4c780112 100644 --- a/pkg/plugin/implementation/internal/common/auth.go +++ b/pkg/plugin/implementation/internal/common/auth.go @@ -11,6 +11,7 @@ import ( "strings" "sync" "sync/atomic" + "time" "github.com/beckn-one/beckn-onix/pkg/log" "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common/util" @@ -26,7 +27,7 @@ type AuthProfile struct { // operator guessing which block to look at. Provider string - // One of none, basic, header, query or oauth2. + // One of none, basic, header, query, oauth2 or tokenQuery. Scheme string // Variable NAMES, never the values. @@ -47,6 +48,30 @@ type AuthProfile struct { // Variable names. The values never appear in config, the registry or a log. ClientIDEnv string ClientSecretEnv string + + // tokenQuery only. The JSON keys the token endpoint expects, and the + // variables holding what goes under them. + // + // The KEYS are configured because they are one provider's spelling rather + // than a standard: the live one this was written against wants + // access_name and password, and the next will want something else. + TokenUserField string + TokenUserEnv string + TokenSecretName string + TokenSecretEnv string + + // Where the token sits in the response body, as a top-level key. + TokenResponseField string + + // How long a token may be held, as written in config and as parsed. + // + // Configured because the response carries no expiry at all -- nothing in it + // says how long the token lives, so an operator states what they believe + // and this refuses to guess. Set it too long and calls fail with the + // provider's own rejection until it lapses; the cache is dropped on such a + // rejection so the next call re-exchanges rather than waiting that out. + TokenTTLRaw string + TokenTTL time.Duration } // authenticator is one provider's profile plus the token it holds. @@ -91,13 +116,43 @@ func (a *AuthProfile) validate() error { "%s: authScheme oauth2 requires tokenUrl, clientIdEnv and clientSecretEnv", a.Provider) } + case util.AuthSchemeTokenQuery: + // Every one of these is required, and none can be defaulted: the JSON + // keys, the response key and the parameter name are all this + // provider's spelling, and guessing any of them sends a malformed + // request whose rejection says nothing about the cause. + if a.TokenURL == "" || a.TokenUserField == "" || a.TokenUserEnv == "" || + a.TokenSecretName == "" || a.TokenSecretEnv == "" || + a.TokenResponseField == "" || a.QueryName == "" { + return fmt.Errorf("%s: authScheme tokenQuery requires tokenUrl, "+ + "tokenUserField, tokenUserEnv, tokenSecretField, tokenSecretEnv, "+ + "tokenResponseField and queryName", a.Provider) + } + if a.TokenTTLRaw == "" { + return fmt.Errorf("%s: authScheme tokenQuery requires tokenTtl, "+ + "because the token response carries no expiry to read", a.Provider) + } + ttl, err := time.ParseDuration(a.TokenTTLRaw) + if err != nil { + return fmt.Errorf("%s: tokenTtl %q is not a duration: %w", + a.Provider, a.TokenTTLRaw, err) + } + // Below the skew every token is already expired when it arrives, so + // each call would exchange one and then discard it -- two round trips + // per request and a token endpoint hit at the request rate. + if ttl <= util.TokenRefreshSkew { + return fmt.Errorf("%s: tokenTtl %s must exceed the %s refresh skew, "+ + "or every token is expired before it is used", + a.Provider, ttl, util.TokenRefreshSkew) + } + a.TokenTTL = ttl case "": return fmt.Errorf("%s: authScheme is required, "+ "and is none where the upstream needs no credential", a.Provider) default: return fmt.Errorf( - "%s: unknown authScheme %q: must be none, basic, header, query or oauth2", - a.Provider, a.Scheme) + "%s: unknown authScheme %q: must be none, basic, header, query, "+ + "oauth2 or tokenQuery", a.Provider, a.Scheme) } return nil } @@ -181,6 +236,16 @@ var authFields = map[string]func(*AuthProfile, string){ "tokenUrl": func(p *AuthProfile, v string) { p.TokenURL = v }, "clientIdEnv": func(p *AuthProfile, v string) { p.ClientIDEnv = v }, "clientSecretEnv": func(p *AuthProfile, v string) { p.ClientSecretEnv = v }, + + "tokenUserField": func(p *AuthProfile, v string) { p.TokenUserField = v }, + "tokenUserEnv": func(p *AuthProfile, v string) { p.TokenUserEnv = v }, + "tokenSecretField": func(p *AuthProfile, v string) { p.TokenSecretName = v }, + "tokenSecretEnv": func(p *AuthProfile, v string) { p.TokenSecretEnv = v }, + "tokenResponseField": func(p *AuthProfile, v string) { p.TokenResponseField = v }, + // Kept as written and turned into a duration by validate, so a bad value is + // refused at startup naming the provider rather than read as zero -- which + // would mean re-exchanging a token on every single call. + "tokenTtl": func(p *AuthProfile, v string) { p.TokenTTLRaw = v }, } // providerIDFrom returns the provider half of "|". @@ -236,11 +301,22 @@ func (s *Step) authenticate(auth *authenticator, req *http.Request) error { query.Set(cfg.QueryName, value) req.URL.RawQuery = query.Encode() case util.AuthSchemeOAuth2: - token, err := s.bearerToken(req.Context(), auth) + token, err := s.providerToken(req.Context(), auth) if err != nil { return err } req.Header.Set("Authorization", "Bearer "+token) + + case util.AuthSchemeTokenQuery: + // Exchanged like oauth2, placed like query: the same held token, put + // where this provider reads it from. + token, err := s.providerToken(req.Context(), auth) + if err != nil { + return err + } + query := req.URL.Query() + query.Set(cfg.QueryName, token) + req.URL.RawQuery = query.Encode() } return nil } diff --git a/pkg/plugin/implementation/internal/common/http.go b/pkg/plugin/implementation/internal/common/http.go index 56304098..d558ca00 100644 --- a/pkg/plugin/implementation/internal/common/http.go +++ b/pkg/plugin/implementation/internal/common/http.go @@ -116,6 +116,19 @@ func (s *Step) attempt(ctx context.Context, auth *authenticator, call model.Acti // quoted back, credential and all. log.Warnf(ctx, "provider returned %s for %s %s: %s", resp.Status, method, requested, s.redactString(util.Explain(body))) + + // A held token the provider has stopped accepting is dropped, so the + // next call exchanges a fresh one. + // + // This is what keeps a wrong tokenTtl from being an outage. The + // lifetime is an operator's ESTIMATE -- the endpoint states no expiry + // -- so a too-generous one leaves a dead token cached, and without this + // every call would fail until it lapsed. One request pays; the next + // recovers. + if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { + s.forgetToken(auth) + } + err := fmt.Errorf("provider returned %s", resp.Status) // 5xx and 429 ask to be tried again. Every other 4xx is a statement // about the request, which will not improve. diff --git a/pkg/plugin/implementation/internal/common/redact.go b/pkg/plugin/implementation/internal/common/redact.go index 155c2d28..6d59711f 100644 --- a/pkg/plugin/implementation/internal/common/redact.go +++ b/pkg/plugin/implementation/internal/common/redact.go @@ -129,6 +129,25 @@ func (a *authenticator) secretForms() []string { forms = append(forms, encoded) } return forms + case util.AuthSchemeTokenQuery: + // oauth2's two halves, with query's escaping: the secret goes to the + // token endpoint, and the token it returns travels in a QUERY STRING -- + // which is the exposed placement, so its escaped form has to be + // covered too or a token containing "+" or "=" survives redaction. + var forms []string + if secret := os.Getenv(a.cfg.TokenSecretEnv); secret != "" { + forms = append(forms, secret) + } + // Read without tokenMu: reached from inside the exchange, which holds it. + if held := a.token.Load(); held != nil && held.value != "" { + forms = append(forms, held.value) + if encoded := url.QueryEscape(held.value); encoded != held.value { + forms = append(forms, encoded) + } + } + // tokenUserField's value is NOT redacted, for the same reason a client + // id is not: it identifies, it does not authenticate. + return forms } return nil } diff --git a/pkg/plugin/implementation/internal/common/token.go b/pkg/plugin/implementation/internal/common/token.go index 93ded54b..093cafa0 100644 --- a/pkg/plugin/implementation/internal/common/token.go +++ b/pkg/plugin/implementation/internal/common/token.go @@ -1,4 +1,5 @@ -// The oauth2 client_credentials exchange and the token each provider holds. +// The token exchanges -- oauth2 client_credentials and tokenQuery -- and the +// token each provider holds. package common import ( @@ -17,16 +18,21 @@ import ( "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common/util" ) -// bearerToken returns a token to send, exchanging one if the held token has +// providerToken returns a token to send, exchanging one if the held token has // expired or there is none. // +// Scheme-neutral on purpose: it holds a value and an expiry and knows nothing +// about where the token came from or where it is going. oauth2 sends the +// result in a header, tokenQuery in a query parameter, and both want exactly +// this caching. +// // The lock spans the fetch on purpose: without it a cold start sends every // concurrent request to the issuer at once. It costs one wait per token // lifetime. // // NOTHING IS CACHED ON FAILURE, so a brief outage at the token endpoint does // not leave this step holding a failure for a lifetime it never obtained. -func (s *Step) bearerToken(ctx context.Context, auth *authenticator) (string, error) { +func (s *Step) providerToken(ctx context.Context, auth *authenticator) (string, error) { if held := auth.token.Load(); held != nil && time.Now().Before(held.expiry) { return held.value, nil } @@ -53,18 +59,91 @@ type cachedToken struct { expiry time.Time } +// forgetToken drops a held token, so the next call exchanges a fresh one. +// +// Called when the provider itself rejects the credential, which is the only +// authority on whether a token is still good: an expiry held here is a claim +// about the future -- read from expires_in, or estimated by an operator in +// tokenTtl -- and the provider can disagree with it at any time. +// +// A no-op for a scheme that holds nothing, so the caller does not have to ask +// which scheme it is. +func (s *Step) forgetToken(auth *authenticator) { + if auth == nil { + return + } + auth.token.Store(nil) +} + // tokenResponse is the part of an OAuth2 token response this step reads. type tokenResponse struct { AccessToken string `json:"access_token"` ExpiresIn int `json:"expires_in"` } -// exchangeToken performs the client_credentials grant. +// exchangeToken obtains a token, by whichever exchange the scheme names. // // Every failure carries 502, because it is the exchange with the provider's // issuer that failed. Unclassified they would surface as 500, telling a peer // this adapter broke when it did not. func (s *Step) exchangeToken(ctx context.Context, auth *authenticator) (string, time.Duration, error) { + if auth.cfg.Scheme == util.AuthSchemeTokenQuery { + return s.exchangeQueryToken(ctx, auth) + } + return s.exchangeOAuth2Token(ctx, auth) +} + +// exchangeQueryToken POSTs a JSON body of configured field names and reads the +// token out of a configured response field. +// +// The lifetime comes from config, not from the response: this endpoint says +// nothing about how long its token lives. That is the whole reason tokenTtl is +// required rather than defaulted -- see AuthProfile. +func (s *Step) exchangeQueryToken(ctx context.Context, auth *authenticator) (string, time.Duration, error) { + cfg := auth.cfg + user, secret := os.Getenv(cfg.TokenUserEnv), os.Getenv(cfg.TokenSecretEnv) + if user == "" || secret == "" { + return "", 0, s.missingCredential(ctx, cfg.Provider, util.AuthSchemeTokenQuery, + cfg.TokenUserEnv+" and "+cfg.TokenSecretEnv) + } + + // Marshalled rather than concatenated, so a credential containing a quote + // or a backslash cannot break out of the JSON it travels in. + payload, err := json.Marshal(map[string]string{ + cfg.TokenUserField: user, + cfg.TokenSecretName: secret, + }) + if err != nil { + return "", 0, s.permanentTokenErr( + fmt.Errorf("token request body could not be built: %w", err)) + } + + body, err := s.postForToken(ctx, cfg, payload, "application/json") + if err != nil { + return "", 0, err + } + + // Decoded into a map because the field name is configured: a struct tag + // cannot be written for a key that is not known until config is read. + var fields map[string]any + if err := json.Unmarshal(body, &fields); err != nil { + return "", 0, s.permanentTokenErr(fmt.Errorf("token response from %s is not JSON: %w", + cfg.TokenURL, err)) + } + token, ok := fields[cfg.TokenResponseField].(string) + if !ok || token == "" { + return "", 0, s.permanentTokenErr(fmt.Errorf( + "token response from %s carries no %s", cfg.TokenURL, cfg.TokenResponseField)) + } + + // The skew is subtracted here for the same reason oauth2 subtracts it from + // expires_in: a request that passed the expiry check must not arrive after + // the token died. validate has already refused a ttl at or below it. + return token, cfg.TokenTTL - util.TokenRefreshSkew, nil +} + +// exchangeOAuth2Token performs the client_credentials grant. +func (s *Step) exchangeOAuth2Token(ctx context.Context, auth *authenticator) (string, time.Duration, error) { cfg := auth.cfg clientID, clientSecret := os.Getenv(cfg.ClientIDEnv), os.Getenv(cfg.ClientSecretEnv) if clientID == "" || clientSecret == "" { @@ -77,18 +156,51 @@ func (s *Step) exchangeToken(ctx context.Context, auth *authenticator) (string, "client_id": {clientID}, "client_secret": {clientSecret}, } + body, err := s.postForToken(ctx, cfg, []byte(form.Encode()), + "application/x-www-form-urlencoded") + if err != nil { + return "", 0, err + } + + var parsed tokenResponse + if err := json.Unmarshal(body, &parsed); err != nil { + return "", 0, s.permanentTokenErr(fmt.Errorf("token response from %s is not JSON: %w", + cfg.TokenURL, err)) + } + if parsed.AccessToken == "" { + return "", 0, s.permanentTokenErr( + fmt.Errorf("token response from %s carries no access_token", cfg.TokenURL)) + } + lifetime, ok := tokenLifetime(parsed.ExpiresIn, util.TokenRefreshSkew) + if !ok { + return "", 0, s.permanentTokenErr(fmt.Errorf( + "token response from %s carries no usable expires_in, so its lifetime is unknown", + cfg.TokenURL)) + } + return parsed.AccessToken, lifetime, nil +} + +// postForToken sends the exchange and returns the body of a 2xx, classifying +// every failure the same way for both schemes. +// +// Shared deliberately: the retry decision is the interesting part -- an +// unreachable endpoint, an unreadable response, a 5xx and a 429 are worth +// another attempt, and a 4xx is configuration that will fail identically next +// time. Two copies of that would drift. +func (s *Step) postForToken(ctx context.Context, cfg AuthProfile, payload []byte, + contentType string) ([]byte, error) { req, err := http.NewRequestWithContext(ctx, http.MethodPost, cfg.TokenURL, - strings.NewReader(form.Encode())) + strings.NewReader(string(payload))) if err != nil { // A tokenUrl that will not parse is configuration. - return "", 0, s.permanentTokenErr( + return nil, s.permanentTokenErr( fmt.Errorf("token request could not be built: %w", err)) } - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Content-Type", contentType) resp, err := s.httpClient.Do(req) if err != nil { - return "", 0, s.tokenErr(fmt.Errorf("token endpoint %s could not be reached: %w", + return nil, s.tokenErr(fmt.Errorf("token endpoint %s could not be reached: %w", cfg.TokenURL, err)) } defer resp.Body.Close() @@ -96,7 +208,7 @@ func (s *Step) exchangeToken(ctx context.Context, auth *authenticator) (string, // Bounded like any other upstream read. body, err := io.ReadAll(io.LimitReader(resp.Body, s.config.MaxResponseBytes+1)) if err != nil { - return "", 0, s.tokenErr(fmt.Errorf("token response could not be read: %w", err)) + return nil, s.tokenErr(fmt.Errorf("token response could not be read: %w", err)) } if resp.StatusCode < 200 || resp.StatusCode >= 300 { // The status, not the body: a failure body routinely quotes the @@ -106,32 +218,14 @@ func (s *Step) exchangeToken(ctx context.Context, auth *authenticator) (string, err := fmt.Errorf("token endpoint returned %s", resp.Status) // The same rule the provider's own status gets: 5xx and 429 ask to be // tried again, and every other 4xx is a statement about the request -- - // here, usually that the client credentials are wrong. + // here, usually that the credentials are wrong. if resp.StatusCode < http.StatusInternalServerError && resp.StatusCode != http.StatusTooManyRequests { - return "", 0, s.permanentTokenErr(err) + return nil, s.permanentTokenErr(err) } - return "", 0, s.tokenErr(err) + return nil, s.tokenErr(err) } - - var parsed tokenResponse - if err := json.Unmarshal(body, &parsed); err != nil { - // A 2xx that is not JSON is the issuer misbehaving in a way another - // attempt will not change. - return "", 0, s.permanentTokenErr(fmt.Errorf("token response from %s is not JSON: %w", - cfg.TokenURL, err)) - } - if parsed.AccessToken == "" { - return "", 0, s.permanentTokenErr( - fmt.Errorf("token response from %s carries no access_token", cfg.TokenURL)) - } - lifetime, ok := tokenLifetime(parsed.ExpiresIn, util.TokenRefreshSkew) - if !ok { - return "", 0, s.permanentTokenErr(fmt.Errorf( - "token response from %s carries no usable expires_in, so its lifetime is unknown", - cfg.TokenURL)) - } - return parsed.AccessToken, lifetime, nil + return body, nil } // tokenErr classifies a failed exchange. Always 502: the failure is the diff --git a/pkg/plugin/implementation/internal/common/tokenquery_test.go b/pkg/plugin/implementation/internal/common/tokenquery_test.go new file mode 100644 index 00000000..45496fac --- /dev/null +++ b/pkg/plugin/implementation/internal/common/tokenquery_test.go @@ -0,0 +1,366 @@ +// tokenQuery: the exchange, the placement, the held token and its redaction. +// +// A separate file from common_test.go on purpose -- that one is already past +// two thousand lines, and everything here is about one scheme. +package common + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/beckn-one/beckn-onix/pkg/plugin/implementation/internal/common/util" +) + +// queryTokenServer stands in for a provider's own token endpoint: JSON in, a +// token under a name of its choosing out. +type queryTokenServer struct { + calls atomic.Int32 + field string // the response key; "token" when empty + token string // rotates per call when empty + status int + body string // overrides the JSON when set + gotUser string + gotSecret string + gotType string +} + +func (s *queryTokenServer) start(t *testing.T) string { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + count := s.calls.Add(1) + s.gotType = r.Header.Get("Content-Type") + + raw, _ := io.ReadAll(r.Body) + var sent map[string]string + _ = json.Unmarshal(raw, &sent) + s.gotUser, s.gotSecret = sent["access_name"], sent["password"] + + if s.status != 0 { + w.WriteHeader(s.status) + } + if s.body != "" { + fmt.Fprint(w, s.body) + return + } + token := s.token + if token == "" { + token = fmt.Sprintf("tok-%d", count) + } + field := s.field + if field == "" { + field = "token" + } + fmt.Fprintf(w, `{%q:%q}`, field, token) + })) + t.Cleanup(server.Close) + return server.URL +} + +// tokenQueryProfile is the fully-populated profile, for tests that then blank +// one field to prove it is required. +func tokenQueryProfile(tokenURL string) AuthProfile { + return AuthProfile{ + Scheme: util.AuthSchemeTokenQuery, + TokenURL: tokenURL, + TokenUserField: "access_name", + TokenUserEnv: "TEST_TQ_USER", + TokenSecretName: "password", + TokenSecretEnv: "TEST_TQ_SECRET", + TokenResponseField: "token", + QueryName: "token", + TokenTTLRaw: "10m", + } +} + +// tokenQueryStep records the query string each provider call arrived with. +func tokenQueryStep(t *testing.T, tokenURL string, seen *[]string, + status *int, tweak ...func(*AuthProfile)) *Step { + t.Helper() + provider := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *seen = append(*seen, r.URL.RawQuery) + if status != nil && *status != 0 { + w.WriteHeader(*status) + fmt.Fprintf(w, `{"error":"Invalid token"}`) + return + } + fmt.Fprint(w, `{"ok":true}`) + })) + t.Cleanup(provider.Close) + + return newStep(t, &stubRegistry{plan: testPlan(provider.URL, http.MethodGet)}, + &stubMapper{requestResult: []byte(`{}`), responseResult: []byte(`{"a":1}`)}, + func(c *Config) { + profile := tokenQueryProfile(tokenURL) + for _, apply := range tweak { + apply(&profile) + } + // validate normally runs at startup; the profile carries a parsed + // TTL by the time a call is made. + if err := profile.validate(); err != nil { + t.Fatalf("profile does not validate: %v", err) + } + c.setProviderAuth(profile) + }) +} + +// The whole point of the scheme: credentials POSTed as JSON under CONFIGURED +// keys, and the token that comes back placed in the QUERY STRING. Neither half +// matches oauth2, which is why bending that scheme could not serve it. +func TestTokenQuerySendsTheExchangedTokenAsAQueryParameter(t *testing.T) { + t.Setenv("TEST_TQ_USER", "Masters-Data-Provider") + t.Setenv("TEST_TQ_SECRET", "the-password") + + ts := &queryTokenServer{token: "the-token"} + var seen []string + step := tokenQueryStep(t, ts.start(t), &seen, nil) + + if _, err := runStep(t, step, selectBody); err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + if len(seen) != 1 || !strings.Contains(seen[0], "token=the-token") { + t.Errorf("provider saw query %q, want it to carry token=the-token", seen) + } + if ts.gotType != "application/json" { + t.Errorf("token endpoint saw Content-Type %q, want application/json", ts.gotType) + } + if ts.gotUser != "Masters-Data-Provider" || ts.gotSecret != "the-password" { + t.Errorf("token endpoint saw %q/%q under access_name/password, want the configured pair", + ts.gotUser, ts.gotSecret) + } +} + +// The token is held for its configured lifetime. Without this the provider's +// token endpoint takes an extra round trip on every single call. +func TestTokenQueryReusesTheTokenWithinItsTTL(t *testing.T) { + t.Setenv("TEST_TQ_USER", "user") + t.Setenv("TEST_TQ_SECRET", "secret") + + ts := &queryTokenServer{} + var seen []string + step := tokenQueryStep(t, ts.start(t), &seen, nil) + + for i := 0; i < 5; i++ { + if _, err := runStep(t, step, selectBody); err != nil { + t.Fatalf("request %d: %v", i, err) + } + } + if got := ts.calls.Load(); got != 1 { + t.Errorf("token endpoint called %d times for 5 requests, want 1", got) + } + for i, query := range seen { + if !strings.Contains(query, "token=tok-1") { + t.Errorf("request %d sent %q, want the cached token", i, query) + } + } +} + +// THE SELF-HEAL. The lifetime is an operator's estimate -- the endpoint states +// no expiry -- so a too-generous tokenTtl leaves a dead token cached. Without +// dropping it on the provider's own rejection, every call fails until the ttl +// lapses, which for a ten-minute guess is a ten-minute outage. +func TestTokenQueryForgetsATokenTheProviderRejects(t *testing.T) { + t.Setenv("TEST_TQ_USER", "user") + t.Setenv("TEST_TQ_SECRET", "secret") + + ts := &queryTokenServer{} + var seen []string + status := http.StatusForbidden + step := tokenQueryStep(t, ts.start(t), &seen, &status) + + // First call: the provider rejects the freshly exchanged token. + if _, err := runStep(t, step, selectBody); err == nil { + t.Fatal("Run() succeeded against a 403, want an error") + } + if got := ts.calls.Load(); got != 1 { + t.Fatalf("token endpoint called %d times, want 1", got) + } + + // Second call: the held token must NOT be reused, even though the ttl has + // not lapsed. It is a fresh exchange or the scheme cannot recover. + status = 0 + if _, err := runStep(t, step, selectBody); err != nil { + t.Fatalf("second Run() = %v, want it to recover with a fresh token", err) + } + if got := ts.calls.Load(); got != 2 { + t.Errorf("token endpoint called %d times over two requests, want 2 -- "+ + "the rejected token was reused instead of re-exchanged", got) + } + if len(seen) != 2 || strings.Contains(seen[1], "token=tok-1") { + t.Errorf("second request sent %q, want a token other than the rejected tok-1", seen) + } +} + +// Every field is required, and the message has to name what is missing: an +// operator reading "requires tokenUrl, tokenUserField, ..." can fix it, while +// a malformed request rejected by the provider says nothing about the cause. +func TestTokenQueryRefusesAnIncompleteProfile(t *testing.T) { + for _, tc := range []struct { + field string + blank func(*AuthProfile) + }{ + {"tokenUrl", func(p *AuthProfile) { p.TokenURL = "" }}, + {"tokenUserField", func(p *AuthProfile) { p.TokenUserField = "" }}, + {"tokenUserEnv", func(p *AuthProfile) { p.TokenUserEnv = "" }}, + {"tokenSecretField", func(p *AuthProfile) { p.TokenSecretName = "" }}, + {"tokenSecretEnv", func(p *AuthProfile) { p.TokenSecretEnv = "" }}, + {"tokenResponseField", func(p *AuthProfile) { p.TokenResponseField = "" }}, + {"queryName", func(p *AuthProfile) { p.QueryName = "" }}, + } { + t.Run(tc.field, func(t *testing.T) { + profile := tokenQueryProfile("https://issuer.example/token") + profile.Provider = "a-provider" + tc.blank(&profile) + + err := profile.validate() + if err == nil { + t.Fatalf("validate() accepted a profile with no %s", tc.field) + } + if !strings.Contains(err.Error(), tc.field) { + t.Errorf("error %q does not name the missing %s", err, tc.field) + } + if !strings.Contains(err.Error(), "a-provider") { + t.Errorf("error %q does not name the provider, which an operator "+ + "needs to know which block to edit", err) + } + }) + } +} + +// tokenTtl is required, and it has a floor. At or below the refresh skew every +// token is already expired when it arrives, so each call would exchange one and +// throw it away -- two round trips per request, forever, with nothing failing +// loudly enough to notice. +func TestTokenQueryRefusesAnUnusableTTL(t *testing.T) { + for _, tc := range []struct{ name, ttl, wantIn string }{ + {"absent", "", "requires tokenTtl"}, + {"not a duration", "ten minutes", "is not a duration"}, + {"at the skew", util.TokenRefreshSkew.String(), "must exceed"}, + {"below the skew", "1s", "must exceed"}, + } { + t.Run(tc.name, func(t *testing.T) { + profile := tokenQueryProfile("https://issuer.example/token") + profile.Provider = "a-provider" + profile.TokenTTLRaw = tc.ttl + + err := profile.validate() + if err == nil { + t.Fatalf("validate() accepted tokenTtl %q", tc.ttl) + } + if !strings.Contains(err.Error(), tc.wantIn) { + t.Errorf("error %q does not contain %q", err, tc.wantIn) + } + }) + } +} + +// A ttl above the skew is accepted, and the held lifetime is shortened by it -- +// so a request that passed the expiry check cannot arrive after the token died. +func TestTokenQuerySubtractsTheSkewFromTheConfiguredTTL(t *testing.T) { + profile := tokenQueryProfile("https://issuer.example/token") + profile.TokenTTLRaw = "10m" + if err := profile.validate(); err != nil { + t.Fatalf("validate() = %v, want a 10m ttl accepted", err) + } + if profile.TokenTTL != 10*time.Minute { + t.Errorf("TokenTTL = %s, want 10m parsed from config", profile.TokenTTL) + } + // The subtraction itself happens in the exchange; this pins the arithmetic + // it depends on, so a change to the skew cannot silently invert it. + if profile.TokenTTL-util.TokenRefreshSkew <= 0 { + t.Errorf("held lifetime %s is not positive", profile.TokenTTL-util.TokenRefreshSkew) + } +} + +// A 2xx that does not carry the configured field is the endpoint misbehaving in +// a way another attempt will not change, and the message names the field so an +// operator can tell a misconfigured tokenResponseField from a broken provider. +func TestTokenQueryRefusesAResponseWithoutTheConfiguredField(t *testing.T) { + t.Setenv("TEST_TQ_USER", "user") + t.Setenv("TEST_TQ_SECRET", "secret") + + // The endpoint answers 200 with a different key than the one configured. + ts := &queryTokenServer{body: `{"access_token":"wrong-shape"}`} + var seen []string + step := tokenQueryStep(t, ts.start(t), &seen, nil) + + _, err := runStep(t, step, selectBody) + if err == nil { + t.Fatal("Run() succeeded on a response with no token field, want an error") + } + if !strings.Contains(err.Error(), "token") { + t.Errorf("error %q does not name the missing field", err) + } + if len(seen) != 0 { + t.Errorf("provider was called %d times despite no token, want 0", len(seen)) + } +} + +// The secret and the token must never reach a log or a returned error. The +// token matters more here than under oauth2: it travels in a query string, +// which transport errors quote whole. +func TestTokenQueryRedactsTheSecretAndTheToken(t *testing.T) { + t.Setenv("TEST_TQ_USER", "an-identifier") + t.Setenv("TEST_TQ_SECRET", "the-secret-value") + + ts := &queryTokenServer{token: "tok+with/escapes="} + var seen []string + step := tokenQueryStep(t, ts.start(t), &seen, nil) + + if _, err := runStep(t, step, selectBody); err != nil { + t.Fatalf("Run() returned an unexpected error: %v", err) + } + + // secretForms is what redaction substitutes; assert on it directly, since a + // passing request produces no error text to inspect. + var forms []string + for _, auth := range step.auth { + forms = append(forms, auth.secretForms()...) + } + joined := strings.Join(forms, "|") + + for _, want := range []string{"the-secret-value", "tok+with/escapes="} { + if !strings.Contains(joined, want) { + t.Errorf("secretForms %q does not cover %q", forms, want) + } + } + // The escaped form too, or a token with "+" survives redaction in a URL. + if !strings.Contains(joined, "tok%2Bwith%2Fescapes%3D") { + t.Errorf("secretForms %q does not cover the query-escaped token", forms) + } + // The identifier is not a credential and is left readable, so a log still + // says who this adapter authenticated as. + if strings.Contains(joined, "an-identifier") { + t.Errorf("secretForms %q redacts the identifier, which is not a secret", forms) + } +} + +// An unset credential is a configuration fault: it fails permanently rather +// than being retried, and the message names the variables without printing +// them. +func TestTokenQueryWithNoCredentialFailsWithoutRetrying(t *testing.T) { + t.Setenv("TEST_TQ_USER", "") + t.Setenv("TEST_TQ_SECRET", "") + + ts := &queryTokenServer{} + var seen []string + step := tokenQueryStep(t, ts.start(t), &seen, nil) + + _, err := runStep(t, step, selectBody) + if err == nil { + t.Fatal("Run() succeeded with no credential set, want an error") + } + if got := ts.calls.Load(); got != 0 { + t.Errorf("token endpoint called %d times with no credential, want 0", got) + } + if !util.IsPermanent(err) { + t.Errorf("error %v is retryable; an unset credential will not fix itself", err) + } +} diff --git a/pkg/plugin/implementation/internal/common/util/constant.go b/pkg/plugin/implementation/internal/common/util/constant.go index 56fa7e8b..df4cb1d1 100644 --- a/pkg/plugin/implementation/internal/common/util/constant.go +++ b/pkg/plugin/implementation/internal/common/util/constant.go @@ -63,13 +63,33 @@ const ( // Only client_credentials. That grant has no user to redirect and no // refresh token, so the other flows would be dead code. AuthSchemeOAuth2 = "oauth2" + + // Credentials are POSTed as JSON and the token that comes back goes in the + // QUERY STRING. For an upstream that issues short-lived tokens from its own + // endpoint rather than an OAuth2 one -- neither the request shape nor the + // placement matches oauth2, so it cannot be served by bending that scheme: + // + // oauth2 form-encoded client_id/client_secret -> access_token, + // sent as Authorization: Bearer + // tokenQuery JSON body of CONFIGURED field names -> a CONFIGURED field, + // sent as a query parameter + // + // The field names are configured rather than fixed because "access_name" + // and "password" are one provider's spelling, not a standard. + // + // Inherits query's exposure -- the token reaches proxy logs and transport + // errors -- so it is redacted the same way. See redact. + AuthSchemeTokenQuery = "tokenQuery" ) // CodeUpstreamUnavailable: the provider could not be reached or failed. Not // this adapter's fault and not the caller's. const CodeUpstreamUnavailable = "NET_DOWNSTREAM_UNAVAILABLE" -// How early an oauth2 token stops being trusted. Must exceed the round trip to -// the provider, so a request that passed the expiry check cannot arrive after -// the token died. Costs one extra exchange per lifetime. +// How early an exchanged token stops being trusted. Must exceed the round trip +// to the provider, so a request that passed the expiry check cannot arrive +// after the token died. Costs one extra exchange per lifetime. +// +// Applied to oauth2's expires_in and to tokenQuery's configured tokenTtl +// alike: both are a claim about a lifetime, and the same race sits under both. const TokenRefreshSkew = 60 * time.Second