[GitHub] [rocketmq] Switch-vov edited a comment on issue #1178: Unreachable dead code in com.alibaba.rocketmq.client.impl.MQClientAPIImpl#processSendResponse

[GitBox <gi...@apache.org>]

Switch-vov edited a comment on issue #1178: Unreachable dead code in  com.alibaba.rocketmq.client.impl.MQClientAPIImpl#processSendResponse
URL: https://github.com/apache/rocketmq/issues/1178#issuecomment-540333943
 
 
   @xiangwangcheng @laosijikaichele
   
   After test it, it is reachable code, but it is very ambiguous.
   So, I think it should be refactoring to below code.
   
   before update:
   
   ``` java
       private SendResult processSendResponse(
           final String brokerName,
           final Message msg,
           final RemotingCommand response
       ) throws MQBrokerException, RemotingCommandException {
           switch (response.getCode()) {
               case ResponseCode.FLUSH_DISK_TIMEOUT:
               case ResponseCode.FLUSH_SLAVE_TIMEOUT:
               case ResponseCode.SLAVE_NOT_AVAILABLE: {
               }
               case ResponseCode.SUCCESS: {
                   SendStatus sendStatus = SendStatus.SEND_OK;
                   switch (response.getCode()) {
                       case ResponseCode.FLUSH_DISK_TIMEOUT:
                           sendStatus = SendStatus.FLUSH_DISK_TIMEOUT;
                           break;
                       case ResponseCode.FLUSH_SLAVE_TIMEOUT:
                           sendStatus = SendStatus.FLUSH_SLAVE_TIMEOUT;
                           break;
                       case ResponseCode.SLAVE_NOT_AVAILABLE:
                           sendStatus = SendStatus.SLAVE_NOT_AVAILABLE;
                           break;
                       case ResponseCode.SUCCESS:
                           sendStatus = SendStatus.SEND_OK;
                           break;
                       default:
                           assert false;
                           break;
                   }
   
                   // some code
               }
               default:
                   break;
           }
   
           throw new MQBrokerException(response.getCode(), response.getRemark());
       }
   ```
   
   updated code:
   
   ``` java
       private SendResult processSendResponse(
           final String brokerName,
           final Message msg,
           final RemotingCommand response
       ) throws MQBrokerException, RemotingCommandException {
           SendStatus sendStatus = null;
           switch (response.getCode()) {
               case ResponseCode.FLUSH_DISK_TIMEOUT: {
                   sendStatus = SendStatus.FLUSH_DISK_TIMEOUT;
                   break;
               }
               case ResponseCode.FLUSH_SLAVE_TIMEOUT: {
                   sendStatus = SendStatus.FLUSH_SLAVE_TIMEOUT;
                   break;
               }
               case ResponseCode.SLAVE_NOT_AVAILABLE: {
                   sendStatus = SendStatus.SLAVE_NOT_AVAILABLE;
                   break;
               }
               case ResponseCode.SUCCESS: {
                   sendStatus = SendStatus.SEND_OK;
                   break;
               }
               default:
                   break;
           }
   
           if (sendStatus != null) {
               // some code
           }
   
           throw new MQBrokerException(response.getCode(), response.getRemark());
       }
   
   ```
   
   unit test code `org.apache.rocketmq.client.impl.MQClientAPIImplTest#testSendMessageSync_WithProcessSendResponseHandleException`:
   
   ``` java
       @Test
       public void testSendMessageSync_WithProcessSendResponseHandleException() throws InterruptedException, RemotingException, MQBrokerException {
           int[] responseCodes = {ResponseCode.FLUSH_DISK_TIMEOUT, ResponseCode.FLUSH_SLAVE_TIMEOUT,
                   ResponseCode.SLAVE_NOT_AVAILABLE};
           SendStatus[] sendStatuses = {SendStatus.FLUSH_DISK_TIMEOUT, SendStatus.FLUSH_SLAVE_TIMEOUT, SendStatus.SLAVE_NOT_AVAILABLE};
           for (int i = 0; i < responseCodes.length; i++) {
               final int responseCode = responseCodes[i];
               doAnswer(new Answer() {
                   @Override
                   public Object answer(InvocationOnMock mock) throws Throwable {
                       RemotingCommand request = mock.getArgument(1);
                       RemotingCommand response = createSuccessResponse(request);
                       response.setCode(responseCode);
                       return response;
                   }
               }).when(remotingClient).invokeSync(anyString(), any(RemotingCommand.class), anyLong());
   
               SendMessageRequestHeader requestHeader = createSendMessageRequestHeader();
               SendResult sendResult = mqClientAPI.sendMessage(brokerAddr, brokerName, msg, requestHeader,
                       3 * 1000, CommunicationMode.SYNC, new SendMessageContext(), defaultMQProducerImpl);
               assertThat(sendResult.getSendStatus()).isEqualTo(sendStatuses[i]);
           }
       }
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services