You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/08/18 10:02:29 UTC
[GitHub] [shardingsphere-elasticjob] TeslaCN opened a new pull request #1384: Add new module elasticjob-restful
TeslaCN opened a new pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384
Fixes #1284 .
Changes proposed in this pull request:
- Add new module elasticjob-restful.
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472207023
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/AmbiguousPathPatternException.java
##########
@@ -0,0 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+/**
+ * A path matched more than one path patterns and failed to determine which pattern is more proper.
+ */
+public class AmbiguousPathPatternException extends RuntimeException {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/DefaultMappingContext.java
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import lombok.RequiredArgsConstructor;
+
+/**
+ * Default mapping context.
+ *
+ * @param <T> Type of payload
+ */
+@RequiredArgsConstructor
+public class DefaultMappingContext<T> implements MappingContext<T> {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/ExceptionHandleResult.java
##########
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import lombok.Builder;
+import lombok.Getter;
+
+@Builder
+@Getter
+public class ExceptionHandleResult {
Review comment:
Add final
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] kezhenxu94 commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472594315
##########
File path: elasticjob-infra/elasticjob-restful/pom.xml
##########
@@ -0,0 +1,66 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ Licensed to the Apache Software Foundation (ASF) under one or more
+ ~ contributor license agreements. See the NOTICE file distributed with
+ ~ this work for additional information regarding copyright ownership.
+ ~ The ASF licenses this file to You under the Apache License, Version 2.0
+ ~ (the "License"); you may not use this file except in compliance with
+ ~ the License. You may obtain a copy of the License at
+ ~
+ ~ http://www.apache.org/licenses/LICENSE-2.0
+ ~
+ ~ Unless required by applicable law or agreed to in writing, software
+ ~ distributed under the License is distributed on an "AS IS" BASIS,
+ ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ ~ See the License for the specific language governing permissions and
+ ~ limitations under the License.
+ -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>elasticjob-infra</artifactId>
+ <groupId>org.apache.shardingsphere.elasticjob</groupId>
+ <version>3.0.0-beta-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>elasticjob-restful</artifactId>
+ <name>${project.artifactId}</name>
+
+ <dependencies>
+ <dependency>
+ <groupId>io.netty</groupId>
+ <artifactId>netty-codec-http</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>com.google.code.gson</groupId>
+ <artifactId>gson</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.projectlombok</groupId>
+ <artifactId>lombok</artifactId>
+ <scope>compile</scope>
Review comment:
`provided` is recommended according to the `Lombok`'s doc
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] TeslaCN commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472711931
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/NettyRestfulService.java
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import com.google.common.base.Strings;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.ChannelFuture;
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.socket.nio.NioServerSocketChannel;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.RestfulServiceChannelInitializer;
+
+/**
+ * Implemented {@link RestfulService} via Netty.
+ */
+@Slf4j
+public final class NettyRestfulService implements RestfulService {
+
+ private final NettyRestfulServiceConfiguration configuration;
+
+ private ServerBootstrap serverBootstrap;
+
+ private EventLoopGroup eventLoopGroup;
+
+ public NettyRestfulService(final NettyRestfulServiceConfiguration configuration) {
+ this.configuration = configuration;
+ }
+
+ private void initServerBootstrap() {
+ eventLoopGroup = new NioEventLoopGroup();
+ serverBootstrap = new ServerBootstrap()
+ .group(eventLoopGroup)
Review comment:
I think one group for boss and worker is enough for now. Default threads count can be specified by `-Dio.netty.eventLoopThreads`.
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r473096953
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexUrlPatternMapTest.java
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.apache.shardingsphere.elasticjob.restful.mapping.MappingContext;
+import org.apache.shardingsphere.elasticjob.restful.mapping.RegexUrlPatternMap;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+public class RegexUrlPatternMapTest {
+
+ @Test
+ public void assertRegexUrlPatternMap() {
+ RegexUrlPatternMap<Integer> urlPatternMap = new RegexUrlPatternMap<>();
+ urlPatternMap.put("/app/{jobName}", 1);
+ urlPatternMap.put("/app/list", 2);
+ urlPatternMap.put("/app/{jobName}/disable", 3);
+ urlPatternMap.put("/app/{jobName}/enable", 4);
+ MappingContext<Integer> mappingContext = urlPatternMap.match("/app/myJob");
+ assertNotNull(mappingContext);
+ assertEquals("/app/{jobName}", mappingContext.pattern());
Review comment:
Should use assertThat
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexUrlPatternMapTest.java
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.apache.shardingsphere.elasticjob.restful.mapping.MappingContext;
+import org.apache.shardingsphere.elasticjob.restful.mapping.RegexUrlPatternMap;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+public class RegexUrlPatternMapTest {
+
+ @Test
+ public void assertRegexUrlPatternMap() {
+ RegexUrlPatternMap<Integer> urlPatternMap = new RegexUrlPatternMap<>();
+ urlPatternMap.put("/app/{jobName}", 1);
+ urlPatternMap.put("/app/list", 2);
+ urlPatternMap.put("/app/{jobName}/disable", 3);
+ urlPatternMap.put("/app/{jobName}/enable", 4);
+ MappingContext<Integer> mappingContext = urlPatternMap.match("/app/myJob");
+ assertNotNull(mappingContext);
+ assertEquals("/app/{jobName}", mappingContext.pattern());
+ assertEquals(Integer.valueOf(1), mappingContext.payload());
+ mappingContext = urlPatternMap.match("/app/list");
+ assertNotNull(mappingContext);
+ assertEquals("/app/list", mappingContext.pattern());
+ assertEquals(Integer.valueOf(2), mappingContext.payload());
+ mappingContext = urlPatternMap.match("/job/list");
+ assertNull(mappingContext);
+ }
+
+ @Test
+ public void assertAmbiguous() {
+ RegexUrlPatternMap<Integer> urlPatternMap = new RegexUrlPatternMap<>();
+ urlPatternMap.put("/foo/{bar}/{fooName}/status", 10);
+ urlPatternMap.put("/foo/{bar}/operate/{metrics}", 11);
+ MappingContext<Integer> mappingContext = urlPatternMap.match("/foo/barValue/operate/status");
+ assertNotNull(mappingContext);
+ assertEquals("/foo/{bar}/operate/{metrics}", mappingContext.pattern());
Review comment:
Should use assertThat
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472103540
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/PathMatcher.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import java.util.Map;
+
+public interface PathMatcher {
Review comment:
Miss java doc
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472204659
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/HandlerNotFoundException.java
##########
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import java.text.MessageFormat;
+
+public class HandlerNotFoundException extends RuntimeException {
Review comment:
Add final
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472295022
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/pipeline/NettyRestfulServiceTest.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import com.google.gson.Gson;
+import io.netty.buffer.ByteBufUtil;
+import io.netty.buffer.Unpooled;
+import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpUtil;
+import io.netty.handler.codec.http.HttpVersion;
+import lombok.SneakyThrows;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulService;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulServiceConfiguration;
+import org.apache.shardingsphere.elasticjob.restful.RestfulService;
+import org.apache.shardingsphere.elasticjob.restful.controller.JobController;
+import org.apache.shardingsphere.elasticjob.restful.handler.CustomIllegalStateExceptionHandler;
+import org.apache.shardingsphere.elasticjob.restful.pojo.JobPojo;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertEquals;
+
+public class NettyRestfulServiceTest {
+
+ private static final String HOST = "localhost";
+
+ private static final int PORT = 18080;
+
+ private static RestfulService restfulService;
+
+ @BeforeClass
+ public static void init() {
+ NettyRestfulServiceConfiguration configuration = new NettyRestfulServiceConfiguration(PORT);
+ configuration.addControllerInstance(new JobController());
+ configuration.addExceptionHandler(IllegalStateException.class, new CustomIllegalStateExceptionHandler());
+ restfulService = new NettyRestfulService(configuration);
+ restfulService.startup();
+ }
+
+ @SneakyThrows
+ @Test(timeout = 10000L)
+ public void testRequestNormal() {
+ String cron = "0 * * * * ?";
+ String uri = String.format("/job/myGroup/myJob?cron=%s", URLEncoder.encode(cron, "UTF-8"));
+ String description = "Descriptions about this job.";
+ byte[] body = description.getBytes(StandardCharsets.UTF_8);
+ DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, uri, Unpooled.wrappedBuffer(body));
+ request.headers().set(HttpHeaderNames.CONTENT_TYPE, HttpHeaderValues.TEXT_PLAIN);
+ HttpUtil.setContentLength(request, body.length);
+ HttpClient.request(HOST, PORT, request, httpResponse -> {
+ assertEquals(200, httpResponse.status().code());
+ byte[] bytes = ByteBufUtil.getBytes(httpResponse.content());
+ String json = new String(bytes, StandardCharsets.UTF_8);
+ Gson gson = new Gson();
+ JobPojo jobPojo = gson.fromJson(json, JobPojo.class);
+ assertEquals(cron, jobPojo.getCron());
+ assertEquals("myGroup", jobPojo.getGroup());
+ assertEquals("myJob", jobPojo.getName());
+ assertEquals(description, jobPojo.getDescription());
+ }, 10000L);
+ }
+
+ @Test(timeout = 10000L)
+ public void testCustomExceptionHandler() {
Review comment:
Use assertXXX
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472656310
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/NettyRestfulService.java
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import com.google.common.base.Strings;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.ChannelFuture;
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.socket.nio.NioServerSocketChannel;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.RestfulServiceChannelInitializer;
+
+/**
+ * Implemented {@link RestfulService} via Netty.
+ */
+@Slf4j
+public final class NettyRestfulService implements RestfulService {
+
+ private final NettyRestfulServiceConfiguration configuration;
+
+ private ServerBootstrap serverBootstrap;
+
+ private EventLoopGroup eventLoopGroup;
+
+ public NettyRestfulService(final NettyRestfulServiceConfiguration configuration) {
+ this.configuration = configuration;
+ }
+
+ private void initServerBootstrap() {
+ eventLoopGroup = new NioEventLoopGroup();
+ serverBootstrap = new ServerBootstrap()
+ .group(eventLoopGroup)
Review comment:
Do we need boss and worker group ? worker group size is CPU * 2 + 1 by default ?
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472294526
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/pipeline/HttpRequestDispatcherTest.java
##########
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import com.google.common.collect.Lists;
+import io.netty.channel.embedded.EmbeddedChannel;
+import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpVersion;
+import org.apache.shardingsphere.elasticjob.restful.HandlerNotFoundException;
+import org.apache.shardingsphere.elasticjob.restful.controller.JobController;
+import org.junit.Test;
+
+public class HttpRequestDispatcherTest {
+
+ @Test(expected = HandlerNotFoundException.class)
+ public void testDispatcher() {
Review comment:
Use assertXXX
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/pipeline/NettyRestfulServiceTest.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import com.google.gson.Gson;
+import io.netty.buffer.ByteBufUtil;
+import io.netty.buffer.Unpooled;
+import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpUtil;
+import io.netty.handler.codec.http.HttpVersion;
+import lombok.SneakyThrows;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulService;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulServiceConfiguration;
+import org.apache.shardingsphere.elasticjob.restful.RestfulService;
+import org.apache.shardingsphere.elasticjob.restful.controller.JobController;
+import org.apache.shardingsphere.elasticjob.restful.handler.CustomIllegalStateExceptionHandler;
+import org.apache.shardingsphere.elasticjob.restful.pojo.JobPojo;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertEquals;
+
+public class NettyRestfulServiceTest {
+
+ private static final String HOST = "localhost";
+
+ private static final int PORT = 18080;
+
+ private static RestfulService restfulService;
+
+ @BeforeClass
+ public static void init() {
+ NettyRestfulServiceConfiguration configuration = new NettyRestfulServiceConfiguration(PORT);
+ configuration.addControllerInstance(new JobController());
+ configuration.addExceptionHandler(IllegalStateException.class, new CustomIllegalStateExceptionHandler());
+ restfulService = new NettyRestfulService(configuration);
+ restfulService.startup();
+ }
+
+ @SneakyThrows
+ @Test(timeout = 10000L)
+ public void testRequestNormal() {
Review comment:
Use assertXXX
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/pipeline/NettyRestfulServiceTest.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import com.google.gson.Gson;
+import io.netty.buffer.ByteBufUtil;
+import io.netty.buffer.Unpooled;
+import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpUtil;
+import io.netty.handler.codec.http.HttpVersion;
+import lombok.SneakyThrows;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulService;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulServiceConfiguration;
+import org.apache.shardingsphere.elasticjob.restful.RestfulService;
+import org.apache.shardingsphere.elasticjob.restful.controller.JobController;
+import org.apache.shardingsphere.elasticjob.restful.handler.CustomIllegalStateExceptionHandler;
+import org.apache.shardingsphere.elasticjob.restful.pojo.JobPojo;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertEquals;
+
+public class NettyRestfulServiceTest {
+
+ private static final String HOST = "localhost";
+
+ private static final int PORT = 18080;
+
+ private static RestfulService restfulService;
+
+ @BeforeClass
+ public static void init() {
+ NettyRestfulServiceConfiguration configuration = new NettyRestfulServiceConfiguration(PORT);
+ configuration.addControllerInstance(new JobController());
+ configuration.addExceptionHandler(IllegalStateException.class, new CustomIllegalStateExceptionHandler());
+ restfulService = new NettyRestfulService(configuration);
+ restfulService.startup();
+ }
+
+ @SneakyThrows
+ @Test(timeout = 10000L)
+ public void testRequestNormal() {
+ String cron = "0 * * * * ?";
+ String uri = String.format("/job/myGroup/myJob?cron=%s", URLEncoder.encode(cron, "UTF-8"));
+ String description = "Descriptions about this job.";
+ byte[] body = description.getBytes(StandardCharsets.UTF_8);
+ DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, uri, Unpooled.wrappedBuffer(body));
+ request.headers().set(HttpHeaderNames.CONTENT_TYPE, HttpHeaderValues.TEXT_PLAIN);
+ HttpUtil.setContentLength(request, body.length);
+ HttpClient.request(HOST, PORT, request, httpResponse -> {
+ assertEquals(200, httpResponse.status().code());
+ byte[] bytes = ByteBufUtil.getBytes(httpResponse.content());
+ String json = new String(bytes, StandardCharsets.UTF_8);
+ Gson gson = new Gson();
+ JobPojo jobPojo = gson.fromJson(json, JobPojo.class);
+ assertEquals(cron, jobPojo.getCron());
+ assertEquals("myGroup", jobPojo.getGroup());
+ assertEquals("myJob", jobPojo.getName());
+ assertEquals(description, jobPojo.getDescription());
+ }, 10000L);
+ }
+
+ @Test(timeout = 10000L)
+ public void testCustomExceptionHandler() {
+ DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/job/throw/IllegalState");
+ request.headers().set("Exception-Message", "An illegal state exception message.");
+ HttpClient.request(HOST, PORT, request, httpResponse -> {
+ // Handle by CustomExceptionHandler
+ assertEquals(403, httpResponse.status().code());
+ }, 10000L);
+ }
+
+ @Test(timeout = 10000L)
+ public void testUsingDefaultExceptionHandler() {
+ DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/job/throw/IllegalArgument");
+ request.headers().set("Exception-Message", "An illegal argument exception message.");
+ HttpClient.request(HOST, PORT, request, httpResponse -> {
+ // Handle by CustomExceptionHandler
+ assertEquals(500, httpResponse.status().code());
+ }, 10000L);
+ }
+
+ @Test(timeout = 10000L)
+ public void testReturnStatusCode() {
Review comment:
Use assertXXX
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/pipeline/NettyRestfulServiceTest.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import com.google.gson.Gson;
+import io.netty.buffer.ByteBufUtil;
+import io.netty.buffer.Unpooled;
+import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpUtil;
+import io.netty.handler.codec.http.HttpVersion;
+import lombok.SneakyThrows;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulService;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulServiceConfiguration;
+import org.apache.shardingsphere.elasticjob.restful.RestfulService;
+import org.apache.shardingsphere.elasticjob.restful.controller.JobController;
+import org.apache.shardingsphere.elasticjob.restful.handler.CustomIllegalStateExceptionHandler;
+import org.apache.shardingsphere.elasticjob.restful.pojo.JobPojo;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertEquals;
+
+public class NettyRestfulServiceTest {
+
+ private static final String HOST = "localhost";
+
+ private static final int PORT = 18080;
+
+ private static RestfulService restfulService;
+
+ @BeforeClass
+ public static void init() {
+ NettyRestfulServiceConfiguration configuration = new NettyRestfulServiceConfiguration(PORT);
+ configuration.addControllerInstance(new JobController());
+ configuration.addExceptionHandler(IllegalStateException.class, new CustomIllegalStateExceptionHandler());
+ restfulService = new NettyRestfulService(configuration);
+ restfulService.startup();
+ }
+
+ @SneakyThrows
+ @Test(timeout = 10000L)
+ public void testRequestNormal() {
+ String cron = "0 * * * * ?";
+ String uri = String.format("/job/myGroup/myJob?cron=%s", URLEncoder.encode(cron, "UTF-8"));
+ String description = "Descriptions about this job.";
+ byte[] body = description.getBytes(StandardCharsets.UTF_8);
+ DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, uri, Unpooled.wrappedBuffer(body));
+ request.headers().set(HttpHeaderNames.CONTENT_TYPE, HttpHeaderValues.TEXT_PLAIN);
+ HttpUtil.setContentLength(request, body.length);
+ HttpClient.request(HOST, PORT, request, httpResponse -> {
+ assertEquals(200, httpResponse.status().code());
+ byte[] bytes = ByteBufUtil.getBytes(httpResponse.content());
+ String json = new String(bytes, StandardCharsets.UTF_8);
+ Gson gson = new Gson();
+ JobPojo jobPojo = gson.fromJson(json, JobPojo.class);
+ assertEquals(cron, jobPojo.getCron());
+ assertEquals("myGroup", jobPojo.getGroup());
+ assertEquals("myJob", jobPojo.getName());
+ assertEquals(description, jobPojo.getDescription());
+ }, 10000L);
+ }
+
+ @Test(timeout = 10000L)
+ public void testCustomExceptionHandler() {
+ DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/job/throw/IllegalState");
+ request.headers().set("Exception-Message", "An illegal state exception message.");
+ HttpClient.request(HOST, PORT, request, httpResponse -> {
+ // Handle by CustomExceptionHandler
+ assertEquals(403, httpResponse.status().code());
+ }, 10000L);
+ }
+
+ @Test(timeout = 10000L)
+ public void testUsingDefaultExceptionHandler() {
Review comment:
Use assertXXX
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472205219
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/RegexUrlPatternMap.java
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import com.google.common.base.Preconditions;
+
+import java.text.MessageFormat;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.regex.Pattern;
+
+/**
+ * Implemented {@link UrlPatternMap} by regular expression.
+ *
+ * @param <V> Type of payload
+ */
+public class RegexUrlPatternMap<V> implements UrlPatternMap<V> {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/RestfulServiceChannelInitializer.java
##########
@@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import io.netty.channel.Channel;
+import io.netty.channel.ChannelInitializer;
+import io.netty.channel.ChannelPipeline;
+import io.netty.handler.codec.http.HttpObjectAggregator;
+import io.netty.handler.codec.http.HttpServerCodec;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.ExceptionHandling;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.HandleMethodExecutor;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.HandlerParameterDecoder;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.HttpRequestDispatcher;
+
+/**
+ * Initialize channel pipeline.
+ */
+public class RestfulServiceChannelInitializer extends ChannelInitializer<Channel> {
Review comment:
Add final
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- merged pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- merged pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472103752
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/RegexPathMatcher.java
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+public class RegexPathMatcher implements PathMatcher {
Review comment:
Add final to class
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472205600
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/deserializer/impl/JsonRequestBodyDeserializer.java
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.deserializer.impl;
+
+import com.google.gson.Gson;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import org.apache.shardingsphere.elasticjob.restful.deserializer.RequestBodyDeserializer;
+
+import java.nio.charset.StandardCharsets;
+
+/**
+ * Deserializer for <code>application/json</code>.
+ */
+public class JsonRequestBodyDeserializer implements RequestBodyDeserializer {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/deserializer/impl/TextPlainRequestBodyDeserializer.java
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.deserializer.impl;
+
+import io.netty.handler.codec.http.HttpHeaderValues;
+import org.apache.shardingsphere.elasticjob.restful.deserializer.RequestBodyDeserializer;
+
+import java.nio.charset.StandardCharsets;
+import java.text.MessageFormat;
+
+/**
+ * Deserializer for <code>text/plain</code>.
+ */
+public class TextPlainRequestBodyDeserializer implements RequestBodyDeserializer {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/pipeline/ExceptionHandling.java
##########
@@ -0,0 +1,102 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import io.netty.buffer.Unpooled;
+import io.netty.channel.ChannelHandler.Sharable;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.handler.codec.http.DefaultFullHttpResponse;
+import io.netty.handler.codec.http.FullHttpResponse;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.netty.handler.codec.http.HttpUtil;
+import io.netty.handler.codec.http.HttpVersion;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.elasticjob.restful.ExceptionHandleResult;
+import org.apache.shardingsphere.elasticjob.restful.ExceptionHandler;
+import org.apache.shardingsphere.elasticjob.restful.Http;
+import org.apache.shardingsphere.elasticjob.restful.serializer.ResponseBodySerializer;
+import org.apache.shardingsphere.elasticjob.restful.serializer.ResponseBodySerializerFactory;
+
+import java.util.Map;
+
+/**
+ * Catch exceptions and look for a ExceptionHandler.
+ */
+@Sharable
+public class ExceptionHandling extends ChannelInboundHandlerAdapter {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/pipeline/HandleMethodExecutor.java
##########
@@ -0,0 +1,76 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import io.netty.buffer.Unpooled;
+import io.netty.channel.ChannelHandler.Sharable;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.handler.codec.http.DefaultFullHttpResponse;
+import io.netty.handler.codec.http.FullHttpResponse;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.netty.handler.codec.http.HttpUtil;
+import io.netty.handler.codec.http.HttpVersion;
+import org.apache.shardingsphere.elasticjob.restful.HandleContext;
+import org.apache.shardingsphere.elasticjob.restful.Handler;
+import org.apache.shardingsphere.elasticjob.restful.serializer.ResponseBodySerializer;
+import org.apache.shardingsphere.elasticjob.restful.serializer.ResponseBodySerializerFactory;
+
+import java.lang.reflect.InvocationTargetException;
+
+/**
+ * The handler which actually executes handle method and creates HTTP response for responding.
+ * If an exception occurred when executing handle method, this handler would pass it to Handler named {@link ExceptionHandling}.
+ */
+@Sharable
+public class HandleMethodExecutor extends ChannelInboundHandlerAdapter {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/pipeline/HandlerParameterDecoder.java
##########
@@ -0,0 +1,157 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import com.google.common.base.Preconditions;
+import io.netty.buffer.ByteBufUtil;
+import io.netty.channel.ChannelHandler.Sharable;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.handler.codec.UnsupportedMessageTypeException;
+import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.HttpUtil;
+import io.netty.handler.codec.http.QueryStringDecoder;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.elasticjob.restful.HandleContext;
+import org.apache.shardingsphere.elasticjob.restful.Handler;
+import org.apache.shardingsphere.elasticjob.restful.HandlerParameter;
+import org.apache.shardingsphere.elasticjob.restful.Http;
+import org.apache.shardingsphere.elasticjob.restful.MappingContext;
+import org.apache.shardingsphere.elasticjob.restful.PathMatcher;
+import org.apache.shardingsphere.elasticjob.restful.RegexPathMatcher;
+import org.apache.shardingsphere.elasticjob.restful.deserializer.RequestBodyDeserializer;
+import org.apache.shardingsphere.elasticjob.restful.deserializer.RequestBodyDeserializerFactory;
+
+import java.text.MessageFormat;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * This handler is used for preparing parameters before executing handle method.
+ * It prepares arguments declared by {@link org.apache.shardingsphere.elasticjob.restful.annotation.Param}
+ * and {@link org.apache.shardingsphere.elasticjob.restful.annotation.RequestBody}, and deserializes arguments to declared type.
+ */
+@Slf4j
+@Sharable
+public class HandlerParameterDecoder extends ChannelInboundHandlerAdapter {
Review comment:
Add final
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472204783
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/HandlerParameter.java
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+/**
+ * Describe parameters of a handle method.
+ */
+@RequiredArgsConstructor
+@Getter
+public class HandlerParameter {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/Http.java
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import lombok.AccessLevel;
+import lombok.NoArgsConstructor;
+
+/**
+ * Constants for HTTP.
+ */
+@NoArgsConstructor(access = AccessLevel.PRIVATE)
+public class Http {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/NettyRestfulService.java
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import com.google.common.base.Strings;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.ChannelFuture;
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.socket.nio.NioServerSocketChannel;
+import lombok.extern.slf4j.Slf4j;
+
+/**
+ * Implemented {@link RestfulService} via Netty.
+ */
+@Slf4j
+public class NettyRestfulService implements RestfulService {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/NettyRestfulServiceConfiguration.java
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import com.google.common.base.Preconditions;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import lombok.Setter;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Configuration for {@link NettyRestfulService}.
+ */
+@Getter
+@RequiredArgsConstructor
+public class NettyRestfulServiceConfiguration {
Review comment:
Add final
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- edited a comment on pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- edited a comment on pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#issuecomment-675567271
Another suggestion is to add more packages to split the classes. below is just a demo.
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472297922
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexPathMatcherTest.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class RegexPathMatcherTest {
+
+ @Test
+ public void testCaptureTemplate() {
+ PathMatcher pathMatcher = new RegexPathMatcher();
+ Map<String, String> variables = pathMatcher.captureVariables("/app/{jobName}/disable/{until}/done", "/app/myJob/disable/20201231/done?name=some_name&value=some_value");
+ assertFalse(variables.isEmpty());
+ assertEquals(2, variables.size());
+ assertEquals("myJob", variables.get("jobName"));
+ assertEquals("20201231", variables.get("until"));
+ assertNull(variables.get("app"));
+ }
+
+ @Test
+ public void testCapturePatternWithoutTemplate() {
+ PathMatcher pathMatcher = new RegexPathMatcher();
+ Map<String, String> variables = pathMatcher.captureVariables("/app", "/app");
+ assertTrue(variables.isEmpty());
+ }
+
+ @Test
+ public void testMatch() {
Review comment:
Use assertXXX
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexPathMatcherTest.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class RegexPathMatcherTest {
+
+ @Test
+ public void testCaptureTemplate() {
+ PathMatcher pathMatcher = new RegexPathMatcher();
+ Map<String, String> variables = pathMatcher.captureVariables("/app/{jobName}/disable/{until}/done", "/app/myJob/disable/20201231/done?name=some_name&value=some_value");
+ assertFalse(variables.isEmpty());
+ assertEquals(2, variables.size());
+ assertEquals("myJob", variables.get("jobName"));
+ assertEquals("20201231", variables.get("until"));
+ assertNull(variables.get("app"));
+ }
+
+ @Test
+ public void testCapturePatternWithoutTemplate() {
Review comment:
Use assertXXX
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexPathMatcherTest.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class RegexPathMatcherTest {
+
+ @Test
+ public void testCaptureTemplate() {
Review comment:
Use assertXXX
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472649795
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/NettyRestfulService.java
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import com.google.common.base.Strings;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.ChannelFuture;
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.socket.nio.NioServerSocketChannel;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.RestfulServiceChannelInitializer;
+
+/**
+ * Implemented {@link RestfulService} via Netty.
+ */
+@Slf4j
+public final class NettyRestfulService implements RestfulService {
+
+ private final NettyRestfulServiceConfiguration configuration;
+
+ private ServerBootstrap serverBootstrap;
+
+ private EventLoopGroup eventLoopGroup;
+
+ public NettyRestfulService(final NettyRestfulServiceConfiguration configuration) {
+ this.configuration = configuration;
+ }
+
+ private void initServerBootstrap() {
+ eventLoopGroup = new NioEventLoopGroup();
+ serverBootstrap = new ServerBootstrap()
+ .group(eventLoopGroup)
+ .channel(NioServerSocketChannel.class)
+ .childHandler(new RestfulServiceChannelInitializer(configuration));
+ }
+
+ @Override
+ public void startup() {
+ initServerBootstrap();
+ ChannelFuture channelFuture;
+ if (!Strings.isNullOrEmpty(configuration.getHost())) {
+ channelFuture = serverBootstrap.bind(configuration.getHost(), configuration.getPort());
+ } else {
+ channelFuture = serverBootstrap.bind(configuration.getPort());
+ }
+ channelFuture.addListener(future -> {
+ if (future.isSuccess()) {
+ log.info("Restful Service started on port {}.", configuration.getPort());
+ } else {
+ log.error("", future.cause());
Review comment:
Give more error info instead of empty seems more friendly?
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472298272
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexPathMatcherTest.java
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class RegexPathMatcherTest {
+
+ @Test
+ public void testCaptureTemplate() {
+ PathMatcher pathMatcher = new RegexPathMatcher();
+ Map<String, String> variables = pathMatcher.captureVariables("/app/{jobName}/disable/{until}/done", "/app/myJob/disable/20201231/done?name=some_name&value=some_value");
+ assertFalse(variables.isEmpty());
+ assertEquals(2, variables.size());
+ assertEquals("myJob", variables.get("jobName"));
+ assertEquals("20201231", variables.get("until"));
+ assertNull(variables.get("app"));
+ }
+
+ @Test
+ public void testCapturePatternWithoutTemplate() {
+ PathMatcher pathMatcher = new RegexPathMatcher();
+ Map<String, String> variables = pathMatcher.captureVariables("/app", "/app");
+ assertTrue(variables.isEmpty());
+ }
+
+ @Test
+ public void testMatch() {
+ PathMatcher pathMatcher = new RegexPathMatcher();
+ assertTrue(pathMatcher.matches("/app/{jobName}", "/app/myJob"));
+ }
+
+ @Test
+ public void testValidatePathPattern() {
Review comment:
Seems empty method, need to be deleted?
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472297740
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexUrlPatternMapTest.java
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+public class RegexUrlPatternMapTest {
+
+ @Test
+ public void testRegexUrlPatternMap() {
+ RegexUrlPatternMap<Integer> urlPatternMap = new RegexUrlPatternMap<>();
+ urlPatternMap.put("/app/{jobName}", 1);
+ urlPatternMap.put("/app/list", 2);
+ urlPatternMap.put("/app/{jobName}/disable", 3);
+ urlPatternMap.put("/app/{jobName}/enable", 4);
+ MappingContext<Integer> mappingContext = urlPatternMap.match("/app/myJob");
+ assertNotNull(mappingContext);
+ assertEquals("/app/{jobName}", mappingContext.pattern());
+ assertEquals(Integer.valueOf(1), mappingContext.payload());
+ mappingContext = urlPatternMap.match("/app/list");
+ assertNotNull(mappingContext);
+ assertEquals("/app/list", mappingContext.pattern());
+ assertEquals(Integer.valueOf(2), mappingContext.payload());
+ mappingContext = urlPatternMap.match("/job/list");
+ assertNull(mappingContext);
+ }
+
+ @Test
+ public void testAmbiguous() {
+ RegexUrlPatternMap<Integer> urlPatternMap = new RegexUrlPatternMap<>();
+ urlPatternMap.put("/foo/{bar}/{fooName}/status", 10);
+ urlPatternMap.put("/foo/{bar}/operate/{metrics}", 11);
+ MappingContext<Integer> mappingContext = urlPatternMap.match("/foo/barValue/operate/status");
+ assertNotNull(mappingContext);
+ assertEquals("/foo/{bar}/operate/{metrics}", mappingContext.pattern());
+ assertEquals(Integer.valueOf(11), mappingContext.payload());
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testDuplicate() {
Review comment:
Use assertXXX
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexUrlPatternMapTest.java
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+public class RegexUrlPatternMapTest {
+
+ @Test
+ public void testRegexUrlPatternMap() {
+ RegexUrlPatternMap<Integer> urlPatternMap = new RegexUrlPatternMap<>();
+ urlPatternMap.put("/app/{jobName}", 1);
+ urlPatternMap.put("/app/list", 2);
+ urlPatternMap.put("/app/{jobName}/disable", 3);
+ urlPatternMap.put("/app/{jobName}/enable", 4);
+ MappingContext<Integer> mappingContext = urlPatternMap.match("/app/myJob");
+ assertNotNull(mappingContext);
+ assertEquals("/app/{jobName}", mappingContext.pattern());
+ assertEquals(Integer.valueOf(1), mappingContext.payload());
+ mappingContext = urlPatternMap.match("/app/list");
+ assertNotNull(mappingContext);
+ assertEquals("/app/list", mappingContext.pattern());
+ assertEquals(Integer.valueOf(2), mappingContext.payload());
+ mappingContext = urlPatternMap.match("/job/list");
+ assertNull(mappingContext);
+ }
+
+ @Test
+ public void testAmbiguous() {
Review comment:
Use assertXXX
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexUrlPatternMapTest.java
##########
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+public class RegexUrlPatternMapTest {
+
+ @Test
+ public void testRegexUrlPatternMap() {
Review comment:
Use assertXXX
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#issuecomment-675567271
Another suggestion is to add more packages to split the classes.
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472291572
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/annotation/Mapping.java
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Declare what HTTP method and path is used to invoke the handler.
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Mapping {
+
+ /**
+ * Http method.
+ *
+ * @return Http method
+ */
+ String method();
+
+ /**
+ * Path pattern of this handler.
+ * Such as <code>/app/{jobName}/enable</code>.
+ *
+ * @return Path pattern
+ */
+ String pattern();
Review comment:
If names path, it's the same with springmvc, more friendly and familiar to users.
@terrymanu please give some ideas.
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472206189
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/pipeline/HttpRequestDispatcher.java
##########
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import io.netty.channel.ChannelHandler.Sharable;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.HttpMethod;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.elasticjob.restful.HandleContext;
+import org.apache.shardingsphere.elasticjob.restful.Handler;
+import org.apache.shardingsphere.elasticjob.restful.HandlerMappingRegistry;
+import org.apache.shardingsphere.elasticjob.restful.HandlerNotFoundException;
+import org.apache.shardingsphere.elasticjob.restful.MappingContext;
+import org.apache.shardingsphere.elasticjob.restful.RestfulController;
+import org.apache.shardingsphere.elasticjob.restful.annotation.ContextPath;
+import org.apache.shardingsphere.elasticjob.restful.annotation.Mapping;
+
+import java.lang.reflect.Method;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * If a HTTP request reached, HttpRequestDispatcher would lookup a proper Handler for the request.
+ * Assemble a {@link HandleContext} with HTTP request and {@link MappingContext}, then pass it to the next in-bound handler.
+ */
+@Sharable
+@Slf4j
+public class HttpRequestDispatcher extends ChannelInboundHandlerAdapter {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/serializer/impl/JsonResponseBodySerializer.java
##########
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.serializer.impl;
+
+import com.google.gson.Gson;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import org.apache.shardingsphere.elasticjob.restful.serializer.ResponseBodySerializer;
+
+import java.nio.charset.StandardCharsets;
+
+/**
+ * Serializer for <code>application/json</code>.
+ */
+public class JsonResponseBodySerializer implements ResponseBodySerializer {
Review comment:
Add final
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472647842
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/handler/Handler.java
##########
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.handler;
+
+import lombok.Getter;
+import org.apache.shardingsphere.elasticjob.restful.Http;
+import org.apache.shardingsphere.elasticjob.restful.annotation.ParamSource;
+import org.apache.shardingsphere.elasticjob.restful.annotation.Param;
+import org.apache.shardingsphere.elasticjob.restful.annotation.RequestBody;
+import org.apache.shardingsphere.elasticjob.restful.annotation.Returning;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Parameter;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * Handle holds a handle method and an instance for method invoking.
+ * Describes parameters requirements of handle method.
+ */
+public final class Handler {
+
+ private final Object instance;
+
+ private final Method handleMethod;
+
+ @Getter
+ private final List<HandlerParameter> handlerParameters;
+
+ /**
+ * HTTP status code to return.
+ */
+ @Getter
+ private final int httpStatusCode;
+
+ /**
+ * Content type to producing.
+ */
+ @Getter
+ private final String producing;
+
+ public Handler(final Object instance, final Method handleMethod) {
+ this.instance = instance;
+ this.handleMethod = handleMethod;
+ this.handlerParameters = parseHandleMethodParameter();
+ this.httpStatusCode = parseReturning();
+ this.producing = parseProducing();
+ }
+
+ /**
+ * Execute handle method with required arguments.
+ *
+ * @param args Required arguments
+ * @return Method invoke result
+ * @throws InvocationTargetException Wraps exception thrown by invoked method
+ * @throws IllegalAccessException Handle method is not accessible
+ */
+ public Object execute(final Object... args) throws InvocationTargetException, IllegalAccessException {
+ return handleMethod.invoke(instance, args);
+ }
+
+ private List<HandlerParameter> parseHandleMethodParameter() {
+ List<HandlerParameter> params = new ArrayList<>();
Review comment:
LinkedList is prefered
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] terrymanu commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r473118966
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/annotation/Mapping.java
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Declare what HTTP method and path is used to invoke the handler.
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface Mapping {
+
+ /**
+ * Http method.
+ *
+ * @return Http method
+ */
+ String method();
+
+ /**
+ * Path pattern of this handler.
+ * Such as <code>/app/{jobName}/enable</code>.
+ *
+ * @return Path pattern
+ */
+ String pattern();
Review comment:
I think so, it is better to keep same with famous framework, which can let user easy to understand.
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/NettyRestfulService.java
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import com.google.common.base.Strings;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.ChannelFuture;
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.socket.nio.NioServerSocketChannel;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.RestfulServiceChannelInitializer;
+
+/**
+ * Implemented {@link RestfulService} via Netty.
+ */
+@Slf4j
+public final class NettyRestfulService implements RestfulService {
+
+ private final NettyRestfulServiceConfiguration configuration;
+
+ private ServerBootstrap serverBootstrap;
+
+ private EventLoopGroup eventLoopGroup;
+
+ public NettyRestfulService(final NettyRestfulServiceConfiguration configuration) {
+ this.configuration = configuration;
+ }
+
+ private void initServerBootstrap() {
+ eventLoopGroup = new NioEventLoopGroup();
+ serverBootstrap = new ServerBootstrap()
+ .group(eventLoopGroup)
Review comment:
1. Generally, separate boss and worker group is best practice.
If no special reason, it is better separate them.
2. I prefer add default worker group size as `CPU * 2 + 1`, which can simplify user configuration and suitable for most scenario.
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472828584
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/NettyRestfulService.java
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import com.google.common.base.Strings;
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.ChannelFuture;
+import io.netty.channel.EventLoopGroup;
+import io.netty.channel.nio.NioEventLoopGroup;
+import io.netty.channel.socket.nio.NioServerSocketChannel;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.elasticjob.restful.pipeline.RestfulServiceChannelInitializer;
+
+/**
+ * Implemented {@link RestfulService} via Netty.
+ */
+@Slf4j
+public final class NettyRestfulService implements RestfulService {
+
+ private final NettyRestfulServiceConfiguration configuration;
+
+ private ServerBootstrap serverBootstrap;
+
+ private EventLoopGroup eventLoopGroup;
+
+ public NettyRestfulService(final NettyRestfulServiceConfiguration configuration) {
+ this.configuration = configuration;
+ }
+
+ private void initServerBootstrap() {
+ eventLoopGroup = new NioEventLoopGroup();
+ serverBootstrap = new ServerBootstrap()
+ .group(eventLoopGroup)
Review comment:
@terrymanu please help to give more ideas.
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r473098179
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/pipeline/NettyRestfulServiceTest.java
##########
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful.pipeline;
+
+import com.google.gson.Gson;
+import io.netty.buffer.ByteBufUtil;
+import io.netty.buffer.Unpooled;
+import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.HttpHeaderNames;
+import io.netty.handler.codec.http.HttpHeaderValues;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpUtil;
+import io.netty.handler.codec.http.HttpVersion;
+import lombok.SneakyThrows;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulService;
+import org.apache.shardingsphere.elasticjob.restful.NettyRestfulServiceConfiguration;
+import org.apache.shardingsphere.elasticjob.restful.RestfulService;
+import org.apache.shardingsphere.elasticjob.restful.controller.JobController;
+import org.apache.shardingsphere.elasticjob.restful.handler.CustomIllegalStateExceptionHandler;
+import org.apache.shardingsphere.elasticjob.restful.pojo.JobPojo;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+
+import static org.junit.Assert.assertEquals;
+
+public class NettyRestfulServiceTest {
+
+ private static final String HOST = "localhost";
+
+ private static final int PORT = 18080;
+
+ private static RestfulService restfulService;
+
+ @BeforeClass
+ public static void init() {
+ NettyRestfulServiceConfiguration configuration = new NettyRestfulServiceConfiguration(PORT);
+ configuration.setHost(HOST);
+ configuration.addControllerInstance(new JobController());
+ configuration.addExceptionHandler(IllegalStateException.class, new CustomIllegalStateExceptionHandler());
+ restfulService = new NettyRestfulService(configuration);
+ restfulService.startup();
+ }
+
+ @SneakyThrows
+ @Test(timeout = 10000L)
+ public void assertRequestWithParameters() {
+ String cron = "0 * * * * ?";
+ String uri = String.format("/job/myGroup/myJob?cron=%s", URLEncoder.encode(cron, "UTF-8"));
+ String description = "Descriptions about this job.";
+ byte[] body = description.getBytes(StandardCharsets.UTF_8);
+ DefaultFullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, uri, Unpooled.wrappedBuffer(body));
+ request.headers().set(HttpHeaderNames.CONTENT_TYPE, HttpHeaderValues.TEXT_PLAIN);
+ HttpUtil.setContentLength(request, body.length);
+ HttpClient.request(HOST, PORT, request, httpResponse -> {
+ assertEquals(200, httpResponse.status().code());
+ byte[] bytes = ByteBufUtil.getBytes(httpResponse.content());
+ String json = new String(bytes, StandardCharsets.UTF_8);
+ Gson gson = new Gson();
+ JobPojo jobPojo = gson.fromJson(json, JobPojo.class);
+ assertEquals(cron, jobPojo.getCron());
Review comment:
Should use assertThat
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r472207299
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/HandleContext.java
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import io.netty.handler.codec.http.FullHttpRequest;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import lombok.Setter;
+
+/**
+ * HandleContext will hold a instance of HTTP request, {@link MappingContext} and arguments for handle method invoking.
+ *
+ * @param <T> Type of MappingContext
+ */
+@RequiredArgsConstructor
+@Getter
+@Setter
+public class HandleContext<T> {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/Handler.java
##########
@@ -0,0 +1,106 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import lombok.Getter;
+import org.apache.shardingsphere.elasticjob.restful.annotation.Param;
+import org.apache.shardingsphere.elasticjob.restful.annotation.RequestBody;
+import org.apache.shardingsphere.elasticjob.restful.annotation.Returning;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Parameter;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+
+/**
+ * Handle holds a handle method and an instance for method invoking.
+ * Describes parameters requirements of handle method.
+ */
+public class Handler {
Review comment:
Add final
##########
File path: elasticjob-infra/elasticjob-restful/src/main/java/org/apache/shardingsphere/elasticjob/restful/HandlerMappingRegistry.java
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpRequest;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * HandlerMappingRegistry stores mappings of handlers.
+ * Search a proper {@link MappingContext} by HTTP method and request URI.
+ */
+public class HandlerMappingRegistry {
Review comment:
Add final
----------------------------------------------------------------
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
[GitHub] [shardingsphere-elasticjob] Technoboy- commented on a change in pull request #1384: Add new module elasticjob-restful
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #1384:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1384#discussion_r473096747
##########
File path: elasticjob-infra/elasticjob-restful/src/test/java/org/apache/shardingsphere/elasticjob/restful/RegexPathMatcherTest.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.elasticjob.restful;
+
+import org.apache.shardingsphere.elasticjob.restful.mapping.PathMatcher;
+import org.apache.shardingsphere.elasticjob.restful.mapping.RegexPathMatcher;
+import org.junit.Test;
+
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+public class RegexPathMatcherTest {
+
+ @Test
+ public void assertCaptureTemplate() {
+ PathMatcher pathMatcher = new RegexPathMatcher();
+ Map<String, String> variables = pathMatcher.captureVariables("/app/{jobName}/disable/{until}/done", "/app/myJob/disable/20201231/done?name=some_name&value=some_value");
+ assertFalse(variables.isEmpty());
+ assertEquals(2, variables.size());
Review comment:
Should use assertThat
----------------------------------------------------------------
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