handle TODO

acl/src/main/java/org/apache/rocketmq/acl/common/Permission.java

@@ -77,7 +77,8 @@ public class Permission {
         }
     }
 
-    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;
         }

acl/src/main/java/org/apache/rocketmq/acl/plain/PlainAccessResource.java

@@ -76,6 +76,7 @@ public class PlainAccessResource implements AccessResource {
         }
         return retryTopic.substring(MixAll.RETRY_GROUP_TOPIC_PREFIX.length());
     }
+
     public static String getRetryTopic(String group) {
         if (group == null) {
             return null;

acl/src/main/java/org/apache/rocketmq/acl/plain/PlainPermissionLoader.java

@@ -45,12 +45,12 @@ public class PlainPermissionLoader {
 
     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 @@ public class PlainPermissionLoader {
     private boolean isWatchStart;
 
     public PlainPermissionLoader() {
-        //TODO test what will happen if initialize failed
         initialize();
         watch();
     }
@@ -97,9 +96,15 @@ public class PlainPermissionLoader {
             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 class PlainPermissionLoader {
                             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 class PlainPermissionLoader {
                         }
                     }
                 }
+
                 @Override
                 public String getServiceName() {
                     return "AclWatcherService";
@@ -240,7 +245,6 @@ public class PlainPermissionLoader {
             return;
         }
 
-
         //Step 3, check the signature
         String signature = AclUtils.calSignature(plainAccessResource.getContent(), ownedAccess.getSecretKey());
         if (!signature.equals(plainAccessResource.getSignature())) {

acl/src/main/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyFactory.java

@@ -32,8 +32,10 @@ public class RemoteAddressStrategyFactory {
     }
 
     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("}")) {

acl/src/test/java/org/apache/rocketmq/acl/common/AclUtilsTest.java

@@ -129,13 +129,13 @@ public class AclUtilsTest {
     @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);
     }
 }

acl/src/test/java/org/apache/rocketmq/acl/plain/PlainPermissionLoaderTest.java

@@ -63,7 +63,7 @@ public class PlainPermissionLoaderTest {
 
         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 class PlainPermissionLoaderTest {
     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 class PlainPermissionLoaderTest {
         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 class PlainPermissionLoaderTest {
         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 class PlainPermissionLoaderTest {
             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 class PlainPermissionLoaderTest {
 
     @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();
 
     }

acl/src/test/java/org/apache/rocketmq/acl/plain/RemoteAddressStrategyTest.java

@@ -24,14 +24,18 @@ public class RemoteAddressStrategyTest {
 
     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");

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>