You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org> on 2023/03/02 22:34:31 UTC
[GitHub] [druid] github-code-scanning[bot] commented on a diff in pull request #13877: Extend the IT framework to allow tests in extensions
github-code-scanning[bot] commented on code in PR #13877:
URL: https://github.com/apache/druid/pull/13877#discussion_r1123806649
##########
processing/src/main/java/org/apache/druid/guice/ExtensionsLoader.java:
##########
@@ -109,7 +116,7 @@
}
@VisibleForTesting
- public Map<Pair<File, Boolean>, URLClassLoader> getLoadersMap()
+ public Map<String, URLClassLoader> getLoadersMap()
Review Comment:
## Exposing internal representation
getLoadersMap exposes the internal representation stored in field loaders. The value may be modified [after this call to getLoadersMap](1).
[Show more details](https://github.com/apache/druid/security/code-scanning/4368)
##########
processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java:
##########
@@ -217,58 +241,121 @@
final File absolutePathExtension = temporaryFolder.newFolder();
- final ExtensionsConfig config = new ExtensionsConfig()
- {
- @Override
- public LinkedHashSet<String> getLoadList()
- {
- return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()));
- }
-
- @Override
- public String getDirectory()
- {
- return extensionsDir.getAbsolutePath();
- }
- };
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir.getAbsolutePath())
+ .loadList(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()))
+ .build();
final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage");
final File random_extension = new File(extensionsDir, "random-extensions");
mysql_metadata_storage.mkdir();
random_extension.mkdir();
- final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension};
- final File[] actualFileList = extnLoader.getExtensionFilesToLoad();
- Assert.assertArrayEquals(expectedFileList, actualFileList);
+ final List<File> expectedFileList = Arrays.asList(mysql_metadata_storage, absolutePathExtension);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_and_path() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir1, extn1);
+ extn1Dir.mkdir();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+ String extn2 = "extension2";
+ File extn2Dir = new File(extensionsDir2, extn2);
+ extn2Dir.mkdir();
Review Comment:
## Ignored error status of call
Method testGetExtensionFilesToLoad_with_load_list_and_path ignores exceptional return value of File.mkdir.
[Show more details](https://github.com/apache/druid/security/code-scanning/4373)
##########
processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java:
##########
@@ -217,58 +241,121 @@
final File absolutePathExtension = temporaryFolder.newFolder();
- final ExtensionsConfig config = new ExtensionsConfig()
- {
- @Override
- public LinkedHashSet<String> getLoadList()
- {
- return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()));
- }
-
- @Override
- public String getDirectory()
- {
- return extensionsDir.getAbsolutePath();
- }
- };
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir.getAbsolutePath())
+ .loadList(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()))
+ .build();
final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage");
final File random_extension = new File(extensionsDir, "random-extensions");
mysql_metadata_storage.mkdir();
random_extension.mkdir();
- final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension};
- final File[] actualFileList = extnLoader.getExtensionFilesToLoad();
- Assert.assertArrayEquals(expectedFileList, actualFileList);
+ final List<File> expectedFileList = Arrays.asList(mysql_metadata_storage, absolutePathExtension);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_and_path() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir1, extn1);
+ extn1Dir.mkdir();
Review Comment:
## Ignored error status of call
Method testGetExtensionFilesToLoad_with_load_list_and_path ignores exceptional return value of File.mkdir.
[Show more details](https://github.com/apache/druid/security/code-scanning/4372)
##########
processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java:
##########
@@ -217,58 +241,121 @@
final File absolutePathExtension = temporaryFolder.newFolder();
- final ExtensionsConfig config = new ExtensionsConfig()
- {
- @Override
- public LinkedHashSet<String> getLoadList()
- {
- return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()));
- }
-
- @Override
- public String getDirectory()
- {
- return extensionsDir.getAbsolutePath();
- }
- };
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir.getAbsolutePath())
+ .loadList(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()))
+ .build();
final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage");
final File random_extension = new File(extensionsDir, "random-extensions");
mysql_metadata_storage.mkdir();
random_extension.mkdir();
- final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension};
- final File[] actualFileList = extnLoader.getExtensionFilesToLoad();
- Assert.assertArrayEquals(expectedFileList, actualFileList);
+ final List<File> expectedFileList = Arrays.asList(mysql_metadata_storage, absolutePathExtension);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_and_path() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir1, extn1);
+ extn1Dir.mkdir();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+ String extn2 = "extension2";
+ File extn2Dir = new File(extensionsDir2, extn2);
+ extn2Dir.mkdir();
+
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir1.getAbsolutePath())
+ .path(Collections.singletonList(extensionsDir2.getAbsolutePath()))
+ .loadList(Arrays.asList(extn1, extn2))
+ .build();
+ final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
+
+ final List<File> expectedFileList = Arrays.asList(extn1Dir, extn2Dir);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_with_path_only() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+
+ // For fun, mix up the order
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir2, extn1);
+ extn1Dir.mkdir();
+ String extn2 = "extension2";
+ File extn2Dir = new File(extensionsDir1, extn2);
+ extn2Dir.mkdir();
Review Comment:
## Ignored error status of call
Method testGetExtensionFilesToLoad_with_load_list_with_path_only ignores exceptional return value of File.mkdir.
[Show more details](https://github.com/apache/druid/security/code-scanning/4375)
##########
processing/src/test/java/org/apache/druid/guice/ExtensionsLoaderTest.java:
##########
@@ -217,58 +241,121 @@
final File absolutePathExtension = temporaryFolder.newFolder();
- final ExtensionsConfig config = new ExtensionsConfig()
- {
- @Override
- public LinkedHashSet<String> getLoadList()
- {
- return Sets.newLinkedHashSet(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()));
- }
-
- @Override
- public String getDirectory()
- {
- return extensionsDir.getAbsolutePath();
- }
- };
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir.getAbsolutePath())
+ .loadList(Arrays.asList("mysql-metadata-storage", absolutePathExtension.getAbsolutePath()))
+ .build();
final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
final File mysql_metadata_storage = new File(extensionsDir, "mysql-metadata-storage");
final File random_extension = new File(extensionsDir, "random-extensions");
mysql_metadata_storage.mkdir();
random_extension.mkdir();
- final File[] expectedFileList = new File[]{mysql_metadata_storage, absolutePathExtension};
- final File[] actualFileList = extnLoader.getExtensionFilesToLoad();
- Assert.assertArrayEquals(expectedFileList, actualFileList);
+ final List<File> expectedFileList = Arrays.asList(mysql_metadata_storage, absolutePathExtension);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_and_path() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir1, extn1);
+ extn1Dir.mkdir();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+ String extn2 = "extension2";
+ File extn2Dir = new File(extensionsDir2, extn2);
+ extn2Dir.mkdir();
+
+ final ExtensionsConfig config = ExtensionsConfig.builder()
+ .directory(extensionsDir1.getAbsolutePath())
+ .path(Collections.singletonList(extensionsDir2.getAbsolutePath()))
+ .loadList(Arrays.asList(extn1, extn2))
+ .build();
+ final ExtensionsLoader extnLoader = new ExtensionsLoader(config);
+
+ final List<File> expectedFileList = Arrays.asList(extn1Dir, extn2Dir);
+ final List<File> actualFileList = extnLoader.getExtensionFilesToLoad();
+ Assert.assertEquals(expectedFileList, actualFileList);
+ }
+
+ @Test
+ public void testGetExtensionFilesToLoad_with_load_list_with_path_only() throws IOException
+ {
+ // The loader only looks for directories: doesn't matter if they are empty.
+ final File extensionsDir1 = temporaryFolder.newFolder();
+ final File extensionsDir2 = temporaryFolder.newFolder();
+
+ // For fun, mix up the order
+ String extn1 = "extension1";
+ File extn1Dir = new File(extensionsDir2, extn1);
+ extn1Dir.mkdir();
Review Comment:
## Ignored error status of call
Method testGetExtensionFilesToLoad_with_load_list_with_path_only ignores exceptional return value of File.mkdir.
[Show more details](https://github.com/apache/druid/security/code-scanning/4374)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org