Feature/dpav 3162 producer config policy attributes - #70
Merged
nikan-negaresh-informed merged 37 commits intoSep 9, 2026
Merged
Conversation
Repository tests need real Postgres to exercise the plpgsql soft-delete triggers and partial unique indexes on the policy attribute schema, which the project's shared H2 test profile cannot run. AbstractPostgresRepositoryTest boots a Postgres container per test class, applies the real Flyway migrations, and leaves the existing H2-backed test setup untouched. DPAV-3154
Maps the attribute_definition table (policy attribute vocabulary: namespace, name, data type, validation metadata) with a findByNamespaceAndName lookup. JSONB columns (allowed_values, classification) map as raw String via @JdbcTypeCode(SqlTypes.JSON) - this layer carries them opaquely rather than inventing a structured shape ahead of the service layer that will interpret them. Repository tests cover the lookup and the uq_attribute_definition__namespace_name uniqueness constraint. DPAV-3156
Maps the attribute_definition_scope table (which scopes a definition is bound to, whether required there, and its default value) with a findByAttributeDefinitionId lookup returning all bindings for a definition. Repository tests cover a definition bound to multiple scopes and the uq_attribute_definition_scope__definition_scope uniqueness constraint. DPAV-3157
Maps the attribute_value table with a findByAttributeDefinitionScopeIdAndEntityIdAndIsDeletedFalse lookup for the live value(s) recorded against a given scope binding and entity. entityId stays a plain Long column, not a JPA relationship - it is a polymorphic reference whose target table varies by scope, per the migration's comment. uq_attr_value_live is a partial unique index on (attribute_definition_scope_id, entity_id, value), so it only rejects an exact-duplicate live value - it does not by itself enforce a single live value per entity for single-valued attributes. Repository tests cover the duplicate-value rejection, that a distinct value for the same binding+entity is accepted, and that re-adding a duplicate value succeeds once the prior one is soft-deleted. DPAV-3158
Verifies the migration's five AFTER DELETE triggers (trg_organisation_attribute_value_soft_delete and its consumer, producer, product, and product_consumer counterparts): deleting an owning row soft-deletes its live attribute_value rows instead of leaving them orphaned, and deleting an entity with no attribute values is a no-op against attribute_value. DPAV-3159
…a JUnit extension CI's build job failed: the first several repository test classes each timed out acquiring a JDBC connection (CannotCreateTransactionException), while classes running later in the same job passed. Relying on @Testcontainers/@container to start the shared static container in each class's beforeAll raced against Spring building that class's ApplicationContext - some classes got a HikariCP pool built before the container was actually accepting TCP connections. Start the container in a static initializer instead, before any JUnit lifecycle callback runs for any subclass. Verified locally: all 5 repository test classes together (19 tests) now pass in ~9s using a single shared Spring context, versus repeatedly timing out over several minutes before. Full suite (296 tests) still green.
…itFields SonarCloud flagged 5.6% new-code duplication (gate: <=3%) - the is_deleted/created_at/created_by/updated_at/updated_by block was copy-pasted identically across AttributeDefinition, AttributeDefinitionScope, and AttributeValue. Extracted into a @MappedSuperclass all three now extend; Hibernate still maps the fields into each entity's own table exactly as before, so no schema or behavior change.
Trivy's security-scanning check flagged 3 CRITICAL CVEs (CVE-2026-65182, CVE-2026-65905, CVE-2026-68525) in tomcat-embed-core 10.1.55, the version Spring Boot 3.5.16 manages by default. Fixed upstream in 10.1.58; 10.1.58 itself isn't published to Maven Central, so pin to 10.1.59 (next available release, also fixed) via the tomcat.version override property Spring Boot's parent POM exposes for this. Not introduced by this branch - develop's last scan predates these CVEs being published to Trivy's DB and would fail the same way if rescanned today.
Add ComparisonOperator/Combinator/FilterNode/FilterCompilationException mirroring opa-pov's filter package (adapted to this project's package layout), the base vocabulary the upcoming Specification compiler will validate and compile caller filters against.
Add AttributeType, ResourceType/ResourceAttribute/ResourceDefinition, ConfigurationResourceRegistry (static fixed columns) and DynamicAttributeResolver (per-lookup resolution against the existing attribute_definition/attribute_definition_scope tables, so a newly registered attribute is filterable without a restart). One caller attribute name resolves through ConfigurationResourceRegistry.resolve regardless of whether it turns out to be a fixed column or a dynamic attribute. Adds AttributeDefinitionScopeRepository.findByAttributeDefinition_Id AndAttributeScope_CodeAndIsDeletedFalse to resolve a dynamic attribute's live scope binding in one query instead of joining attribute_scope at filter-compile time.
…r filters
Compile a validated FilterNode into a Spring Data JPA Specification: a
fixed attribute becomes a direct CriteriaBuilder predicate on the
entity path, a dynamic attribute becomes a correlated EXISTS subquery
against attribute_value scoped by the attribute's resolved
attribute_definition_scope.id (never a caller-supplied string), with
the value cast per the attribute's declared data_type.
Settled the JSONB extraction mechanism against a real Postgres
container: the originally-planned #>> '{}' / jsonb_extract_path_text
zero-path-element call isn't reachable through JPA's
CriteriaBuilder.function, so the compiler casts the column to text via
HibernateCriteriaBuilder.cast and unquotes STRING values with btrim
instead - documented in design.md alongside the simplified EXISTS
subquery (correlates on attribute_definition_scope_id directly, no
attribute_scope join needed inside the subquery).
Full suite (347 tests) green after this change.
…vice layers Extend ProducerRepository/ConsumerRepository with JpaSpecificationExecutor, overriding findAll(Specification) with an @EntityGraph so the filtered path fetch-joins products/productConsumers instead of N+1-loading them (verified via Hibernate statistics in a real-Postgres test). Add a Specification-accepting overload to ProducerService/ConsumerService (and their impls) that ANDs the caller's compiled filter with client-id scoping - ConfigurationProviderImpl only holds these service interfaces, not the repositories, so the filter has to cross that boundary too. Full suite (353 tests) green after this change.
… endpoints Add FilterRequestParser (JSON parse + 20-comparison cap, mirroring opa_poc.api.SearchRequest's cap) and a GlobalExceptionHandler mapping for FilterCompilationException: REQUEST origin -> 400 with the exception's own message (already scoped to only the caller-supplied attribute name), POLICY origin -> 500 with a generic body, detail server-side only. Extend ConfigurationController with an optional `filter` query param on both endpoints, and ConfigurationProvider/ConfigurationProviderImpl with a 3-arg overload (existing 2-arg methods delegate to it unchanged) that builds one Specification from producer_id/consumer_id and the compiled caller filter together, replacing the old in-memory id narrowing on that path. Updates existing ConfigurationProviderImplTest/ConfigurationController Test/ConfigurationPolicyEnforcementIntegrationTest mocks for the new service-layer Specification overloads - id-narrowing correctness now lives in SpecificationPredicateCompilerTest and ProducerConsumerSpecificationRepositoryTest instead of these mocked unit tests. Full suite (365 tests) green after this change.
Cover spec.md's core observable-behaviour requirements with real Postgres, the real Specification compiler, and real service/converter beans: filter on a fixed column, filter on a dynamically registered attribute, the client-scope boundary (a filter cannot widen access to another client's records), unfiltered behaviour is unchanged, and a newly-registered dynamic attribute is filterable without a restart. Exercises ProducerService/ConsumerService directly rather than through ConfigurationProviderImpl (pulls in unrelated certificate-validation/ product-consumer machinery) or over HTTP (no @SpringBootTest/full security-stack precedent exists anywhere in this codebase to build on) - documented in the test's class Javadoc as the narrowest real-Postgres slice that actually proves the new query path end-to-end. Full suite (370 tests) green after this change.
…nerics Karpathy-guidelines/simplify pass: FilterNode.Literal mirrored opa_poc.filter's policy-emitted constant predicate, but nothing in this change emits one (no policy-emitted row filter is in scope) - it was dead code with no caller ever constructing it. Dropped, along with its compiler switch case and the countComparisons case for it. Also drops SpecificationPredicateCompiler.buildComparison's unused AttributeType parameter and replaces its repeated fully-qualified jakarta.persistence.criteria.Expression references with a plain import - no behavior change. mvn clean verify (370 tests) and spotless:check both green after this change.
- Restore pre-existing behaviour whenever no caller filter is supplied: ConfigurationProviderImpl.getFilteredActiveProducers/ getFilteredConsumers now route through the old JOIN-FETCH-based service methods (not the new Specification/@EntityGraph path) for every no-filter request, including ones that still supply producer_id/consumer_id. The old JOIN FETCH is an implicit inner join and silently excludes a producer with zero products; the new @EntityGraph fetch is an outer join and would have started including it for every caller, not just new-filter users - confirmed empirically against real Postgres (1 row via the old path, 0 via the new one for a zero-product producer). - Reject a syntactically valid but semantically incomplete filter (a comparison missing "attribute"/"operator", a group missing "combinator", a bare JSON `null`, or a null element inside "nodes") in FilterRequestParser, instead of letting it throw an unhandled NullPointerException deeper in resolution/compilation. The @NotNull/@notblank annotations on the FilterNode records were never enforced - this project has no Bean Validation provider on the classpath - so readValue() alone doesn't catch these. - Reject neq/not_in against a multi-valued dynamic attribute: each Comparison compiles to one EXISTS subquery, so neq/not_in meant "EXISTS a value that doesn't match" (true as soon as any other value is present), not "does not have this value" as a caller would expect. eq/in keep their unambiguous "has a matching value" EXISTS semantics. - Extract the repeated `(root, query, cb) -> cb.equal(root.get(field), value)` Specification idiom (independently hand-rolled 3x) into Specifications.fieldEquals. Adds regression tests for all four: routing-decision unit tests, a real-Postgres test documenting the old-vs-new join semantics, FilterRequestParser null-validation cases, and multi-valued eq/neq compiler tests. Full suite (383 tests) green, spotless clean.
BigDecimal.equals() compares scale as well as value, so a validity of "0.00" was not recognised as the ZERO sentinel for "no expiry" and would incorrectly fall through to the granted-date/validity check. Flagged by SonarCloud (new_reliability_rating C, blocking PR #69's quality gate) as java:S9351.
- ConfigurationResourceRegistry: extract a fixed(name, jpaPath, type) helper so each fixed attribute's logical name is written once instead of twice (map key + constructor arg), removing the "description"/"active"/"orgId"/"scheduleType"/"scheduleExpression" duplicated-literal (S1192) smells. - SpecificationPredicateCompiler: drop the unnecessary raw Expression cast in the IN/NOT_IN branches (S1905) - Expression<?>.in(...) works directly. - FilterRequestParser.validate: use record deconstruction patterns instead of binding-then-accessor-calls (S6878). - FilterRequestParserTest: replace 4 near-identical rejection tests with one @ParameterizedTest (S5976). - SpecificationPredicateCompilerTest/DynamicAttributeResolverTest: extract the Specification/resolver construction out of each assertThatThrownBy lambda so only the one call that can actually throw remains inside it (S5778), and drop an unused local variable (S1854/S1481). - ConfigurationProviderImplTest: drop unnecessary eq(...) matchers around constant arguments (S6068) and extract an inline mock() call to a named local variable (S9016). None of these were quality-gate blocking (new_maintainability_rating was already A) - fixed because they were visible on the Sonar PR dashboard. Full suite (383 tests) green, spotless clean.
- Rename a local "resolver" var to "underTest" in DynamicAttributeResolverTest (java:S1117 - it shadowed the field of the same name). - Add positive SpecificationPredicateCompiler tests for neq/in/ not_in/lt/lte/gte/contains against fixed columns - only eq/gt had positive coverage before, leaving most of the operator switch in buildComparison untested. - Add the CONSUMER analogue of the producer filter-present routing test, closing the two uncovered lines in ConfigurationProviderImpl.getFilteredConsumers's filter-present branch (the producer branch was already covered). Full suite (389 tests) green, spotless clean.
Drops the FilterNode/Combinator/ComparisonOperator DSL, the SpecificationPredicateCompiler, and the resource attribute registry - the whole mechanism translating a caller-supplied filter query param into a JPA Specification. The attribute-schema persistence layer this built on top of stays.
Covers the filter/ package's own unit tests plus the two integration tests exercising it end-to-end (ConfigurationFilteringIntegrationTest, ProducerConsumerSpecificationRepositoryTest).
…oints Reverts ConfigurationController to its pre-filter shape: no FilterRequestParser dependency, no filter request param on /producer or /consumer, calls the pre-existing 2-arg provider methods directly.
…vider Reverts ConfigurationProvider/ConfigurationProviderImpl to their pre-filter shape: single 2-arg getConsumerConfigByClientId/ getProducerConfigByClientId methods, no SpecificationPredicateCompiler dependency, no getFilteredConsumers/getFilteredActiveProducers/ idAndFilterSpecification helpers.
Removes the Specification-accepting overloads on ProducerService/ ConsumerService (and their impls) added only to support the filter query param, and the JpaSpecificationExecutor/@EntityGraph findAll overrides on ProducerRepository/ConsumerRepository that backed them - no other caller used any of them.
…finder Removes findByAttributeDefinition_IdAndAttributeScope_CodeAndIsDeletedFalse, added only for the now-deleted DynamicAttributeResolver and unused by anything else.
Removes GlobalExceptionHandler's handleFilterCompilationException and its tests - the exception type it handled is gone. Also updates ConfigurationPolicyEnforcementIntegrationTest (unrelated PEP feature, only incidentally wired FilterRequestParser into its controller instance) to build ConfigurationController with its 2-arg constructor and stub/verify the 2-arg provider methods.
…ScopeCode DPAV-3162 needs "every live policy attribute for entity X in scope S", not the one-named-attribute lookup the repository already had. Adds a JOIN FETCH query resolving the defining attribute_definition_scope/ attribute_definition alongside each attribute_value row, so callers don't N+1 per row.
PolicyAttributeDTO mirrors the legacy AttributesDTO's three fields (name, value, type) - a drop-in "policy" counterpart per DPAV-3162, backed by the EAV schema instead of product_consumer_attribute. PolicyAttributeScope is a closed vocabulary for the four attribute_scope.code values this change resolves (PRODUCER, CONSUMER, ORGANISATION, SUBSCRIPTION) - deliberately not PRODUCT, out of scope per the proposal. Verified against real Postgres that each code names a seeded attribute_scope row.
Wraps findLiveByEntityIdAndScopeCode and maps to PolicyAttributeDTO: name = namespace.name, value rendered as plain text via ObjectMapper.readTree(...).asText() (strips a JSON string's quotes, renders a number/boolean as-is), falling back to the raw stored text on a parse failure rather than throwing - a display-layer concern, not a predicate to compile.
ProducerDTO.policyAttributes, ConsumerDTO.policyAttributes/ organisationPolicyAttributes, ProductConsumerDTO.policyAttributes - each a final List initialised to new ArrayList<>() (matching the existing attributes field's pattern), so an unpopulated field always serialises as [], never null or omitted. Also adds ProductConsumerDTO.id (@JsonIgnore) and populates it in ProductConsumerConverter - the DTO previously carried no reference to its own product_consumer.id, which SUBSCRIPTION-scope policy attribute lookups need as their entity key (design.md already specified this key; the DTO just didn't carry it yet).
…entId Attaches policy attributes to the assembled producer config DTO graph: producer (PRODUCER scope), each allowed consumer (CONSUMER) and its organisation (ORGANISATION), and each subscription (SUBSCRIPTION, keyed by product_consumer.id). getConsumerConfigByClientId is untouched. populateConsumersForProducers now also sets product.configurations (previously never populated on the producer-config path) - without it, ProductConsumerDTO.policyAttributes would sit on an object the response never actually returns, silently failing to fulfil the already-approved spec's "each subscription's entry... includes it in its policy attributes" requirement. The same filtered findByDataProviderId result already computed is reused for both configurations and resolved consumers, no extra query added. Full suite (315 tests) green.
…ibutes Seeds live attribute values across all four scopes (PRODUCER, CONSUMER, ORGANISATION, SUBSCRIPTION) against a real Postgres container and asserts the assembled ProducerConfigDTO carries them at the right level, plus the empty-array case for a sibling producer with none.
Notes that each producer, allowed consumer, its organisation, and each subscription now carry live policy attributes in the response.
Contributor
✅ OSS Checks PassedAll tracked OSS checks passed in this run.
Results from commit 203a55a, view the full job summary ♻️ This comment has been updated with latest results. |
|
nikan-negaresh-informed
approved these changes
Sep 9, 2026
nikan-negaresh-informed
marked this pull request as ready for review
September 9, 2026 10:45
nikan-negaresh-informed
deleted the
feature/DPAV-3162-producer-config-policy-attributes
branch
September 9, 2026 10:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sensitive Credential Checks
Motivation and Context
DPAV-3162:
GET /api/v1/configuration/producerdid not surface any policy scope attributes (from the EAVattribute_scope/attribute_definition/attribute_valueschema), forcing downstream PDP evaluation to fetch them separately.Description
AttributeValueRepository.findLiveByEntityIdAndScopeCodeto resolve live (non-deleted) attribute values for an entity within oneattribute_scope.PolicyAttributeDTO(name/value/type) and aPolicyAttributeScopeenum (PRODUCER,CONSUMER,ORGANISATION,SUBSCRIPTION).PolicyAttributeService/PolicyAttributeServiceImpl, rendering each attribute's JSON value as plain text with a fallback to the raw stored text on parse failure.policyAttributestoProducerDTO/ProductConsumerDTOandpolicyAttributes/organisationPolicyAttributestoConsumerDTO, always serialised as[]when empty.PolicyAttributeServiceintoConfigurationProviderImpl.getProducerConfigByClientId, attaching attributes to the producer, each allowed consumer, its organisation, and each subscription.getConsumerConfigByClientIdis untouched.product.configurationson the producer-config path (previously never set there) - required so each subscription has aProductConsumerDTOto attachSUBSCRIPTION-scope attributes to.GET /api/v1/configuration/producerOpenAPI description to document the new fields.How Has This Been Tested?
PolicyAttributeServiceImpl, DTO serialisation, andConfigurationProviderImplwiring (mockedPolicyAttributeService).ProducerConfigDTO, plus the empty-array case for a resource with none.mvn clean verifyandmvn spotless:checkpass.Screenshots (if appropriate):
Checklist: