Remove unnecessary message processed custom objects

[lojzatran]

Before we were using custom objects to check if a particular message has been processed or not. Additionally we set lastFetchTimestamp according to the lastModifiedAt of the last processed message. As discussed, checking if message is processed or not is not necessary as this check should be done in the shop - this check is now removed. I also changed setting lastFetchTimestamp to the current start timestamp of the job.

[lojzatran]

In MessageFilter.java#L44. I would return null right away if payment==null. It helps to reduce 1 level in nested-if.

This method seems quite complex and I'm not sure how to do it easily. Can you refactor the code and gimme a snippet of it?

[leungkinghin-ct]

In MessageFilter.java#L44. I would return null right away if payment==null. It helps to reduce 1 level in nested-if.

This method seems quite complex and I'm not sure how to do it easily. Can you refactor the code and gimme a snippet of it?

I minimized the level of nested-if. Is it easier to read in this way?

   public CartAndMessage process(PaymentTransactionCreatedOrUpdatedMessage message) {
        LOG.debug("Called MessageFilter.process with parameter {}", message);
        final Payment payment = getCorrespondingPayment(message);
        if (payment == null) {
            LOG.error("There is no payment in commercetools platform with id {}.", message.getResource().getId());
            timeStampManager.setActualProcessedMessageTimeStamp(message.getLastModifiedAt());
            return null;
        }

        if (!paymentCreationConfigurationManager.isTransactionSuccessAndHasMatchingTransactionTypes(message, payment)) {
            LOG.debug("PaymentTransactionCreatedOrUpdatedMessage {} has incorrect transaction state to be processed.", message.getId());
            timeStampManager.setActualProcessedMessageTimeStamp(message.getLastModifiedAt());
            return null;
        }
        final Optional<Cart> oCart = getCorrespondingCart(payment);
        if (!oCart.isPresent()) {
            LOG.error("There is no cart connected to payment with id {}.", message.getResource().getId());
            timeStampManager.setActualProcessedMessageTimeStamp(message.getLastModifiedAt());
            return null;
        }

        if (oCart.isPresent()) {
            final Cart cart = oCart.get();
            if (cart.getCartState() != CartState.ORDERED) {
                return new CartAndMessage(cart, message);
            } else {
                LOG.debug("Cart {} is already ordered nothing to do.", cart.getId());
            }
        }
        // we tried to do all possible jobs. If CartAndMessage is not returned above -
        // don't try to process this message next time
        timeStampManager.setActualProcessedMessageTimeStamp(message.getLastModifiedAt());
        return null;
    }

[lojzatran]

In MessageFilter.java#L44. I would return null right away if payment==null. It helps to reduce 1 level in nested-if.

This method seems quite complex and I'm not sure how to do it easily. Can you refactor the code and gimme a snippet of it?

I minimized the level of nested-if. Is it easier to read in this way?

   public CartAndMessage process(PaymentTransactionCreatedOrUpdatedMessage message) {
        LOG.debug("Called MessageFilter.process with parameter {}", message);
        final Payment payment = getCorrespondingPayment(message);
        if (payment == null) {
            LOG.error("There is no payment in commercetools platform with id {}.", message.getResource().getId());
            timeStampManager.setActualProcessedMessageTimeStamp(message.getLastModifiedAt());
            return null;
        }

        if (!paymentCreationConfigurationManager.isTransactionSuccessAndHasMatchingTransactionTypes(message, payment)) {
            LOG.debug("PaymentTransactionCreatedOrUpdatedMessage {} has incorrect transaction state to be processed.", message.getId());
            timeStampManager.setActualProcessedMessageTimeStamp(message.getLastModifiedAt());
            return null;
        }
        final Optional<Cart> oCart = getCorrespondingCart(payment);
        if (!oCart.isPresent()) {
            LOG.error("There is no cart connected to payment with id {}.", message.getResource().getId());
            timeStampManager.setActualProcessedMessageTimeStamp(message.getLastModifiedAt());
            return null;
        }

        if (oCart.isPresent()) {
            final Cart cart = oCart.get();
            if (cart.getCartState() != CartState.ORDERED) {
                return new CartAndMessage(cart, message);
            } else {
                LOG.debug("Cart {} is already ordered nothing to do.", cart.getId());
            }
        }
        // we tried to do all possible jobs. If CartAndMessage is not returned above -
        // don't try to process this message next time
        timeStampManager.setActualProcessedMessageTimeStamp(message.getLastModifiedAt());
        return null;
    }

https://github.com/commercetools/commercetools-payment-to-order-processor/pull/92/commits/32a38aa13ad367c8dd96c7775925fe55c0f4b0d7