Feature/ad 368

[pjaneta]

Description

Tested scenarios

Fixed issue:

[kpieloch]

CardSubscriptionHandler.java, IdealSubscriptionHandler.java, KlarnaSubscriptionHandler.java, PayPalSubscriptionHandler.java

These four files share almost identical logic in updatePaymentRequest. Did you consider a generalisation of this code ? For example:

PaymentMethodHandler {

    protected abstract T getPaymentMethodDetails(PaymentRequest request);
    protected abstract T createNewPaymentMethodDetails();

    @Override
    public void updatePaymentRequest(PaymentRequest paymentRequest, CartData cartData, ...) {
        if (StringUtils.isNotEmpty(cartData.getAdyenSelectedReference())) {
            T details = getPaymentMethodDetails(paymentRequest);
            if (details == null) {
                details = createNewPaymentMethodDetails();
            }
            
            details.setStoredPaymentMethodId(cartData.getAdyenSelectedReference());
            
            CheckoutPaymentMethod checkoutPaymentMethod = new CheckoutPaymentMethod();
            checkoutPaymentMethod.setActualInstance(details);
            paymentRequest.setPaymentMethod(checkoutPaymentMethod);

            paymentRequest.setRecurringProcessingModel(PaymentRequest.RecurringProcessingModelEnum.SUBSCRIPTION);
            paymentRequest.setShopperInteraction(PaymentRequest.ShopperInteractionEnum.CONTAUTH);
        }
    }
}

[kpieloch]

Throwing NotImplementedException inside an Event Listener is risky. If the event system retries.

Suggested change

	throw new NotImplementedException("TokenizationWebhookEventListener not implemented for type " + data.getEventType());
	LOG.warn("Received Tokenization event of type [" + data.getEventType() + "] which is not currently handled.");

[kpieloch]

The strings "test" and "live" are hardcoded in the cross-check logic.
Move these to constants in AdyenConstants or a similar configuration class to ensure consistency across the codebase.

[kpieloch]

If this cannot be refactored now, the comment should be more descriptive explaining why it is set to null (likely to detach the cloned cart from the original order context for the CronJob) and link to a Jira ticket/backlog item.

[kpieloch]

This logic assumes that cartData.getSubscriptionOrder() is false for the initial checkout and true only for the subsequent recurring runs. This might throw a NullPointerException (unless it's a primitive boolean, but the bean definition showed java.lang.Boolean).

[kpieloch]

The initialization block is an instance initializer, but the variable is static. This means objectMapper is re-initialized every time the controller is instantiated (which is fine for Singleton beans, but technically incorrect Java semantics for static variables).

Suggested change

	}
	private static final ObjectMapper objectMapper = new ObjectMapper()
	.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);