feat: issuance process pending guard

[thomasrutger]

What this PR changes/adds

Introduces a pending guard that can stall an issuance process (on the issuer side), until some external condition is met.

Why it does that

In some circumstances it may be desirable to stall issuance until an external process validates the request, such as a manual approval procedure.

Further notes

I noticed that the following block in DcpIssuanceFlowEndToEndTest checked for the process to be in the ISSUED state, not the REQUESTED state. I assumed this was incorrect and changed it such that it works with the updated test layout.

        // wait for the request status to be requested on the holder side
        await().pollInterval(INTERVAL)
                .atMost(TIMEOUT)
                .untilAsserted(() -> assertThat(identityHub.getCredentialRequestForParticipant(PARTICIPANT_ID)).hasSize(1)
                        .allSatisfy(t -> {
                            assertThat(t.getState()).isEqualTo(HolderRequestState.ISSUED.code());
                            assertThat(t.getHolderPid()).isEqualTo("test-request-id");
                        }));

Who will sponsor this feature?

@paullatzelsperger

Linked Issue(s)

Closes #829

[thomasrutger]

@paullatzelsperger Should I also update DcpAnonymousIssuanceFlowEndToEndTest and/or DcpIssuanceFlowAllInOneTest?

[paullatzelsperger]

@paullatzelsperger Should I also update DcpAnonymousIssuanceFlowEndToEndTest and/or DcpIssuanceFlowAllInOneTest?

The anonymous test may be useful. the all-in-one less so, because it tests a different scenario.

[paullatzelsperger]

looks good, some minor refactoring is required in the E2E tests

[paullatzelsperger]

please move the pending guard test into a second test method issuanceFlow_withGuard leaving the original method untouched. Yes, that might duplicate some code, but that second method would only need to assert that the issuance process does not advance automatically and remains in the given state.

[paullatzelsperger]

see comment in the anonymous holder test

[paullatzelsperger]

this should probably be explicitly asserted in a test as well

[thomasrutger]

@paullatzelsperger

1. I split the e2e tests with a separate tests for the guard. note that I had to change the original test by explicitly getting the issuance processes by holderPid. Runtimes are not cleaned up in between tests and that would result in failed assertions (number of processes must match 1). A cleanup hook would also work, but I figured that would slow down the test and this works as well. Let me know if this is acceptable.
2. I added a test to IssuanceProcessStoreTestBase to test persistence of the pending flag.
3. After switching the first holder-side check from ISSUED to REQUESTED the test became flaky because the request often advances to ISSUED before await() runs. I reverted that change to keep it stable. That said, the current order still feels odd—it asserts the holder reaches ISSUED before verifying the issuer reached DELIVERED. Would it make sense to first wait for the issuer to hit DELIVERED, then assert the holder is ISSUED, so the checks line up with the real lifecycle?

[paullatzelsperger]

@thomasrutger our typical work model on PRs is once you've resolved all comments, or post replies to comments, you re-request the review to let the reviewer know you're ready for another round.
That way, we have it in our to-to list, and it limits the scope of subsequent changes, which makes the review process easier.

[paullatzelsperger]

this assertion is not needed: we are only interesting the issuer-side request to have the pending flag set, i.e the assertion in line #244

[paullatzelsperger]

see comment in the anonymous test

[paullatzelsperger]

this tests the update case, and technically the read case, but not the insert case. I would expect one method update_shouldPersistPendingFlag and insert_shouldPersistPendingFlag

[github-actions]

This pull request is stale because it has been open for 14 days with no activity.

[github-actions]

This pull request was closed because it has been inactive for 7 days since being marked as stale.