This is an automated email from the ASF dual-hosted git repository.

lizhanhui pushed a commit to branch bazel
in repository https://gitbox.apache.org/repos/asf/rocketmq.git

commit 384022447c6ca6f4517ba37db4ed8591c8af1e3d
Author: Li Zhanhui <[email protected]>
AuthorDate: Tue Aug 23 14:12:12 2022 +0800

    Fix AclUtils
---
 .../org/apache/rocketmq/acl/common/AclUtils.java   |  90 +++++-----------
 .../apache/rocketmq/acl/common/AclUtilsTest.java   | 115 ++++++---------------
 2 files changed, 61 insertions(+), 144 deletions(-)

diff --git a/acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java 
b/acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java
index bde50422c..69e03523c 100644
--- a/acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java
+++ b/acl/src/main/java/org/apache/rocketmq/acl/common/AclUtils.java
@@ -17,14 +17,11 @@
 package org.apache.rocketmq.acl.common;
 
 import com.alibaba.fastjson.JSONObject;
-import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
-import java.io.FileOutputStream;
 import java.io.FileWriter;
-import java.io.IOException;
+import java.io.InputStream;
 import java.io.PrintWriter;
-import java.nio.channels.FileChannel;
 import java.util.Map;
 import java.util.SortedMap;
 import org.apache.commons.lang3.StringUtils;
