From 501126f07694a03215bb1e486152ce77850eb3de Mon Sep 17 00:00:00 2001 From: kkinformed <312492264+kkinformed@users.noreply.github.com> Date: Thu, 15 Oct 2026 12:52:01 +0200 Subject: [PATCH] feat(dpav-3324): add subscription status to product discover and view Signed-off-by: kkinformed <312492264+kkinformed@users.noreply.github.com> --- docs/tests/products/product-discover.md | 77 ++++++++- docs/tests/products/product-view.md | 6 +- .../controller/v1/ProductController.java | 16 +- .../dto/product/DiscoveredProductDTO.java | 14 +- .../ProductDiscoveryRepository.java | 18 +++ .../service/data/ProductDiscoveryService.java | 14 ++ .../service/data/ProductViewService.java | 15 +- .../impl/ProductDiscoveryServiceImpl.java | 6 +- .../data/impl/ProductViewServiceImpl.java | 7 +- .../discovery/DiscoveredProductAssembler.java | 42 +++++ .../controller/v1/ProductControllerTest.java | 67 ++++++-- ...eryRepositorySubscriptionPostgresTest.java | 153 ++++++++++++++++++ ...ctDiscoveryRepositorySubscriptionTest.java | 76 +++++++++ .../impl/ProductDiscoveryServiceImplTest.java | 6 +- .../data/impl/ProductViewServiceImplTest.java | 8 +- .../DiscoveredProductAssemblerTest.java | 43 +++++ 16 files changed, 531 insertions(+), 37 deletions(-) create mode 100644 src/test/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepositorySubscriptionPostgresTest.java create mode 100644 src/test/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepositorySubscriptionTest.java diff --git a/docs/tests/products/product-discover.md b/docs/tests/products/product-discover.md index d6202380..509aceca 100644 --- a/docs/tests/products/product-discover.md +++ b/docs/tests/products/product-discover.md @@ -57,6 +57,7 @@ Related documents: - [G. Refusals by policy, 403 (TC-32 – TC-39)](#g-refusals-by-policy--403) - [H. Malformed requests, 400 (TC-40 – TC-42)](#h-malformed-requests--400) - [I. Policy switched off (TC-43 – TC-44)](#i-policy-switched-off) + - [J. Subscription status (TC-45 – TC-46)](#j-subscription-status) - [9. Edge cases](#9-edge-cases) - [10. Defect triage](#10-defect-triage) @@ -198,7 +199,8 @@ text-searchable, so a single value defaults to `eq` and must match the whole nam "name": "FloodRiskMapZones", "description": "Flood risk zones for England and Wales as geographic areas, …", "type": "topic", - "organisation": { "key": "ENV", "name": "Environment Agency (ENV)" } + "organisation": { "key": "ENV", "name": "Environment Agency (ENV)" }, + "subscriptionStatus": "NOT_SUBSCRIBED" } ], "page": { "number": 0, "size": 20, "numberOfElements": 1, "totalElements": 1, "totalPages": 1 }, @@ -216,10 +218,8 @@ text-searchable, so a single value defaults to `eq` and must match the whole nam ### A result is a summary -A discovery result carries **only**: `id`, `name`, `description`, `type`, and an `organisation` -carrying `key` and `name`. Nothing else: no `topic`, no `source`, no `producer`, no `consumers`, -no `subscribedBy`, no attribute maps. Those belong to -[`GET /api/v1/product/{productId}`](product-view.md). +A discovery result carries **only**: `id`, `name`, `description`, `type`, an `organisation` +carrying `key` and `name`, and `subscriptionStatus`. **Filtering and sorting are not limited to those members.** A caller may filter and sort on anything in its allowed lists, including `topic` and `source`, which no result carries. This @@ -228,6 +228,17 @@ separation is deliberate and has its own test case ([TC-09](#tc-09--filter-on-a- Absent members are **omitted**, never `null` and never `""`. A product with no description simply has no `description` key. +### `subscriptionStatus` + +Whether the **calling** organisation holds a grant on the product, through any of its consumers: +`SUBSCRIBED` or `NOT_SUBSCRIBED`. The organisation is the token's `organisation` claim. + +- It describes the caller only, so it is **not masked**: BCC, for which `subscribedBy` is masked, + still sees its own status. +- A grant counts whatever its `validity`, exactly as for `subscribedBy`. +- An organisation holding the product on several consumers is still one `SUBSCRIBED`. +- Absent when the token carries no organisation. + ### The `page` object | Field | Means | @@ -478,14 +489,15 @@ absent because of masking. **Given** ENV, which has no masked fields at all **When** the request is posted -**Then** the single product carries exactly `id`, `name`, `description`, `type`, `organisation` +**Then** the single product carries exactly `id`, `name`, `description`, `type`, `organisation`, +`subscriptionStatus` **And** `organisation` carries exactly `key` and `name` **And** `topic`, `source`, `producer`, `consumers`, `subscribedBy`, `attributes` are **absent keys** They are not present with a `null` value. ```bash discover "$ENV_TOKEN" '{"text":"FloodRiskMapZones"}' | jq '.products[0] | keys, (.organisation | keys)' -# ["description","id","name","organisation","type"] +# ["description","id","name","organisation","subscriptionStatus","type"] # ["key","name"] ``` @@ -1632,6 +1644,57 @@ answered.) --- +### J. Subscription status + +#### TC-45: Each product reports whether the caller is subscribed + +**Objective.** Confirm `subscriptionStatus` follows the caller's own grants. + +**Preconditions.** P1–P4. + +**Test data.** Empty body, each token. Grants as in [6.2](#62-grants-for-subscribedby). + +**Given** the three callers +**When** each posts `{}` +**Then** exactly these products are `SUBSCRIBED`, and every other product is `NOT_SUBSCRIBED`: + +| Caller | `SUBSCRIBED` | +|---|---| +| ENV | AffordableHousingCompletions, BristolAirQualitySensors, BrownfieldLandAvailability, HelpToBuyApplicants | +| HEG | BristolAirQualitySensors, RiverLevelTelemetry | +| BCC | AffordableHousingCompletions, FloodRiskMapZones, RiverLevelTelemetry | + +```bash +for t in "$ENV_TOKEN" "$HEG_TOKEN" "$BCC_TOKEN"; do + discover "$t" '{}' | jq -c '[.products[] | select(.subscriptionStatus=="SUBSCRIBED") | .name] | sort' +done +``` + +*Why:* a grant on a product the caller cannot discover is not reported. BCC holds +PollutionIncidentReports but cannot discover it, so it does not appear. + +#### TC-46: The status is the caller's own, not anyone else's + +**Objective.** Confirm the same product reports differently per caller. + +**Preconditions.** P1–P4. + +**Given** RiverLevelTelemetry, granted to HEG and twice to BCC, and not to ENV +**When** each caller discovers `{"text": "RiverLevelTelemetry"}` +**Then** ENV sees `NOT_SUBSCRIBED` +**And** HEG and BCC see `SUBSCRIBED`, BCC once despite its two grants. + +```bash +for t in "$ENV_TOKEN" "$HEG_TOKEN" "$BCC_TOKEN"; do + discover "$t" '{"text":"RiverLevelTelemetry"}' | jq -r '.products[0].subscriptionStatus' +done +# NOT_SUBSCRIBED +# SUBSCRIBED +# SUBSCRIBED +``` + +--- + ## 9. Edge cases Each is cheap to run and each has caught, or is designed to catch, a real class of defect. diff --git a/docs/tests/products/product-view.md b/docs/tests/products/product-view.md index eac75bb8..05fa17ec 100644 --- a/docs/tests/products/product-view.md +++ b/docs/tests/products/product-view.md @@ -160,7 +160,8 @@ the PDP at all. ], "subscribedBy": [ { "key": "HEG", "name": "Homes England", "consumers": 1, "since": "2026-02-11T07:00:00" } - ] + ], + "subscriptionStatus": "NOT_SUBSCRIBED" } ``` @@ -178,6 +179,9 @@ Rules that hold for every `200`: - A nested `consumers[].organisation` carries `key` and `name` only; it never carries `attributes`. - Attribute maps keep their JSON types: a multi-valued attribute is an array, a number stays a number. +- **`subscriptionStatus`** is the calling organisation's own status, `SUBSCRIBED` or + `NOT_SUBSCRIBED`, with the same value discovery reports. It is not masked, since it describes the + caller only. See [discover §4](product-discover.md#subscriptionstatus). - The response carries **no `policy` block**. Discovery's response has one (it is how a UI builds a filter panel); view returns the bare product. diff --git a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/controller/v1/ProductController.java b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/controller/v1/ProductController.java index d8c496ba..4ba8ccb5 100644 --- a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/controller/v1/ProductController.java +++ b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/controller/v1/ProductController.java @@ -69,7 +69,8 @@ public ProductController( description = "Searches data products by free text, filters on product fields and policy " + "attributes, sorting and paging. Each result is a summary - id, name, description, type " + "and the owning organisation's key and name - enough to recognise a product, see " - + "whose it is, and ask for it by id; the rest of a " + + "whose it is, and ask for it by id - plus subscriptionStatus, whether the calling " + + "organisation holds a grant on it (SUBSCRIBED or NOT_SUBSCRIBED); the rest of a " + "product is returned by GET /api/v1/product/{productId}. Filtering and sorting are not " + "limited to those four: a search may filter and sort on anything policy allows it, " + "including fields the summary does not carry. The product.discover policy decides " @@ -91,6 +92,7 @@ public ProductController( @ApiResponse(responseCode = "500", description = "Internal server error") public ProductDiscoveryResponseDTO discoverProducts( @Valid @RequestBody(required = false) ProductDiscoveryRequestDTO criteria, + @Parameter(hidden = true) @AuthenticationPrincipal EnhancedPrincipal principal, @Parameter(hidden = true) Optional> policyDecision) { // The decision taken for this request is the only one there is: it carries the caller's // search contract, which the service turns into the query. It is passed on rather than @@ -102,7 +104,7 @@ public ProductDiscoveryResponseDTO discoverProducts( decision.policy().resolution(), decision.reasons(), decision.details().evaluation())); - return productDiscoveryService.discover(criteria, policyDecision); + return productDiscoveryService.discover(criteria, organisationOf(principal), policyDecision); } @PostMapping("/subscribe") @@ -140,8 +142,7 @@ public ProductSubscriptionResponseDTO subscribe( Optional> policyDecision) { // The decision is handed to the service rather than re-taken there: the terms applied must // be the ones this request was allowed on. - return productSubscriptionService.subscribe( - subscription, principal == null ? null : principal.organisation(), policyDecision); + return productSubscriptionService.subscribe(subscription, organisationOf(principal), policyDecision); } @GetMapping("/{productId}") @@ -168,6 +169,7 @@ public ProductSubscriptionResponseDTO subscribe( @ApiResponse(responseCode = "500", description = "Internal server error") public ResponseEntity viewProduct( @Parameter(description = "Product identifier") @PathVariable("productId") Long productId, + @Parameter(hidden = true) @AuthenticationPrincipal EnhancedPrincipal principal, @Parameter(hidden = true) Optional> policyDecision) { policyDecision.ifPresent(decision -> log.debug( "Product view policy decision allow={} policy={} resolution={} reasons={} accessLevel={}", @@ -177,8 +179,12 @@ public ResponseEntity viewProduct( decision.reasons(), decision.details().accessLevel())); return productViewService - .view(productId, policyDecision) + .view(productId, organisationOf(principal), policyDecision) .map(ResponseEntity::ok) .orElseGet(() -> ResponseEntity.notFound().build()); } + + private static String organisationOf(EnhancedPrincipal principal) { + return principal == null ? null : principal.organisation(); + } } diff --git a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/model/dto/product/DiscoveredProductDTO.java b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/model/dto/product/DiscoveredProductDTO.java index c3cf735f..07568810 100644 --- a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/model/dto/product/DiscoveredProductDTO.java +++ b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/model/dto/product/DiscoveredProductDTO.java @@ -41,6 +41,9 @@ * @param producer the producer offering the product * @param consumers the consumers holding a grant, with the terms of each grant * @param subscribedBy the organisations using the product: those with at least one grant + * @param subscriptionStatus whether the calling organisation holds a grant on the product. Carried by + * both endpoints and not subject to masking: it describes the caller, not other organisations. + * Absent when the caller's organisation is not known */ @Builder @JsonInclude(JsonInclude.Include.NON_NULL) @@ -55,7 +58,16 @@ public record DiscoveredProductDTO( Organisation organisation, Producer producer, List consumers, - List subscribedBy) { + List subscribedBy, + SubscriptionStatus subscriptionStatus) { + + /** The calling organisation's relation to a product. */ + public enum SubscriptionStatus { + /** At least one of the organisation's consumers holds a grant on the product. */ + SUBSCRIBED, + /** None of the organisation's consumers holds a grant on the product. */ + NOT_SUBSCRIBED + } @Builder @JsonInclude(JsonInclude.Include.NON_NULL) diff --git a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepository.java b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepository.java index 0d40ced2..3a5739d1 100644 --- a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepository.java +++ b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepository.java @@ -68,6 +68,7 @@ public class ProductDiscoveryRepository { private final String grantsSql; private final String subscribingOrganisationsSql; + private final String subscribedProductIdsSql; public ProductDiscoveryRepository(NamedParameterJdbcTemplate jdbc, DiscoverySchema schema) { this.jdbc = jdbc; @@ -95,6 +96,11 @@ public ProductDiscoveryRepository(NamedParameterJdbcTemplate jdbc, DiscoverySche + " WHERE pc.product_id IN (:ids)" + " GROUP BY pc.product_id, o.organisation_key, o.name" + " ORDER BY pc.product_id, o.organisation_key"; + this.subscribedProductIdsSql = "SELECT DISTINCT pc.product_id" + + " FROM " + this.schema + "product_consumer pc" + + " JOIN " + this.schema + "consumer c ON c.id = pc.consumer_id" + + " JOIN " + this.schema + "organisation o ON o.id = c.org_id" + + " WHERE pc.product_id IN (:ids) AND o.organisation_key = :organisationKey"; } /** One row per product of the page, keyed by the column aliases of {@link ProductSearchQuery}. */ @@ -194,6 +200,18 @@ public List findSubscribingOrganisations(Collection findSubscribedProductIds(String organisationKey, Collection productIds) { + if (organisationKey == null || productIds.isEmpty()) { + return Set.of(); + } + return Set.copyOf(jdbc.queryForList( + subscribedProductIdsSql, Map.of("ids", productIds, "organisationKey", organisationKey), Long.class)); + } + /** One grant ({@code product_consumer} row) with its consumer and the consumer's organisation. */ public record Grant( long grantId, diff --git a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/ProductDiscoveryService.java b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/ProductDiscoveryService.java index 360ebe5d..d078f041 100644 --- a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/ProductDiscoveryService.java +++ b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/ProductDiscoveryService.java @@ -38,7 +38,21 @@ public interface ProductDiscoveryService { * @throws uk.gov.dbt.ndtp.ia.node.management.exception.AccessRejectedException when the criteria * ask for something policy does not permit, or the contract cannot be honoured ({@code 403}) */ + default ProductDiscoveryResponseDTO discover( + ProductDiscoveryRequestDTO criteria, + Optional> decision) { + return discover(criteria, null, decision); + } + + /** + * As {@link #discover(ProductDiscoveryRequestDTO, Optional)}, reporting the calling + * organisation's subscription status on every product. + * + * @param callerOrganisation the calling organisation's key, from the token; null leaves the + * status out + */ ProductDiscoveryResponseDTO discover( ProductDiscoveryRequestDTO criteria, + String callerOrganisation, Optional> decision); } diff --git a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/ProductViewService.java b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/ProductViewService.java index ca770f8c..cde2c5d7 100644 --- a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/ProductViewService.java +++ b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/ProductViewService.java @@ -33,6 +33,19 @@ public interface ProductViewService { * @throws uk.gov.dbt.ndtp.ia.node.management.exception.AccessRejectedException when the contract * cannot be honoured ({@code 403}) */ + default Optional view( + Long productId, Optional> decision) { + return view(productId, null, decision); + } + + /** + * As {@link #view(Long, Optional)}, reporting the calling organisation's subscription status. + * + * @param callerOrganisation the calling organisation's key, from the token; null leaves the + * status out + */ Optional view( - Long productId, Optional> decision); + Long productId, + String callerOrganisation, + Optional> decision); } diff --git a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductDiscoveryServiceImpl.java b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductDiscoveryServiceImpl.java index 93ee3ea8..744d6921 100644 --- a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductDiscoveryServiceImpl.java +++ b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductDiscoveryServiceImpl.java @@ -83,6 +83,7 @@ public ProductDiscoveryServiceImpl( @Override public ProductDiscoveryResponseDTO discover( ProductDiscoveryRequestDTO request, + String callerOrganisation, Optional> decision) { ProductSearchContract contract = planner.contractFor(WHAT, decision); @@ -91,8 +92,9 @@ public ProductDiscoveryResponseDTO discover( ProductSearchQuery query = planner.compile(WHAT, contract, criteria, PROJECTION, decision); long total = repository.count(query); - List products = - total == 0 ? List.of() : assembler.assemble(repository.findPage(query), contract, PROJECTION); + List products = total == 0 + ? List.of() + : assembler.assemble(repository.findPage(query), contract, PROJECTION, callerOrganisation); audit(contract, criteria, total, decision); return response(contract, criteria, products, total); diff --git a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductViewServiceImpl.java b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductViewServiceImpl.java index 16a9a4a2..6263ae17 100644 --- a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductViewServiceImpl.java +++ b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductViewServiceImpl.java @@ -65,7 +65,9 @@ public ProductViewServiceImpl( @Override public Optional view( - Long productId, Optional> decision) { + Long productId, + String callerOrganisation, + Optional> decision) { ProductSearchContract contract = planner.contractFor(WHAT, decision); ProductSearchQuery query = planner.compile(WHAT, contract, criteriaFor(productId), PROJECTION, decision); @@ -76,7 +78,8 @@ public Optional view( // was found. Optional product = rows.isEmpty() ? Optional.empty() - : assembler.assemble(rows, contract, PROJECTION).stream().findFirst(); + : assembler.assemble(rows, contract, PROJECTION, callerOrganisation).stream() + .findFirst(); audit(productId, contract, product, decision); return product; diff --git a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/discovery/DiscoveredProductAssembler.java b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/discovery/DiscoveredProductAssembler.java index a313e9b4..3dc50d54 100644 --- a/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/discovery/DiscoveredProductAssembler.java +++ b/src/main/java/uk/gov/dbt/ndtp/ia/node/management/service/discovery/DiscoveredProductAssembler.java @@ -45,6 +45,20 @@ public DiscoveredProductAssembler(ProductDiscoveryRepository repository) { */ public List assemble( List> rows, ProductSearchContract contract, ProductProjection projection) { + return assemble(rows, contract, projection, null); + } + + /** + * As {@link #assemble(List, ProductSearchContract, ProductProjection)}, adding the calling + * organisation's subscription status to every product. + * + * @param callerOrganisation the calling organisation's key; null leaves the status out + */ + public List assemble( + List> rows, + ProductSearchContract contract, + ProductProjection projection, + String callerOrganisation) { List page = rows.stream().map(row -> new PageRow(row, contract, projection)).toList(); @@ -77,6 +91,7 @@ public List assemble( attributes(PolicyAttributeScopeCode.SUBSCRIPTION, ids(grants, Grant::grantId), contract, projection); List subscribing = subscribingOrganisations(idsShowing(page, ProductBlock.SUBSCRIBED_BY, PageRow::productId)); + Set subscribed = subscribedProductIds(callerOrganisation, page); return page.stream() .map(row -> DiscoveredProductDTO.builder() @@ -117,6 +132,7 @@ public List assemble( row.shows(ProductBlock.SUBSCRIBED_BY) ? subscribedBy(row.productId(), subscribing) : null) + .subscriptionStatus(subscriptionStatus(callerOrganisation, subscribed, row.productId())) .build()) .toList(); } @@ -190,6 +206,32 @@ private List grants(Set productIds) { return productIds.isEmpty() ? List.of() : repository.findGrants(productIds); } + /** + * The products of the page the calling organisation holds a grant on - and nothing asked of the + * repository when the caller's organisation is not known. The caller's own status describes the + * caller, not other organisations, so it is not subject to the masking that withholds consumers + * and subscribedBy. + */ + private Set subscribedProductIds(String callerOrganisation, List page) { + if (callerOrganisation == null) { + return Set.of(); + } + return repository.findSubscribedProductIds( + callerOrganisation, page.stream().map(PageRow::productId).toList()); + } + + /** The caller's status for one product; null, so absent, when the caller's organisation is not known. */ + private static DiscoveredProductDTO.SubscriptionStatus subscriptionStatus( + String callerOrganisation, Set subscribed, long productId) { + if (callerOrganisation == null) { + return null; + } + if (subscribed.contains(productId)) { + return DiscoveredProductDTO.SubscriptionStatus.SUBSCRIBED; + } + return DiscoveredProductDTO.SubscriptionStatus.NOT_SUBSCRIBED; + } + /** The organisations subscribing to the products that show them; see {@link #grants}. */ private List subscribingOrganisations(Set productIds) { return productIds.isEmpty() ? List.of() : repository.findSubscribingOrganisations(productIds); diff --git a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/controller/v1/ProductControllerTest.java b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/controller/v1/ProductControllerTest.java index 00f10307..728b222a 100644 --- a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/controller/v1/ProductControllerTest.java +++ b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/controller/v1/ProductControllerTest.java @@ -9,6 +9,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.containsString; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @@ -133,7 +134,7 @@ private void authenticateAs(String clientId) { /** What the service answers unless a test says otherwise. */ private void serviceReturns(ProductDiscoveryResponseDTO response) { - when(productDiscoveryService.discover(any(), any())).thenReturn(response); + when(productDiscoveryService.discover(any(), any(), any())).thenReturn(response); } private static ProductDiscoveryResponseDTO oneProduct() { @@ -146,6 +147,36 @@ private static ProductDiscoveryResponseDTO oneProduct() { .build(); } + @Test + void discover_callerOrganisation_comesFromTheTokenAndReachesTheService() throws Exception { + serviceReturns(oneProduct()); + + mockMvc.perform(post("/api/v1/product/discover") + .contentType(MediaType.APPLICATION_JSON) + .content("{}")) + .andExpect(status().isOk()); + + verify(productDiscoveryService).discover(any(), eq("test-organisation"), any()); + } + + @Test + void discover_subscriptionStatus_isSerialisedOnEachProduct() throws Exception { + serviceReturns(ProductDiscoveryResponseDTO.builder() + .products(List.of(DiscoveredProductDTO.builder() + .id(3L) + .name("FloodRiskMapZones") + .subscriptionStatus(DiscoveredProductDTO.SubscriptionStatus.SUBSCRIBED) + .build())) + .page(ProductDiscoveryResponseDTO.Page.of(0, 20, 1, 1)) + .build()); + + mockMvc.perform(post("/api/v1/product/discover") + .contentType(MediaType.APPLICATION_JSON) + .content("{}")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.products[0].subscriptionStatus").value("SUBSCRIBED")); + } + @Test void discover_criteria_areBoundAndPassedToTheService() throws Exception { serviceReturns(oneProduct()); @@ -164,7 +195,7 @@ void discover_criteria_areBoundAndPassedToTheService() throws Exception { .andExpect(status().isOk()); ArgumentCaptor criteria = ArgumentCaptor.forClass(ProductDiscoveryRequestDTO.class); - verify(productDiscoveryService).discover(criteria.capture(), any()); + verify(productDiscoveryService).discover(criteria.capture(), any(), any()); ProductDiscoveryRequestDTO sent = criteria.getValue(); assertThat(sent.text()).isEqualTo("flood"); assertThat(sent.page()).isEqualTo(2); @@ -206,7 +237,7 @@ void discover_emptyBody_reachesTheServiceAsNoCriteria() throws Exception { .andExpect(status().isOk()) .andExpect(jsonPath("$.products").isEmpty()); - verify(productDiscoveryService).discover(isNull(), any()); + verify(productDiscoveryService).discover(isNull(), any(), any()); } /** An unknown top-level field is dropped at binding rather than rejected, as before. */ @@ -280,7 +311,7 @@ void discover_malformedJsonBody_returns400() throws Exception { /** Criteria the service cannot act on are the caller's to correct, so they are a 400. */ @Test void discover_invalidCriteria_returns400WithTheReason() throws Exception { - when(productDiscoveryService.discover(any(), any())) + when(productDiscoveryService.discover(any(), any(), any())) .thenThrow(new InvalidSearchCriteriaException("A filter names exactly one of 'field' and 'attribute'")); mockMvc.perform(post("/api/v1/product/discover") @@ -293,7 +324,7 @@ void discover_invalidCriteria_returns400WithTheReason() throws Exception { /** Criteria policy does not permit are a 403 naming what was refused, not a 400. */ @Test void discover_criteriaRefusedByPolicy_returns403WithNamedReasons() throws Exception { - when(productDiscoveryService.discover(any(), any())) + when(productDiscoveryService.discover(any(), any(), any())) .thenThrow(new AccessRejectedException( "Access denied by policy", List.of("filter.attribute_not_permitted:identifiability"), @@ -325,7 +356,7 @@ private Optional> decision ArgumentCaptor>> decision = ArgumentCaptor.forClass(Optional.class); - verify(productDiscoveryService).discover(any(), decision.capture()); + verify(productDiscoveryService).discover(any(), any(), decision.capture()); return decision.getValue(); } @@ -525,7 +556,7 @@ private static PolicyDecision viewDecision() { /** What the view service answers; a null product stands for "no such product for this caller". */ private void viewReturns(DiscoveredProductDTO product) { - when(productViewService.view(any(), any())).thenReturn(Optional.ofNullable(product)); + when(productViewService.view(any(), any(), any())).thenReturn(Optional.ofNullable(product)); } @Test @@ -565,10 +596,24 @@ void view_noProductForThisCaller_returns404WhetherItIsAbsentOrWithheld() throws .andExpect(status().isNotFound()); ArgumentCaptor productId = ArgumentCaptor.forClass(Long.class); - verify(productViewService).view(productId.capture(), any()); + verify(productViewService).view(productId.capture(), any(), any()); assertThat(productId.getValue()).isEqualTo(99L); } + @Test + void view_callerOrganisation_comesFromTheTokenAndReachesTheService() throws Exception { + viewReturns(DiscoveredProductDTO.builder() + .id(7L) + .subscriptionStatus(DiscoveredProductDTO.SubscriptionStatus.NOT_SUBSCRIBED) + .build()); + + mockMvc.perform(get("/api/v1/product/7").requestAttr(PolicyDecision.REQUEST_ATTRIBUTE, viewDecision())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.subscriptionStatus").value("NOT_SUBSCRIBED")); + + verify(productViewService).view(eq(7L), eq("test-organisation"), any()); + } + @Test void view_pathVariable_reachesTheServiceAsALong() throws Exception { viewReturns(DiscoveredProductDTO.builder().id(7L).build()); @@ -577,7 +622,7 @@ void view_pathVariable_reachesTheServiceAsALong() throws Exception { .andExpect(status().isOk()); ArgumentCaptor productId = ArgumentCaptor.forClass(Long.class); - verify(productViewService).view(productId.capture(), any()); + verify(productViewService).view(productId.capture(), any(), any()); assertThat(productId.getValue()).isEqualTo(7L); } @@ -603,7 +648,7 @@ private Optional> viewDecisionH ArgumentCaptor>> decision = ArgumentCaptor.forClass(Optional.class); - verify(productViewService).view(any(), decision.capture()); + verify(productViewService).view(any(), any(), decision.capture()); return decision.getValue(); } @@ -628,7 +673,7 @@ void view_withPolicySwitchedOff_handsTheServiceNoDecision() throws Exception { /** A contract the service cannot honour is a 403 naming why, exactly as on discovery. */ @Test void view_refusedByTheService_returns403WithItsReasons() throws Exception { - when(productViewService.view(any(), any())) + when(productViewService.view(any(), any(), any())) .thenThrow(new AccessRejectedException( "Access denied by policy", List.of("policy.obligation_unsupported"), "error-2")); diff --git a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepositorySubscriptionPostgresTest.java b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepositorySubscriptionPostgresTest.java new file mode 100644 index 00000000..7a87653f --- /dev/null +++ b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepositorySubscriptionPostgresTest.java @@ -0,0 +1,153 @@ +package uk.gov.dbt.ndtp.ia.node.management.persistency.repository.configuration; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.math.BigDecimal; +import java.sql.Timestamp; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import uk.gov.dbt.ndtp.ia.node.management.persistency.entity.configuration.Consumer; +import uk.gov.dbt.ndtp.ia.node.management.persistency.entity.configuration.Producer; +import uk.gov.dbt.ndtp.ia.node.management.persistency.entity.configuration.Product; +import uk.gov.dbt.ndtp.ia.node.management.persistency.entity.configuration.ProductConsumer; +import uk.gov.dbt.ndtp.ia.node.management.persistency.entity.organisation.Organisation; +import uk.gov.dbt.ndtp.ia.node.management.persistency.repository.AbstractPostgresRepositoryTest; +import uk.gov.dbt.ndtp.ia.node.management.persistency.repository.organisation.OrganisationRepository; +import uk.gov.dbt.ndtp.ia.node.management.service.discovery.DiscoverySchema; + +class ProductDiscoveryRepositorySubscriptionPostgresTest extends AbstractPostgresRepositoryTest { + + @Autowired + private NamedParameterJdbcTemplate jdbc; + + @Autowired + private OrganisationRepository organisationRepository; + + @Autowired + private ProducerRepository producerRepository; + + @Autowired + private ConsumerRepository consumerRepository; + + @Autowired + private ProductRepository productRepository; + + @Autowired + private ProductConsumerRepository productConsumerRepository; + + private ProductDiscoveryRepository repository; + private Organisation subscriber; + private Organisation other; + private Product onTwoConsumers; + private Product expired; + private Product othersOnly; + private Product nobodys; + + @BeforeEach + void setUp() { + repository = new ProductDiscoveryRepository(jdbc, new DiscoverySchema("")); + + Organisation owner = organisation(); + Producer producer = producer(owner); + onTwoConsumers = product(producer); + expired = product(producer); + othersOnly = product(producer); + nobodys = product(producer); + + subscriber = organisation(); + Consumer first = consumer(subscriber); + Consumer second = consumer(subscriber); + grant(onTwoConsumers, first, Instant.now(), 30); + grant(onTwoConsumers, second, Instant.now(), 30); + grant(expired, first, Instant.now().minus(400, ChronoUnit.DAYS), 30); + + other = organisation(); + grant(othersOnly, consumer(other), Instant.now(), 30); + } + + @Test + void reportsEveryProductTheOrganisationHoldsAGrantOn_onceEach_whateverItsValidity() { + assertThat(repository.findSubscribedProductIds(subscriber.getOrganisationKey(), allProducts())) + .containsExactlyInAnyOrder(onTwoConsumers.getId(), expired.getId()); + } + + @Test + void doesNotReportAnotherOrganisationsGrants() { + assertThat(repository.findSubscribedProductIds(other.getOrganisationKey(), allProducts())) + .containsExactly(othersOnly.getId()); + } + + @Test + void isLimitedToTheProductsAskedAbout() { + assertThat(repository.findSubscribedProductIds( + subscriber.getOrganisationKey(), List.of(othersOnly.getId(), nobodys.getId()))) + .isEmpty(); + } + + @Test + void unknownOrganisation_holdsNothing() { + assertThat(repository.findSubscribedProductIds("NO_SUCH_ORG", allProducts())) + .isEmpty(); + } + + private List allProducts() { + return List.of(onTwoConsumers.getId(), expired.getId(), othersOnly.getId(), nobodys.getId()); + } + + private Organisation organisation() { + Organisation organisation = new Organisation(); + organisation.setName("Subscription Test Org"); + organisation.setOrganisationKey("SUB_" + unique()); + return organisationRepository.saveAndFlush(organisation); + } + + private Producer producer(Organisation organisation) { + Producer producer = new Producer(); + producer.setName("Producer " + unique()); + producer.setDescription("Subscription test producer"); + producer.setOrg(organisation); + producer.setActive(true); + producer.setHost("localhost"); + producer.setPort(BigDecimal.valueOf(8080)); + producer.setTls(true); + producer.setIdpClientId("producer-" + unique()); + return producerRepository.saveAndFlush(producer); + } + + private Product product(Producer producer) { + Product product = new Product(); + product.setName("Product " + unique()); + product.setTopic("topic." + unique()); + product.setProducer(producer); + return productRepository.saveAndFlush(product); + } + + private Consumer consumer(Organisation organisation) { + Consumer consumer = new Consumer(); + consumer.setName("Consumer " + unique()); + consumer.setOrg(organisation); + consumer.setIdpClientId("consumer-" + unique()); + consumer.setScheduleType("cron"); + return consumerRepository.saveAndFlush(consumer); + } + + private void grant(Product product, Consumer consumer, Instant grantedAt, int validityDays) { + ProductConsumer grant = new ProductConsumer(); + grant.setProduct(product); + grant.setConsumer(consumer); + grant.setGrantedTs(Timestamp.from(grantedAt)); + grant.setValidity(BigDecimal.valueOf(validityDays)); + grant.setScheduleType("cron"); + productConsumerRepository.saveAndFlush(grant); + } + + private static String unique() { + return UUID.randomUUID().toString().substring(0, 8); + } +} diff --git a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepositorySubscriptionTest.java b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepositorySubscriptionTest.java new file mode 100644 index 00000000..0057c73e --- /dev/null +++ b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/persistency/repository/configuration/ProductDiscoveryRepositorySubscriptionTest.java @@ -0,0 +1,76 @@ +package uk.gov.dbt.ndtp.ia.node.management.persistency.repository.configuration; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import uk.gov.dbt.ndtp.ia.node.management.service.discovery.DiscoverySchema; + +@ExtendWith(MockitoExtension.class) +class ProductDiscoveryRepositorySubscriptionTest { + + @Mock + private NamedParameterJdbcTemplate jdbc; + + @Captor + private ArgumentCaptor sql; + + @Captor + private ArgumentCaptor> parameters; + + private ProductDiscoveryRepository repository; + + @BeforeEach + void setUp() { + repository = new ProductDiscoveryRepository(jdbc, new DiscoverySchema("mn")); + } + + @Test + void findSubscribedProductIds_returnsTheProductsTheOrganisationHoldsAGrantOn() { + when(jdbc.queryForList(anyString(), anyMap(), eq(Long.class))).thenReturn(List.of(1L, 3L)); + + assertThat(repository.findSubscribedProductIds("BCC", List.of(1L, 2L, 3L))) + .containsExactlyInAnyOrder(1L, 3L); + } + + @Test + void findSubscribedProductIds_matchesGrantsThroughAnyConsumerOfTheOrganisation() { + repository.findSubscribedProductIds("BCC", List.of(1L, 2L)); + + verify(jdbc).queryForList(sql.capture(), parameters.capture(), eq(Long.class)); + assertThat(sql.getValue()) + .startsWith("SELECT DISTINCT pc.product_id") + .contains("FROM \"mn\".product_consumer pc") + .contains("JOIN \"mn\".consumer c ON c.id = pc.consumer_id") + .contains("JOIN \"mn\".organisation o ON o.id = c.org_id") + .contains("pc.product_id IN (:ids)") + .contains("o.organisation_key = :organisationKey") + .doesNotContain("validity"); + assertThat(parameters.getValue()) + .containsEntry("organisationKey", "BCC") + .containsEntry("ids", List.of(1L, 2L)); + } + + @Test + void findSubscribedProductIds_withNoOrganisationOrNoProducts_queriesNothing() { + assertThat(repository.findSubscribedProductIds(null, List.of(1L))).isEmpty(); + assertThat(repository.findSubscribedProductIds("BCC", List.of())).isEmpty(); + + verify(jdbc, never()).queryForList(anyString(), any(Map.class), eq(Long.class)); + } +} diff --git a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductDiscoveryServiceImplTest.java b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductDiscoveryServiceImplTest.java index b5a9e5d7..e69497f0 100644 --- a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductDiscoveryServiceImplTest.java +++ b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductDiscoveryServiceImplTest.java @@ -165,7 +165,7 @@ private void databaseReturns(long total, List products) { when(repository.count(any())).thenReturn(total); if (total > 0) { when(repository.findPage(any())).thenReturn(List.of(Map.of("id", 1L))); - when(assembler.assemble(any(), any(), any())).thenReturn(products); + when(assembler.assemble(any(), any(), any(), any())).thenReturn(products); } } @@ -192,7 +192,7 @@ private static String selectList(ProductSearchQuery query) { /** The projection the assembler was asked to build the response to. */ private ProductProjection capturedProjection() { ArgumentCaptor projection = ArgumentCaptor.forClass(ProductProjection.class); - verify(assembler).assemble(any(), any(), projection.capture()); + verify(assembler).assemble(any(), any(), projection.capture(), any()); return projection.getValue(); } @@ -461,7 +461,7 @@ void discover_page_isAssembledUnderTheSameContractTheQueryWasBuiltFrom() { service().discover(null, decision(true, permissiveDetails())); ArgumentCaptor contract = ArgumentCaptor.forClass(ProductSearchContract.class); - verify(assembler).assemble(any(), contract.capture(), any()); + verify(assembler).assemble(any(), contract.capture(), any(), any()); assertThat(contract.getValue().isEnforced()).isTrue(); assertThat(contract.getValue().rowFilter()).isInstanceOf(FilterNode.Literal.class); } diff --git a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductViewServiceImplTest.java b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductViewServiceImplTest.java index 62c4a7a4..f42b35cd 100644 --- a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductViewServiceImplTest.java +++ b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/data/impl/ProductViewServiceImplTest.java @@ -187,7 +187,7 @@ private void databaseReturns(DiscoveredProductDTO... products) { products.length == 0 ? List.of() : List.of(Map.of(ProductSearchQuery.ID, (Object) PRODUCT_ID)); when(repository.findPage(any())).thenReturn(rows); if (products.length > 0) { - when(assembler.assemble(any(), any(), any())).thenReturn(List.of(products)); + when(assembler.assemble(any(), any(), any(), any())).thenReturn(List.of(products)); } } @@ -213,7 +213,7 @@ private static String selectList(ProductSearchQuery query) { /** The projection the assembler was asked to build the response to. */ private ProductProjection capturedProjection() { ArgumentCaptor projection = ArgumentCaptor.forClass(ProductProjection.class); - verify(assembler).assemble(any(), any(), projection.capture()); + verify(assembler).assemble(any(), any(), projection.capture(), any()); return projection.getValue(); } @@ -297,7 +297,7 @@ void view_product_isAssembledUnderTheSameContractTheQueryWasBuiltFrom() { service().view(PRODUCT_ID, ownOrganisationDecision()); ArgumentCaptor contract = ArgumentCaptor.forClass(ProductSearchContract.class); - verify(assembler).assemble(any(), contract.capture(), any()); + verify(assembler).assemble(any(), contract.capture(), any(), any()); assertThat(contract.getValue().isEnforced()).isTrue(); assertThat(contract.getValue().rowFilter()) .isEqualTo(decisionRowFilter(ownOrganisationDecision())) @@ -446,7 +446,7 @@ void view_policySwitchedOff_withholdsNothing() { service(false).view(PRODUCT_ID, Optional.empty()); ArgumentCaptor contract = ArgumentCaptor.forClass(ProductSearchContract.class); - verify(assembler).assemble(any(), contract.capture(), any()); + verify(assembler).assemble(any(), contract.capture(), any(), any()); assertThat(contract.getValue().isEnforced()).isFalse(); assertThat(contract.getValue().masksSensitiveAttributes()).isFalse(); assertThat(contract.getValue().isVisible("source")).isTrue(); diff --git a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/discovery/DiscoveredProductAssemblerTest.java b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/discovery/DiscoveredProductAssemblerTest.java index 821aa472..37e60f7f 100644 --- a/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/discovery/DiscoveredProductAssemblerTest.java +++ b/src/test/java/uk/gov/dbt/ndtp/ia/node/management/service/discovery/DiscoveredProductAssemblerTest.java @@ -296,6 +296,49 @@ private ProductSearchContract enforcing(String detailsJson) throws JsonProcessin } /** One page row, keyed by the column aliases of {@link ProductSearchQuery} and {@link ProductField}. */ + @Test + void assemble_callerOrganisation_marksEachProductSubscribedOrNot_inOneQuery() { + when(repository.findSubscribedProductIds(eq("BCC"), any())).thenReturn(Set.of(1L)); + + List products = assembler.assemble( + List.of(summaryRow(1L), summaryRow(2L)), + ProductSearchContract.open(), + ProductProjection.SUMMARY, + "BCC"); + + assertThat(products) + .extracting(DiscoveredProductDTO::subscriptionStatus) + .containsExactly( + DiscoveredProductDTO.SubscriptionStatus.SUBSCRIBED, + DiscoveredProductDTO.SubscriptionStatus.NOT_SUBSCRIBED); + verify(repository).findSubscribedProductIds(eq("BCC"), idsCaptor.capture()); + assertThat(idsCaptor.getValue()).containsExactly(1L, 2L); + } + + @Test + void assemble_subscriptionStatus_isNotWithheldByMasking() throws JsonProcessingException { + ProductSearchContract contract = enforcing(""" + {"visible_fields": ["name"]}"""); + when(repository.findSubscribedProductIds(eq("BCC"), any())).thenReturn(Set.of(1L)); + + DiscoveredProductDTO product = assembler + .assemble(List.of(row(1L)), contract, ProductProjection.FULL, "BCC") + .get(0); + + assertThat(product.subscribedBy()).isNull(); + assertThat(product.subscriptionStatus()).isEqualTo(DiscoveredProductDTO.SubscriptionStatus.SUBSCRIBED); + } + + @Test + void assemble_noCallerOrganisation_leavesTheStatusOut_andQueriesNothing() { + DiscoveredProductDTO product = assembler + .assemble(List.of(summaryRow(1L)), ProductSearchContract.open(), ProductProjection.SUMMARY) + .get(0); + + assertThat(product.subscriptionStatus()).isNull(); + verify(repository, never()).findSubscribedProductIds(any(), any()); + } + private static Map row(long productId) { Map row = new LinkedHashMap<>(); row.put(ProductSearchQuery.ID, productId);