You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2018/12/17 13:56:27 UTC

[GitHub] dongeforever closed pull request #606: Handle TODO

dongeforever closed pull request #606: Handle TODO
URL: https://github.com/apache/rocketmq/pull/606
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java b/acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java
index 2fa38b15b..c608f05da 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java
@@ -77,7 +77,8 @@ public static byte parsePermFromString(String permString) {
         }
     }
 
-    public static void parseResourcePerms(PlainAccessResource plainAccessResource, Boolean isTopic, List<String> resources) {
+    public static void parseResourcePerms(PlainAccessResource plainAccessResource, Boolean isTopic,
+        List<String> resources) {
         if (resources == null || resources.isEmpty()) {
             return;
         }
diff --git a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java
index 932a7a94f..9017bf22e 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java
@@ -76,6 +76,7 @@ public static String getGroupFromRetryTopic(String retryTopic) {
         }
         return retryTopic.substring(MixAll.RETRY_GROUP_TOPIC_PREFIX.length());
     }
+
     public static String getRetryTopic(String group) {
         if (group == null) {
             return null;
diff --git a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java
index 36f652211..01161d0cf 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java
@@ -45,12 +45,12 @@
 
     private static final InternalLogger log = InternalLoggerFactory.getLogger(LoggerName.ACL_PLUG_LOGGER_NAME);
 
+    private static final String DEFAULT_PLAIN_ACL_FILE = "/conf/plain_acl.yml";
 
     private String fileHome = System.getProperty(MixAll.ROCKETMQ_HOME_PROPERTY,
         System.getenv(MixAll.ROCKETMQ_HOME_ENV));
 
-    //TODO  rename transport to plain_acl.yml
-    private String fileName = System.getProperty("rocketmq.acl.plain.file", "/conf/transport.yml");
+    private String fileName = System.getProperty("rocketmq.acl.plain.file", DEFAULT_PLAIN_ACL_FILE);
 
     private Map<String/** AccessKey **/, PlainAccessResource> plainAccessResourceMap = new HashMap<>();
 
@@ -61,7 +61,6 @@
     private boolean isWatchStart;
 
     public PlainPermissionLoader() {
-        //TODO test what will happen if initialize failed
         initialize();
         watch();
     }
@@ -97,9 +96,15 @@ private void watch() {
             log.warn("Watch need jdk equal or greater than 1.7, current version is {}", str[1]);
             return;
         }
+
         try {
+            int fileIndex = fileName.lastIndexOf("/") + 1;
+            String watchDirectory = fileName.substring(0, fileIndex);
+            final String watchFileName = fileName.substring(fileIndex);
+            log.info("watch directory is {} , watch directory file name is {} ", fileHome + watchDirectory, watchFileName);
+
             final WatchService watcher = FileSystems.getDefault().newWatchService();
-            Path p = Paths.get(fileHome + "/conf/");
+            Path p = Paths.get(fileHome + watchDirectory);
             p.register(watcher, StandardWatchEventKinds.ENTRY_MODIFY, StandardWatchEventKinds.ENTRY_CREATE);
             ServiceThread watcherServcie = new ServiceThread() {
 
@@ -109,11 +114,10 @@ public void run() {
                             WatchKey watchKey = watcher.take();
                             List<WatchEvent<?>> watchEvents = watchKey.pollEvents();
                             for (WatchEvent<?> event : watchEvents) {
-                                //TODO use variable instead of raw text
-                                if ("transport.yml".equals(event.context().toString())
+                                if (watchFileName.equals(event.context().toString())
                                     && (StandardWatchEventKinds.ENTRY_MODIFY.equals(event.kind())
                                     || StandardWatchEventKinds.ENTRY_CREATE.equals(event.kind()))) {
-                                    log.info("transprot.yml make a difference  change is : ", event.toString());
+                                    log.info("{} make a difference  change is : {}", watchFileName, event.toString());
                                     PlainPermissionLoader.this.clearPermissionInfo();
                                     initialize();
                                 }
@@ -126,6 +130,7 @@ public void run() {
                         }
                     }
                 }
+
                 @Override
                 public String getServiceName() {
                     return "AclWatcherService";
@@ -240,7 +245,6 @@ public void validate(PlainAccessResource plainAccessResource) {
             return;
         }
 
-
         //Step 3, check the signature
         String signature = AclUtils.calSignature(plainAccessResource.getContent(), ownedAccess.getSecretKey());
         if (!signature.equals(plainAccessResource.getSignature())) {
diff --git a/acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java b/acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java
index 679e846d1..b82d79388 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java
@@ -32,8 +32,10 @@ public RemoteAddressStrategy getRemoteAddressStrategy(PlainAccessResource plainA
     }
 
     public RemoteAddressStrategy getRemoteAddressStrategy(String remoteAddr) {
-        //TODO if the white addr is not configured, should reject it.
-        if (StringUtils.isBlank(remoteAddr) || "*".equals(remoteAddr)) {
+        if (StringUtils.isBlank(remoteAddr)) {
+            throw new AclException("Must fill in the white list address");
+        }
+        if ("*".equals(remoteAddr)) {
             return NULL_NET_ADDRESS_STRATEGY;
         }
         if (remoteAddr.endsWith("}")) {
diff --git a/acl/src/test/java/org/apache/rocketmq/acl/common/AclUtilsTest.java b/acl/src/test/java/org/apache/rocketmq/acl/common/AclUtilsTest.java
index 36af31f91..72bcda6bb 100644
--- a/acl/src/test/java/org/apache/rocketmq/acl/common/AclUtilsTest.java
+++ b/acl/src/test/java/org/apache/rocketmq/acl/common/AclUtilsTest.java
@@ -129,13 +129,13 @@ public void isMinusTest() {
     @Test
     public void getYamlDataObjectTest() {
 
-        Map<String, Object> map = AclUtils.getYamlDataObject("src/test/resources/conf/transport.yml", Map.class);
+        Map<String, Object> map = AclUtils.getYamlDataObject("src/test/resources/conf/plain_acl.yml", Map.class);
         Assert.assertFalse(map.isEmpty());
     }
 
     @Test(expected = Exception.class)
     public void getYamlDataObjectExceptionTest() {
 
-        AclUtils.getYamlDataObject("transport.yml", Map.class);
+        AclUtils.getYamlDataObject("plain_acl.yml", Map.class);
     }
 }
diff --git a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java b/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java
index de9b45dc3..4f5ae5b52 100644
--- a/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java
+++ b/acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java
@@ -63,7 +63,7 @@ public void init() throws NoSuchFieldException, SecurityException, IOException {
 
         System.setProperty("java.version", "1.6.11");
         System.setProperty("rocketmq.home.dir", "src/test/resources");
-        System.setProperty("romcketmq.acl.plain.fileName", "/conf/transport.yml");
+        System.setProperty("romcketmq.acl.plain.fileName", "/conf/plain_acl.yml");
         plainPermissionLoader = new PlainPermissionLoader();
 
     }
@@ -154,16 +154,16 @@ public void checkPermAdmin() {
     public void checkPerm() {
 
         PlainAccessResource plainAccessResource = new PlainAccessResource();
-        plainAccessResource.addResourceAndPerm("pub", Permission.PUB);
-        plainPermissionLoader.checkPerm(PUBPlainAccessResource, plainAccessResource);
-        plainAccessResource.addResourceAndPerm("sub", Permission.SUB);
-        plainPermissionLoader.checkPerm(ANYPlainAccessResource, plainAccessResource);
+        plainAccessResource.addResourceAndPerm("topicA", Permission.PUB);
+        plainPermissionLoader.checkPerm(plainAccessResource, PUBPlainAccessResource);
+        plainAccessResource.addResourceAndPerm("topicB", Permission.SUB);
+        plainPermissionLoader.checkPerm(plainAccessResource, ANYPlainAccessResource);
 
         plainAccessResource = new PlainAccessResource();
-        plainAccessResource.addResourceAndPerm("sub", Permission.SUB);
-        plainPermissionLoader.checkPerm(SUBPlainAccessResource, plainAccessResource);
-        plainAccessResource.addResourceAndPerm("pub", Permission.PUB);
-        plainPermissionLoader.checkPerm(ANYPlainAccessResource, plainAccessResource);
+        plainAccessResource.addResourceAndPerm("topicB", Permission.SUB);
+        plainPermissionLoader.checkPerm(plainAccessResource, SUBPlainAccessResource);
+        plainAccessResource.addResourceAndPerm("topicA", Permission.PUB);
+        plainPermissionLoader.checkPerm(plainAccessResource, ANYPlainAccessResource);
 
     }
 
@@ -226,7 +226,7 @@ public void watchTest() throws IOException, IllegalAccessException {
         System.setProperty("rocketmq.home.dir", "src/test/resources/watch");
         File file = new File("src/test/resources/watch/conf");
         file.mkdirs();
-        File transport = new File("src/test/resources/watch/conf/transport.yml");
+        File transport = new File("src/test/resources/watch/conf/plain_acl.yml");
         transport.createNewFile();
 
         FileWriter writer = new FileWriter(transport);
@@ -240,9 +240,9 @@ public void watchTest() throws IOException, IllegalAccessException {
         PlainPermissionLoader plainPermissionLoader = new PlainPermissionLoader();
 
         Map<String, List<PlainAccessResource>> plainAccessResourceMap = (Map<String, List<PlainAccessResource>>) FieldUtils.readDeclaredField(plainPermissionLoader, "plainAccessResourceMap", true);
-        Assert.assertEquals(plainAccessResourceMap.get("rokcetmq").size(), 1);
+        Assert.assertNotNull(plainAccessResourceMap.get("rokcetmq"));
 
-        writer = new FileWriter(new File("src/test/resources/watch/conf/transport.yml"), true);
+        writer = new FileWriter(new File("src/test/resources/watch/conf/plain_acl.yml"), true);
         writer.write("- accessKey: rokcet1\r\n");
         writer.write("  secretKey: aliyun1\r\n");
         writer.write("  whiteRemoteAddress: 127.0.0.1\r\n");
@@ -256,7 +256,7 @@ public void watchTest() throws IOException, IllegalAccessException {
             e.printStackTrace();
         }
         plainAccessResourceMap = (Map<String, List<PlainAccessResource>>) FieldUtils.readDeclaredField(plainPermissionLoader, "plainAccessResourceMap", true);
-        Assert.assertEquals(plainAccessResourceMap.get("rokcet1").size(), 1);
+        Assert.assertNotNull(plainAccessResourceMap.get("rokcet1"));
 
         transport.delete();
         file.delete();
@@ -267,7 +267,7 @@ public void watchTest() throws IOException, IllegalAccessException {
 
     @Test(expected = AclException.class)
     public void initializeTest() {
-        System.setProperty("romcketmq.acl.plain.fileName", "/conf/transport-null.yml");
+        System.setProperty("rocketmq.acl.plain.file", "/conf/plain_acl_null.yml");
         new PlainPermissionLoader();
 
     }
diff --git a/acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java b/acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java
index 527c5c297..a390c604f 100644
--- a/acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java
+++ b/acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java
@@ -24,14 +24,18 @@
 
     RemoteAddressStrategyFactory remoteAddressStrategyFactory = new RemoteAddressStrategyFactory();
 
+    @Test(expected = AclException.class)
+    public void netaddressStrategyFactoryExceptionTest() {
+        PlainAccessResource plainAccessResource = new PlainAccessResource();
+        remoteAddressStrategyFactory.getRemoteAddressStrategy(plainAccessResource);
+    }
+
     @Test
-    public void NetaddressStrategyFactoryTest() {
+    public void netaddressStrategyFactoryTest() {
         PlainAccessResource plainAccessResource = new PlainAccessResource();
-        RemoteAddressStrategy remoteAddressStrategy = remoteAddressStrategyFactory.getRemoteAddressStrategy(plainAccessResource);
-        Assert.assertEquals(remoteAddressStrategy, RemoteAddressStrategyFactory.NULL_NET_ADDRESS_STRATEGY);
 
         plainAccessResource.setWhiteRemoteAddress("*");
-        remoteAddressStrategy = remoteAddressStrategyFactory.getRemoteAddressStrategy(plainAccessResource);
+        RemoteAddressStrategy remoteAddressStrategy = remoteAddressStrategyFactory.getRemoteAddressStrategy(plainAccessResource);
         Assert.assertEquals(remoteAddressStrategy, RemoteAddressStrategyFactory.NULL_NET_ADDRESS_STRATEGY);
 
         plainAccessResource.setWhiteRemoteAddress("127.0.0.1");
diff --git a/acl/src/test/resources/conf/transport.yml b/acl/src/test/resources/conf/plain_acl.yml
similarity index 100%
rename from acl/src/test/resources/conf/transport.yml
rename to acl/src/test/resources/conf/plain_acl.yml
diff --git a/acl/src/test/resources/conf/transport-null.yml b/acl/src/test/resources/conf/plain_acl_null.yml
similarity index 100%
rename from acl/src/test/resources/conf/transport-null.yml
rename to acl/src/test/resources/conf/plain_acl_null.yml
diff --git a/distribution/conf/transport.yml b/distribution/conf/plain_acl.yml
similarity index 100%
rename from distribution/conf/transport.yml
rename to distribution/conf/plain_acl.yml
diff --git a/pom.xml b/pom.xml
index 1f2091a5b..a337929e4 100644
--- a/pom.xml
+++ b/pom.xml
@@ -216,9 +216,9 @@
                     <execution>
                         <id>generate-effective-dependencies-pom</id>
                         <phase>generate-resources</phase>
-                        <goals>
+                        <!-- <goals>
                             <goal>effective-pom</goal>
-                        </goals>
+                        </goals> -->
                         <configuration>
                             <output>${project.build.directory}/effective-pom/effective-dependencies.xml</output>
                         </configuration>


 

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


With regards,
Apache Git Services