@@ -239,92 +236,62 @@ public class AclUtils {
     }
 
     public static <T> T getYamlDataObject(String path, Class<T> clazz) {
+        try (FileInputStream fis = new FileInputStream(path)) {
+            return getYamlDataObject(fis, clazz);
+        } catch (FileNotFoundException ignore) {
+            return null;
+        } catch (Exception e) {
+            throw new AclException(e.getMessage(), e);
+        }
+    }
+
+    public static <T> T getYamlDataObject(InputStream fis, Class<T> clazz) {
         Yaml yaml = new Yaml();
-        FileInputStream fis = null;
         try {
-            fis = new FileInputStream(new File(path));
             return yaml.loadAs(fis, clazz);
-        } catch (FileNotFoundException ignore) {
-            return null;
         } catch (Exception e) {
-            throw new AclException(e.getMessage());
-        } finally {
-            if (fis != null) {
-                try {
-                    fis.close();
-                } catch (IOException ignore) {
-                }
-            }
+            throw new AclException(e.getMessage(), e);
         }
     }
 
     public static boolean writeDataObject(String path, Map<String, Object> 
dataMap) {
         Yaml yaml = new Yaml();
-        PrintWriter pw = null;
-        try {
-            pw = new PrintWriter(new FileWriter(path));
+        try (PrintWriter pw = new PrintWriter(new FileWriter(path))) {
             String dumpAsMap = yaml.dumpAsMap(dataMap);
             pw.print(dumpAsMap);
             pw.flush();
         } catch (Exception e) {
-            throw new AclException(e.getMessage());
-        } finally {
-            if (pw != null) {
-                pw.close();
-            }
+            throw new AclException(e.getMessage(), e);
         }
         return true;
     }
 
-    public static boolean copyFile(String from, String to) {
-        FileChannel input = null;
-        FileChannel output = null;
-        try {
-            input = new FileInputStream(new File(from)).getChannel();
-            output = new FileOutputStream(new File(to)).getChannel();
-            output.transferFrom(input, 0, input.size());
-            return true;
-        } catch (Exception e) {
-            log.error("file copy error. from={}, to={}", from, to, e);
-        } finally {
-            closeFileChannel(input);
-            closeFileChannel(output);
-        }
-        return false;
-    }
-
-    public static boolean moveFile(String from, String to) {
+    public static RPCHook getAclRPCHook(String fileName) {
+        JSONObject yamlDataObject;
         try {
-            File file = new File(from);
-            return file.renameTo(new File(to));
+            yamlDataObject = AclUtils.getYamlDataObject(fileName,
+                    JSONObject.class);
         } catch (Exception e) {
-            log.error("file move error. from={}, to={}", from, to, e);
-        }
-        return false;
-    }
-
-    private static void closeFileChannel(FileChannel fileChannel) {
-        if (fileChannel != null) {
-            try {
-                fileChannel.close();
-            } catch (IOException e) {
-                log.error("Close file channel error.", e);
-            }
+            log.error("Convert yaml file to data object error, ", e);
+            return null;
         }
+        return buildRpcHook(yamlDataObject);
     }
 
-    public static RPCHook getAclRPCHook(String fileName) {
+    public static RPCHook getAclRPCHook(InputStream inputStream) {
         JSONObject yamlDataObject = null;
         try {
-            yamlDataObject = AclUtils.getYamlDataObject(fileName,
-                    JSONObject.class);
+            yamlDataObject = AclUtils.getYamlDataObject(inputStream, 
JSONObject.class);
         } catch (Exception e) {
             log.error("Convert yaml file to data object error, ", e);
             return null;
         }
+        return buildRpcHook(yamlDataObject);
+    }
 
+    private static RPCHook buildRpcHook(JSONObject yamlDataObject) {
         if (yamlDataObject == null || yamlDataObject.isEmpty()) {
-            log.warn("Cannot find conf file :{}, acl isn't be enabled.", 
fileName);
+            log.warn("Failed to parse configuration to enable ACL.");
             return null;
         }
 
@@ -332,8 +299,7 @@ public class AclUtils {
         String secretKey = 
yamlDataObject.getString(AclConstants.CONFIG_SECRET_KEY);
 
         if (StringUtils.isBlank(accessKey) || StringUtils.isBlank(secretKey)) {
-            log.warn("AccessKey or secretKey is blank, the acl is not 
enabled.");
-
+            log.warn("Failed to enable ACL. Either AccessKey or secretKey is 
blank");
             return null;
         }
         return new AclClientRPCHook(new SessionCredentials(accessKey, 
secretKey));
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 850b5335a..1d60ba48b 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
@@ -19,11 +19,15 @@ package org.apache.rocketmq.acl.common;
 import com.alibaba.fastjson.JSONObject;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.UUID;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.rocketmq.remoting.RPCHook;
 import org.junit.Assert;
@@ -32,13 +36,10 @@ import org.junit.Test;
 public class AclUtilsTest {
 
     @Test
-    public void getAddresses() {
+    public void testGetAddresses() {
         String address = "1.1.1.{1,2,3,4}";
         String[] addressArray = AclUtils.getAddresses(address, "{1,2,3,4}");
-        List<String> newAddressList = new ArrayList<>();
-        for (String a : addressArray) {
-            newAddressList.add(a);
-        }
+        List<String> newAddressList = new 
ArrayList<>(Arrays.asList(addressArray));
 
         List<String> addressList = new ArrayList<>();
         addressList.add("1.1.1.1");
@@ -47,13 +48,11 @@ public class AclUtilsTest {
         addressList.add("1.1.1.4");
         Assert.assertEquals(newAddressList, addressList);
 
-//        IPv6 test
+        // IPv6 test
         String ipv6Address = "1:ac41:9987::bb22:666:{1,2,3,4}";
         String[] ipv6AddressArray = AclUtils.getAddresses(ipv6Address, 
"{1,2,3,4}");
         List<String> newIPv6AddressList = new ArrayList<>();
-        for (String a : ipv6AddressArray) {
-            newIPv6AddressList.add(a);
-        }
+        Collections.addAll(newIPv6AddressList, ipv6AddressArray);
 
         List<String> ipv6AddressList = new ArrayList<>();
         ipv6AddressList.add("1:ac41:9987::bb22:666:1");
@@ -64,7 +63,7 @@ public class AclUtilsTest {
     }
 
     @Test
-    public void isScopeStringArray() {
+    public void testIsScope_StringArray() {
         String address = "12";
 
         for (int i = 0; i < 6; i++) {
@@ -79,7 +78,7 @@ public class AclUtilsTest {
     }
 
     @Test
-    public void isScopeArray() {
+    public void testIsScope_Array() {
         String[] address = StringUtils.split("12.12.12.12", ".");
         boolean isScope = AclUtils.isScope(address, 4);
         Assert.assertTrue(isScope);
@@ -92,7 +91,7 @@ public class AclUtilsTest {
         isScope = AclUtils.isScope(address, 3);
         Assert.assertFalse(isScope);
 
-//        IPv6 test
+        // IPv6 test
         address = StringUtils.split("1050:0000:0000:0000:0005:0600:300c:326b", 
":");
         isScope = AclUtils.isIPv6Scope(address, 8);
         Assert.assertTrue(isScope);
@@ -110,11 +109,10 @@ public class AclUtilsTest {
         Assert.assertFalse(isScope);
         isScope = AclUtils.isIPv6Scope(address, 4);
         Assert.assertTrue(isScope);
-
     }
 
     @Test
-    public void isScopeStringTest() {
+    public void testIsScope_String() {
         for (int i = 0; i < 256; i++) {
             boolean isScope = AclUtils.isScope(i + "");
             Assert.assertTrue(isScope);
@@ -126,7 +124,7 @@ public class AclUtilsTest {
     }
 
     @Test
-    public void isScopeTest() {
+    public void testIsScope_Integral() {
         for (int i = 0; i < 256; i++) {
             boolean isScope = AclUtils.isScope(i);
             Assert.assertTrue(isScope);
@@ -136,7 +134,7 @@ public class AclUtilsTest {
         isScope = AclUtils.isScope(256);
         Assert.assertFalse(isScope);
 
-        //        IPv6 test
+        // IPv6 test
         int min = Integer.parseInt("0", 16);
         int max = Integer.parseInt("ffff", 16);
         for (int i = min; i < max + 1; i++) {
@@ -147,11 +145,10 @@ public class AclUtilsTest {
         Assert.assertFalse(isScope);
         isScope = AclUtils.isIPv6Scope(max + 1);
         Assert.assertFalse(isScope);
-
     }
 
     @Test
-    public void isAsteriskTest() {
+    public void testIsAsterisk() {
         boolean isAsterisk = AclUtils.isAsterisk("*");
         Assert.assertTrue(isAsterisk);
 
@@ -160,7 +157,7 @@ public class AclUtilsTest {
     }
 
     @Test
-    public void isColonTest() {
+    public void testIsComma() {
         boolean isColon = AclUtils.isComma(",");
         Assert.assertTrue(isColon);
 
@@ -169,7 +166,7 @@ public class AclUtilsTest {
     }
 
     @Test
-    public void isMinusTest() {
+    public void testIsMinus() {
         boolean isMinus = AclUtils.isMinus("-");
         Assert.assertTrue(isMinus);
 
@@ -178,30 +175,21 @@ public class AclUtilsTest {
     }
 
     @Test
-    public void v6ipProcessTest() {
+    public void testV6ipProcess() {
         String remoteAddr = "5::7:6:1-200:*";
-        String[] strArray = StringUtils.split(remoteAddr, ":");
         Assert.assertEquals(AclUtils.v6ipProcess(remoteAddr), 
"0005:0000:0000:0000:0007:0006");
-//        Assert.assertEquals(AclUtils.v6ipProcess(remoteAddr, strArray, 3), 
"0005:0000:0000:0000:0007:0006");
 
         remoteAddr = "5::7:6:1-200";
-        strArray = StringUtils.split(remoteAddr, ":");
         Assert.assertEquals(AclUtils.v6ipProcess(remoteAddr), 
"0005:0000:0000:0000:0000:0007:0006");
-//        Assert.assertEquals(AclUtils.v6ipProcess(remoteAddr, strArray, 3), 
"0005:0000:0000:0000:0000:0007:0006");
-
         remoteAddr = "5::7:6:*";
-        strArray = StringUtils.split(remoteAddr, ":");
         Assert.assertEquals(AclUtils.v6ipProcess(remoteAddr), 
"0005:0000:0000:0000:0000:0007:0006");
-//        Assert.assertEquals(AclUtils.v6ipProcess(remoteAddr, strArray, 3), 
"0005:0000:0000:0000:0000:0007:0006");
 
         remoteAddr = "5:7:6:*";
-        strArray = StringUtils.split(remoteAddr, ":");
         Assert.assertEquals(AclUtils.v6ipProcess(remoteAddr), 
"0005:0007:0006");
-//        Assert.assertEquals(AclUtils.v6ipProcess(remoteAddr, strArray, 3), 
"0005:0007:0006");
     }
 
     @Test
-    public void expandIPTest() {
+    public void testExpandIP() {
         Assert.assertEquals(AclUtils.expandIP("::", 8), 
"0000:0000:0000:0000:0000:0000:0000:0000");
         Assert.assertEquals(AclUtils.expandIP("::1", 8), 
"0000:0000:0000:0000:0000:0000:0000:0001");
         Assert.assertEquals(AclUtils.expandIP("3::", 8), 
"0003:0000:0000:0000:0000:0000:0000:0000");
@@ -214,19 +202,18 @@ public class AclUtilsTest {
 
     @SuppressWarnings("unchecked")
     @Test
-    public void getYamlDataObjectTest() {
-
+    public void testGetYamlDataObject() {
         Map<String, Object> map = 
AclUtils.getYamlDataObject("src/test/resources/conf/plain_acl_correct.yml", 
Map.class);
+        Assert.assertNotNull(map);
         Assert.assertFalse(map.isEmpty());
     }
 
     @Test
     public void writeDataObject2YamlFileTest() throws IOException {
-
-        String targetFileName = "src/test/resources/conf/plain_write_acl.yml";
+        String targetFileName = System.getProperty("java.io.tmpdir") + 
UUID.randomUUID() + ".yml";
         File transport = new File(targetFileName);
-        transport.delete();
-        transport.createNewFile();
+        Assert.assertTrue(transport.createNewFile());
+        transport.deleteOnExit();
 
         Map<String, Object> aclYamlMap = new HashMap<String, Object>();
 
@@ -249,17 +236,14 @@ public class AclUtilsTest {
         accounts.add(accountsMap);
         aclYamlMap.put("accounts", accounts);
         Assert.assertTrue(AclUtils.writeDataObject(targetFileName, 
aclYamlMap));
-
-        transport.delete();
     }
 
     @Test
     public void updateExistedYamlFileTest() throws IOException {
-
-        String targetFileName = "src/test/resources/conf/plain_update_acl.yml";
+        String targetFileName = System.getProperty("java.io.tmpdir") + 
UUID.randomUUID() + ".yml";
         File transport = new File(targetFileName);
-        transport.delete();
-        transport.createNewFile();
+        Assert.assertTrue(transport.createNewFile());
+        transport.deleteOnExit();
 
         Map<String, Object> aclYamlMap = new HashMap<String, Object>();
 
@@ -283,53 +267,20 @@ public class AclUtilsTest {
         Map<String, Object> readableMap = 
AclUtils.getYamlDataObject(targetFileName, Map.class);
         List<String> updatedGlobalWhiteRemoteAddrs = (List<String>) 
readableMap.get("globalWhiteRemoteAddrs");
         Assert.assertEquals("192.168.1.2", 
updatedGlobalWhiteRemoteAddrs.get(0));
-
-        transport.delete();
     }
 
     @Test
     public void getYamlDataIgnoreFileNotFoundExceptionTest() {
 
         JSONObject yamlDataObject = 
AclUtils.getYamlDataObject("plain_acl.yml", JSONObject.class);
-        Assert.assertTrue(yamlDataObject == null);
-    }
-
-
-    @Test
-    public void getAclRPCHookTest() {
-
-        //RPCHook errorContRPCHook = 
AclUtils.getAclRPCHook("src/test/resources/conf/plain_acl_format_error.yml");
-        //Assert.assertNull(errorContRPCHook);
-
-        RPCHook noFileRPCHook = 
AclUtils.getAclRPCHook("src/test/resources/plain_acl_format_error1.yml");
-        Assert.assertNull(noFileRPCHook);
-
-        //RPCHook emptyContRPCHook = 
AclUtils.getAclRPCHook("src/test/resources/conf/plain_acl_null.yml");
-        //Assert.assertNull(emptyContRPCHook);
-
-        RPCHook incompleteContRPCHook = 
AclUtils.getAclRPCHook("src/test/resources/conf/plain_acl_incomplete.yml");
-        Assert.assertNull(incompleteContRPCHook);
+        Assert.assertNull(yamlDataObject);
     }
 
     @Test
-    public void testCopyAndMoveFile() {
-        String path = "src/test/resources/conf/plain_acl.yml";
-        String backupPath = "src/test/resources/conf/plain_acl.yml_backup";
-        Map<String, Object> aclYamlData = AclUtils.getYamlDataObject(path, 
Map.class);
-
-        AclUtils.copyFile(path, backupPath);
-        Assert.assertEquals(aclYamlData, 
AclUtils.getYamlDataObject(backupPath, Map.class));
-
-        Map<String, Object> aclYamlMap = new HashMap<String, Object>();
-        List<String> globalWhiteRemoteAddrs = new ArrayList<String>();
-        globalWhiteRemoteAddrs.add("10.10.103.*");
-        globalWhiteRemoteAddrs.add("192.168.0.*");
-        aclYamlMap.put("globalWhiteRemoteAddrs", globalWhiteRemoteAddrs);
-        AclUtils.writeDataObject(path, aclYamlMap);
-        Assert.assertNotEquals(aclYamlData, AclUtils.getYamlDataObject(path, 
Map.class));
-
-        AclUtils.moveFile(backupPath, path);
-        Assert.assertEquals(aclYamlData, AclUtils.getYamlDataObject(path, 
Map.class));
+    public void getAclRPCHookTest() throws IOException {
+        try (InputStream is = 
AclUtilsTest.class.getClassLoader().getResourceAsStream("conf/plain_acl_incomplete.yml"))
 {
+            RPCHook incompleteContRPCHook = AclUtils.getAclRPCHook(is);
+            Assert.assertNull(incompleteContRPCHook);
+        }
     }
-
 }

Reply via email to