Merge pull request #486 from kazuki-ma/kazuki-ma/cleanServlet

kazuki-ma · web-flow · commit 5fef92f351c9 · 2020-03-26T10:22:51.000+09:00
NON-ISSUE Move LineSignatureValidator and delete line-bot-servlet dependency.
diff --git a/line-bot-api-client/build.gradle b/line-bot-api-client/build.gradle
@@ -42,9 +42,7 @@ check.dependsOn integrationTest
 
 dependencies {
     api project(':line-bot-model')
-    implementation project(':line-bot-parser') // TODO: Can we remove it? (By moving LineSignatureValidator)
     implementation 'org.slf4j:slf4j-api'
-
     implementation 'com.squareup.okhttp3:logging-interceptor'
     implementation 'com.squareup.retrofit2:converter-jackson'
     implementation 'com.squareup.retrofit2:retrofit'
diff --git a/line-bot-parser/src/main/java/com/linecorp/bot/parser/LineSignatureValidator.java b/line-bot-parser/src/main/java/com/linecorp/bot/parser/LineSignatureValidator.java
@@ -14,7 +14,7 @@
  * under the License.
  */
 
-package com.linecorp.bot.client;
+package com.linecorp.bot.parser;
 
 import java.security.InvalidKeyException;
 import java.security.MessageDigest;
@@ -24,8 +24,6 @@
 import javax.crypto.Mac;
 import javax.crypto.spec.SecretKeySpec;
 
-import com.linecorp.bot.parser.SignatureValidator;
-
 import lombok.NonNull;
 
 /**
@@ -47,7 +45,6 @@ public LineSignatureValidator(byte[] channelSecret) {
      *
      * @param content Body of the http request in byte array.
      * @param headerSignature Signature value from `X-LINE-Signature` HTTP header
-     *
      * @return True if headerSignature matches signature of the content. False otherwise.
      */
     @Override
@@ -61,7 +58,6 @@ public boolean validateSignature(@NonNull byte[] content, @NonNull String header
      * Generate signature value.
      *
      * @param content Body of the http request.
-     *
      * @return generated signature value.
      */
     public byte[] generateSignature(@NonNull byte[] content) {
diff --git a/line-bot-parser/src/main/java/com/linecorp/bot/parser/WebhookParser.java b/line-bot-parser/src/main/java/com/linecorp/bot/parser/WebhookParser.java
@@ -29,6 +29,8 @@
 
 @Slf4j
 public class WebhookParser {
+    public static final String SIGNATURE_HEADER_NAME = "X-Line-Signature";
+
     private final ObjectMapper objectMapper = ModelObjectMapper.createNewObjectMapper();
     private final SignatureValidator signatureValidator;
 
@@ -46,9 +48,7 @@ public WebhookParser(@NonNull SignatureValidator signatureValidator) {
      *
      * @param signature X-Line-Signature header.
      * @param payload Request body.
-     *
      * @return Parsed result. If there's an error, this method sends response.
-     *
      * @throws WebhookParseException There's an error around signature.
      */
     public CallbackRequest handle(String signature, byte[] payload) throws IOException, WebhookParseException {
diff --git a/line-bot-parser/src/test/java/com/linecorp/bot/parser/LineSignatureValidatorTest.java b/line-bot-parser/src/test/java/com/linecorp/bot/parser/LineSignatureValidatorTest.java
@@ -14,7 +14,7 @@
  * under the License.
  */
 
-package com.linecorp.bot.client;
+package com.linecorp.bot.parser;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
diff --git a/line-bot-servlet/src/main/java/com/linecorp/bot/servlet/LineBotCallbackException.java b/line-bot-servlet/src/main/java/com/linecorp/bot/servlet/LineBotCallbackException.java
@@ -19,7 +19,7 @@
 public class LineBotCallbackException extends Exception {
     private static final long serialVersionUID = -950894346433317253L;
 
-    public LineBotCallbackException(String message) {
-        super(message);
+    public LineBotCallbackException(String message, Exception e) {
+        super(message, e);
     }
 }
diff --git a/line-bot-servlet/src/main/java/com/linecorp/bot/servlet/LineBotCallbackRequestParser.java b/line-bot-servlet/src/main/java/com/linecorp/bot/servlet/LineBotCallbackRequestParser.java
@@ -23,14 +23,21 @@
 
 import com.google.common.io.ByteStreams;
 
-import com.linecorp.bot.client.LineSignatureValidator;
 import com.linecorp.bot.model.event.CallbackRequest;
+import com.linecorp.bot.parser.LineSignatureValidator;
 import com.linecorp.bot.parser.WebhookParseException;
 import com.linecorp.bot.parser.WebhookParser;
 
 import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
 
+/**
+ * Parser of webhook from LINE's messaging server.
+ *
+ * <p>This is a thin wrapper of {@link WebhookParser}.
+ *
+ * @see WebhookParser
+ */
 @Slf4j
 public class LineBotCallbackRequestParser {
     private final WebhookParser parser;
@@ -48,19 +55,17 @@ public LineBotCallbackRequestParser(@NonNull LineSignatureValidator lineSignatur
      * Parses a request.
      *
      * @param req HTTP servlet request.
-     *
      * @return Parsed result. If there's an error, this method sends response.
-     *
      * @throws LineBotCallbackException There's an error around signature.
      */
     public CallbackRequest handle(HttpServletRequest req) throws LineBotCallbackException, IOException {
         // validate signature
-        final String signature = req.getHeader("X-Line-Signature");
+        final String signature = req.getHeader(WebhookParser.SIGNATURE_HEADER_NAME);
         final byte[] json = ByteStreams.toByteArray(req.getInputStream());
         try {
             return parser.handle(signature, json);
         } catch (WebhookParseException e) {
-            throw new LineBotCallbackException(e.getMessage());
+            throw new LineBotCallbackException(e.getMessage(), e);
         }
     }
 
@@ -69,9 +74,7 @@ public CallbackRequest handle(HttpServletRequest req) throws LineBotCallbackExce
      *
      * @param signature X-Line-Signature header.
      * @param payload Request body.
-     *
      * @return Parsed result. If there's an error, this method sends response.
-     *
      * @throws LineBotCallbackException There's an error around signature.
      * @deprecated Use {@link WebhookParser#handle(String, byte[])} instead.
      */
@@ -81,7 +84,7 @@ public CallbackRequest handle(String signature, String payload)
         try {
             return parser.handle(signature, payload.getBytes(StandardCharsets.UTF_8));
         } catch (WebhookParseException e) {
-            throw new LineBotCallbackException(e.getMessage());
+            throw new LineBotCallbackException(e.getMessage(), e);
         }
     }
 
diff --git a/line-bot-servlet/src/test/java/com/linecorp/bot/servlet/LineBotCallbackRequestParserTest.java b/line-bot-servlet/src/test/java/com/linecorp/bot/servlet/LineBotCallbackRequestParserTest.java
@@ -35,11 +35,11 @@
 
 import com.google.common.io.ByteStreams;
 
-import com.linecorp.bot.client.LineSignatureValidator;
 import com.linecorp.bot.model.event.CallbackRequest;
 import com.linecorp.bot.model.event.Event;
 import com.linecorp.bot.model.event.MessageEvent;
 import com.linecorp.bot.model.event.message.TextMessageContent;
+import com.linecorp.bot.parser.LineSignatureValidator;
 
 public class LineBotCallbackRequestParserTest {
     @Rule
diff --git a/line-bot-spring-boot/build.gradle b/line-bot-spring-boot/build.gradle
@@ -17,7 +17,7 @@
 dependencies {
     api project(':line-bot-api-client')
     api project(':line-bot-model')
-    implementation project(':line-bot-servlet')
+    implementation project(':line-bot-parser')
     implementation 'org.springframework.boot:spring-boot-autoconfigure'
     implementation 'org.springframework.boot:spring-boot-starter-web'
     implementation 'com.google.guava:guava'
diff --git a/line-bot-spring-boot/src/main/java/com/linecorp/bot/spring/boot/LineBotWebMvcBeans.java b/line-bot-spring-boot/src/main/java/com/linecorp/bot/spring/boot/LineBotWebMvcBeans.java
@@ -24,8 +24,8 @@
 import org.springframework.context.annotation.Import;
 import org.springframework.stereotype.Component;
 
-import com.linecorp.bot.client.LineSignatureValidator;
-import com.linecorp.bot.servlet.LineBotCallbackRequestParser;
+import com.linecorp.bot.parser.LineSignatureValidator;
+import com.linecorp.bot.parser.WebhookParser;
 import com.linecorp.bot.spring.boot.interceptor.LineBotServerInterceptor;
 import com.linecorp.bot.spring.boot.support.LineBotServerArgumentProcessor;
 
@@ -46,11 +46,11 @@ public LineSignatureValidator lineSignatureValidator() {
     }
 
     /**
-     * Expose {@link LineBotCallbackRequestParser} as {@link Bean}.
+     * Expose {@link WebhookParser} as {@link Bean}.
      */
     @Bean
-    public LineBotCallbackRequestParser lineBotCallbackRequestParser(
+    public WebhookParser lineBotCallbackRequestParser(
             LineSignatureValidator lineSignatureValidator) {
-        return new LineBotCallbackRequestParser(lineSignatureValidator);
+        return new WebhookParser(lineSignatureValidator);
     }
 }
diff --git a/line-bot-spring-boot/src/main/java/com/linecorp/bot/spring/boot/interceptor/LineBotServerInterceptor.java b/line-bot-spring-boot/src/main/java/com/linecorp/bot/spring/boot/interceptor/LineBotServerInterceptor.java
@@ -25,23 +25,25 @@
 import org.springframework.core.MethodParameter;
 import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Component;
+import org.springframework.util.StreamUtils;
 import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.servlet.HandlerInterceptor;
 import org.springframework.web.servlet.ModelAndView;
 
 import com.linecorp.bot.model.event.CallbackRequest;
-import com.linecorp.bot.servlet.LineBotCallbackException;
-import com.linecorp.bot.servlet.LineBotCallbackRequestParser;
+import com.linecorp.bot.parser.WebhookParseException;
+import com.linecorp.bot.parser.WebhookParser;
 import com.linecorp.bot.spring.boot.annotation.LineBotMessages;
 import com.linecorp.bot.spring.boot.support.LineBotServerArgumentProcessor;
 
+import lombok.AllArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 
 @Component
 @Slf4j
+@AllArgsConstructor(onConstructor = @__(@Autowired))
 public class LineBotServerInterceptor implements HandlerInterceptor {
-    @Autowired
-    private LineBotCallbackRequestParser lineBotCallbackRequestParser;
+    private final WebhookParser webhookParser;
 
     @Override
     public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler)
@@ -58,10 +60,12 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
                 continue;
             }
             try {
-                final CallbackRequest callbackRequest = lineBotCallbackRequestParser.handle(request);
+                final String signatureHeader = request.getHeader(WebhookParser.SIGNATURE_HEADER_NAME);
+                final byte[] payload = StreamUtils.copyToByteArray(request.getInputStream());
+                final CallbackRequest callbackRequest = webhookParser.handle(signatureHeader, payload);
                 LineBotServerArgumentProcessor.setValue(request, callbackRequest);
                 return true;
-            } catch (LineBotCallbackException e) {
+            } catch (WebhookParseException e) {
                 log.info("LINE Bot callback exception: {}", e.getMessage());
                 response.sendError(HttpStatus.BAD_REQUEST.value());
                 try (PrintWriter writer = response.getWriter()) {
diff --git a/line-bot-spring-boot/src/test/java/com/linecorp/bot/spring/boot/interceptor/LineBotServerInterceptorTest.java b/line-bot-spring-boot/src/test/java/com/linecorp/bot/spring/boot/interceptor/LineBotServerInterceptorTest.java
@@ -17,29 +17,50 @@
 package com.linecorp.bot.spring.boot.interceptor;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.springframework.boot.test.context.SpringBootTest;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
 import org.springframework.core.MethodParameter;
 import org.springframework.web.method.HandlerMethod;
 import org.springframework.web.servlet.resource.ResourceHttpRequestHandler;
 
-@SpringBootTest
+import com.linecorp.bot.model.event.CallbackRequest;
+import com.linecorp.bot.parser.WebhookParser;
+
 public class LineBotServerInterceptorTest {
-    LineBotServerInterceptor target = new LineBotServerInterceptor();
+    @Rule
+    public final MockitoRule mockitoRule = MockitoJUnit.rule();
+
+    @InjectMocks
+    LineBotServerInterceptor target;
+
+    @Mock
+    WebhookParser webhookParser;
 
     @Mock
     HttpServletRequest request;
 
     @Mock
     HttpServletResponse response;
 
+    @Before
+    public void setUp() throws Exception {
+        when(webhookParser.handle(anyString(), any()))
+                .thenReturn(CallbackRequest.builder().build());
+    }
+
     @Test
     public void preHandleStaticResourceTest() throws Exception {
         ResourceHttpRequestHandler handler = mock(ResourceHttpRequestHandler.class);

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@`
`19`	`19`	`public class LineBotCallbackException extends Exception {`
`20`	`20`	`private static final long serialVersionUID = -950894346433317253L;`
`21`	`21`
`22`		`- public LineBotCallbackException(String message) {`
`23`		`- super(message);`
	`22`	`+ public LineBotCallbackException(String message, Exception e) {`
	`23`	`+ super(message, e);`
`24`	`24`	`}`
`25`	`25`	`}`