IRS Policy Validation accepts subset of AND constraint

Description

Current behaviour

When IRS accepted policy has a AND constraint with 2 constraints and the EDC offer policy has a AND constraint with 1 constraint, IRS accepts the Policy if one of the IRS accepted policies matches the EDC offer policy AND constraint.

Expected behavior

IRS accepted Policies have to match exactly the EDC offer policy.

Steps to reproduce the Bug

IRS accepted Policy with AND constraint A1 and A2
EDC offer with AND constraint A1
IRS job with contract negotiation.
- actual: Negotiation is started with IRS accepting the policy
- expected: Negotiation is aborted with IRS not accepting the policy because constraint A2 is missing in EDC offer

Planning 2

Check for policies is done here

item-relationship-service/irs-edc-client/src/main/java/org/eclipse/tractusx/irs/edc/client/policy/ConstraintCheckerService.java

Lines 46 to 58 in 72b0396

    
           public boolean hasAllConstraint(final Policy acceptedPolicy, final List<Constraint> constraints) { 
        
               final List<Constraints> acceptedConstraintsList = acceptedPolicy.getPermissions() 
        
                                                                               .stream() 
        
                                                                               .map(Permission::getConstraint) 
        
                                                                               .toList(); 
        
               return constraints.stream().allMatch(constraint -> isValidOnList(constraint, acceptedConstraintsList)); 
        
           } 
        
           private boolean isValidOnList(final Constraint constraint, final List<Constraints> acceptedConstraintsList) { 
        
               return acceptedConstraintsList.stream() 
        
                                             .anyMatch(acceptedConstraints -> isSameAs(constraint, acceptedConstraints)); 
        
           }

Make sure that and constraints match for all constraints

This Test should validate that the check is working as expected: https://github.com/eclipse-tractusx/item-relationship-service/blob/a45960ba21905403ef6d55883709ad0e817221ac/irs-edc-client/src/test/java/org/eclipse/tractusx/irs/edc/client/policy/ConstraintCheckerServiceTest.java

    @Test
    void shouldNotAcceptAndConstraintWithOneLessElement1() {
        final AndConstraint andConstraint = createAndConstraint(
                List.of(createAtomicConstraint(TestConstants.FRAMEWORK_AGREEMENT_TRACEABILITY,
                                TestConstants.STATUS_ACTIVE),
                        createAtomicConstraint(TestConstants.PURPOSE, TestConstants.ID_3_1_TRACE)));

        final Policy acceptedPolicy = createPolicyWithAndConstraint(
                List.of(new Operand(TestConstants.FRAMEWORK_AGREEMENT_TRACEABILITY, TestConstants.STATUS_ACTIVE),
                        new Operand(TestConstants.MEMBERSHIP, TestConstants.STATUS_ACTIVE),
                        new Operand(TestConstants.PURPOSE, TestConstants.ID_3_1_TRACE)));

        boolean result = cut.hasAllConstraint(acceptedPolicy, List.of(andConstraint));

        assertThat(result).isFalse();
    }

@ds-jhartmann: shouldn't it be the other way around: andConstraint one constraint more than acceptedPolicy?

        final AndConstraint andConstraint = createAndConstraint(
                List.of(createAtomicConstraint(TestConstants.FRAMEWORK_AGREEMENT_TRACEABILITY,
                                TestConstants.STATUS_ACTIVE),
                        createAtomicConstraint(TestConstants.MEMBERSHIP, TestConstants.STATUS_ACTIVE),
                        createAtomicConstraint(TestConstants.PURPOSE, TestConstants.ID_3_1_TRACE)));

        final Policy acceptedPolicy = createPolicyWithAndConstraint(
                List.of(new Operand(TestConstants.FRAMEWORK_AGREEMENT_TRACEABILITY, TestConstants.STATUS_ACTIVE),
                        new Operand(TestConstants.PURPOSE, TestConstants.ID_3_1_TRACE)));

        boolean result = cut.hasAllConstraint(acceptedPolicy, List.of(andConstraint));

        assertThat(result).isFalse();

@ds-jhartmann: shouldn't it be the other way around: andConstraint one constraint more than acceptedPolicy?

Oh, you're right. The bug describes your behavior. However, the Test example I provided should also fail, since the Policy does not match exactly

Outcome

AND does not allow subsets in any direction anymore (list of constraints must be same size and all must match) -> see corrections in ConstraintCheckerService.isSameAs and ConstraintCheckerServiceTest.SubsetAndTests (https://github.com/eclipse-tractusx/item-relationship-service/blob/main/irs-edc-client/src/test/java/org/eclipse/tractusx/irs/edc/client/policy/ConstraintCheckerServiceTest.java#L106)
fixed tests in PolicyCheckerServiceTest and removed one obsolete test concerning XOne (which we do not support)
improved readability of tests

PR

#797

Successfully tested:
Starting Point: EDC has 2 constraints

IRS accepted policy has 3 constraints: syntax and value match EDC policy
IRS accepted policy has 2 constraints: syntax and value match EDC policy
IRS accepted policy has 2 constraints: syntax does not match EDC policy
IRS accepted policy has 2 constraints: value does not match EDC policy

Testscenario:

Create a policy 'X' with an additional constraint to the "default" policy.

Default policy contains the two constraints

Expected

Policy 'X' will not be accepted as there is an additional

LGFM. PO acceptance in behalf of @jzbmw .

	public boolean hasAllConstraint(final Policy acceptedPolicy, final List<Constraint> constraints) {
	final List<Constraints> acceptedConstraintsList = acceptedPolicy.getPermissions()
	.stream()
	.map(Permission::getConstraint)
	.toList();

	return constraints.stream().allMatch(constraint -> isValidOnList(constraint, acceptedConstraintsList));
	}

	private boolean isValidOnList(final Constraint constraint, final List<Constraints> acceptedConstraintsList) {
	return acceptedConstraintsList.stream()
	.anyMatch(acceptedConstraints -> isSameAs(constraint, acceptedConstraints));
	}