github-advanced-security[bot] commented on code in PR #17829:
URL: https://github.com/apache/druid/pull/17829#discussion_r2010339750
##########
sql/src/main/java/org/apache/druid/sql/calcite/schema/DruidSchemaManager.java:
##########
@@ -55,17 +55,12 @@
@Deprecated
Map<String, DruidTable> getTables();
- default DruidTable getTable(String name)
- {
- return getTables().get(name);
- }
-
default DruidTable getTable(String name, BrokerSegmentMetadataCache
segmentMetadataCache)
{
return getTables().get(name);
}
- default Set<String> getTableNames()
+ default Set<String> getTableNames(BrokerSegmentMetadataCache
segmentMetadataCache)
Review Comment:
## Useless parameter
The parameter 'segmentMetadataCache' is never used.
[Show more
details](https://github.com/apache/druid/security/code-scanning/8768)
##########
processing/src/main/java/org/apache/druid/initialization/DruidModule.java:
##########
@@ -36,6 +40,52 @@
@ExtensionPoint
public interface DruidModule extends com.google.inject.Module
{
+ static DruidModule override(DruidModule baseModule, com.google.inject.Module
overrideGuiceModule)
+ {
+ return new DruidModule()
+ {
+ @SuppressWarnings("unused")
+ @Inject
+ public void injectMe(Injector injector)
+ {
+ injector.injectMembers(baseModule);
+ injector.injectMembers(overrideGuiceModule);
+ }
+
+ @Override
+ public void configure(Binder binder)
+ {
+ binder.install(Modules.override(baseModule).with(overrideGuiceModule));
+ }
+
+ @Override
+ public List<? extends Module> getJacksonModules()
+ {
+ return baseModule.getJacksonModules();
+ }
+ };
+ }
+
+ static DruidModule override(com.google.inject.Module baseModule,
com.google.inject.Module overrideGuiceModule)
Review Comment:
## Confusing overloading of methods
Method DruidModule.override(..) could be confused with overloaded method
[override](1), since dispatch depends on static types.
[Show more
details](https://github.com/apache/druid/security/code-scanning/8764)
##########
sql/src/test/java/org/apache/druid/quidem/DruidQuidemTestBase.java:
##########
@@ -100,58 +116,175 @@
}
filter = new WildcardFileFilter(filterStr);
}
- druidQuidemRunner = new DruidQuidemRunner(createCommandHandler());
+ this.druidQuidemRunner = druidQuidemRunner;
}
- /** Creates a command handler. */
- protected CommandHandler createCommandHandler()
+ protected static class QuidemTestCaseConfiguration
{
- return new DruidQuidemCommandHandler();
+ final String fileName;
+ final String componentSupplierName;
+
+ public QuidemTestCaseConfiguration(String componentSupplierName, String
fileName)
+ {
+ this.fileName = fileName;
+ this.componentSupplierName = componentSupplierName;
+ }
+
+ public String getTestName()
+ {
+ if (componentSupplierName == null) {
+ return fileName;
+ } else {
+ return StringUtils.format("%s@%s", fileName, componentSupplierName);
+ }
+ }
+
+ @Override
+ public String toString()
+ {
+ return getTestName();
+ }
}
- @ParameterizedTest
- @MethodSource("getFileNames")
- public void test(String testFileName) throws Exception
+ public List<QuidemTestCaseConfiguration> getTestConfigs() throws IOException
+ {
+ List<QuidemTestCaseConfiguration> ret = new ArrayList<>();
+ List<String> fileNames = getFileNames();
+ for (String file : fileNames) {
+ try {
+ ret.addAll(getConfigurationsFor(file));
+ }
+ catch (Exception e) {
+ throw DruidException.defensive(e, "While processing configurations for
quidem file [%s]", file);
+ }
+ }
+ return ret;
+ }
+
+ private List<QuidemTestCaseConfiguration> getConfigurationsFor(String
testFileName) throws IOException
{
File inFile = new File(getTestRoot(), testFileName);
+ File outFile = new File(inFile.getParentFile(), inFile.getName() + ".out");
+
+ Set<Class<MultiComponentSupplier>> metaSuppliers =
collectMetaSuppliers(inFile);
+
+ List<Class<? extends QueryComponentSupplier>> testSuppliers = null;
+ switch (metaSuppliers.size()) {
+ case 0:
+ testSuppliers = Collections.singletonList(null);
+ break;
+ case 1:
+ Class<MultiComponentSupplier> metaSupplier =
metaSuppliers.iterator().next();
Review Comment:
## Unread local variable
Variable 'Class<MultiComponentSupplier> metaSupplier' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/8766)
##########
sql/src/test/java/org/apache/druid/quidem/DruidQuidemTestBase.java:
##########
@@ -100,58 +116,175 @@
}
filter = new WildcardFileFilter(filterStr);
}
- druidQuidemRunner = new DruidQuidemRunner(createCommandHandler());
+ this.druidQuidemRunner = druidQuidemRunner;
}
- /** Creates a command handler. */
- protected CommandHandler createCommandHandler()
+ protected static class QuidemTestCaseConfiguration
{
- return new DruidQuidemCommandHandler();
+ final String fileName;
+ final String componentSupplierName;
+
+ public QuidemTestCaseConfiguration(String componentSupplierName, String
fileName)
+ {
+ this.fileName = fileName;
+ this.componentSupplierName = componentSupplierName;
+ }
+
+ public String getTestName()
+ {
+ if (componentSupplierName == null) {
+ return fileName;
+ } else {
+ return StringUtils.format("%s@%s", fileName, componentSupplierName);
+ }
+ }
+
+ @Override
+ public String toString()
+ {
+ return getTestName();
+ }
}
- @ParameterizedTest
- @MethodSource("getFileNames")
- public void test(String testFileName) throws Exception
+ public List<QuidemTestCaseConfiguration> getTestConfigs() throws IOException
+ {
+ List<QuidemTestCaseConfiguration> ret = new ArrayList<>();
+ List<String> fileNames = getFileNames();
+ for (String file : fileNames) {
+ try {
+ ret.addAll(getConfigurationsFor(file));
+ }
+ catch (Exception e) {
+ throw DruidException.defensive(e, "While processing configurations for
quidem file [%s]", file);
+ }
+ }
+ return ret;
+ }
+
+ private List<QuidemTestCaseConfiguration> getConfigurationsFor(String
testFileName) throws IOException
{
File inFile = new File(getTestRoot(), testFileName);
+ File outFile = new File(inFile.getParentFile(), inFile.getName() + ".out");
Review Comment:
## Unread local variable
Variable 'File outFile' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/8765)
##########
sql/src/test/java/org/apache/druid/quidem/DruidQuidemTestBase.java:
##########
@@ -100,58 +116,175 @@
}
filter = new WildcardFileFilter(filterStr);
}
- druidQuidemRunner = new DruidQuidemRunner(createCommandHandler());
+ this.druidQuidemRunner = druidQuidemRunner;
}
- /** Creates a command handler. */
- protected CommandHandler createCommandHandler()
+ protected static class QuidemTestCaseConfiguration
{
- return new DruidQuidemCommandHandler();
+ final String fileName;
+ final String componentSupplierName;
+
+ public QuidemTestCaseConfiguration(String componentSupplierName, String
fileName)
+ {
+ this.fileName = fileName;
+ this.componentSupplierName = componentSupplierName;
+ }
+
+ public String getTestName()
+ {
+ if (componentSupplierName == null) {
+ return fileName;
+ } else {
+ return StringUtils.format("%s@%s", fileName, componentSupplierName);
+ }
+ }
+
+ @Override
+ public String toString()
+ {
+ return getTestName();
+ }
}
- @ParameterizedTest
- @MethodSource("getFileNames")
- public void test(String testFileName) throws Exception
+ public List<QuidemTestCaseConfiguration> getTestConfigs() throws IOException
+ {
+ List<QuidemTestCaseConfiguration> ret = new ArrayList<>();
+ List<String> fileNames = getFileNames();
+ for (String file : fileNames) {
+ try {
+ ret.addAll(getConfigurationsFor(file));
+ }
+ catch (Exception e) {
+ throw DruidException.defensive(e, "While processing configurations for
quidem file [%s]", file);
+ }
+ }
+ return ret;
+ }
+
+ private List<QuidemTestCaseConfiguration> getConfigurationsFor(String
testFileName) throws IOException
{
File inFile = new File(getTestRoot(), testFileName);
+ File outFile = new File(inFile.getParentFile(), inFile.getName() + ".out");
+
+ Set<Class<MultiComponentSupplier>> metaSuppliers =
collectMetaSuppliers(inFile);
+
+ List<Class<? extends QueryComponentSupplier>> testSuppliers = null;
+ switch (metaSuppliers.size()) {
+ case 0:
+ testSuppliers = Collections.singletonList(null);
+ break;
+ case 1:
+ Class<MultiComponentSupplier> metaSupplier =
metaSuppliers.iterator().next();
+ testSuppliers =
MultiComponentSupplier.getSuppliers(metaSuppliers.iterator().next());
+ break;
+ case 2:
+ throw DruidException.defensive("Multiple MetaComponentSuppliers found
[%s].", metaSuppliers);
+ }
+
+ List<QuidemTestCaseConfiguration> ret = new ArrayList<>();
+ for (Class<? extends QueryComponentSupplier> supplier : testSuppliers) {
+ if (supplier == null) {
+ ret.add(new QuidemTestCaseConfiguration(null, testFileName));
+ } else {
+ ret.add(new QuidemTestCaseConfiguration(supplier.getSimpleName(),
testFileName));
+ }
+ }
+ return ret;
+ }
+
+ private Set<Class<MultiComponentSupplier>> collectMetaSuppliers(File inFile)
throws IOException
+ {
+ Set<Class<MultiComponentSupplier>> metaSuppliers = new HashSet<>();
+
+ for (String line : Files.readLines(inFile, StandardCharsets.UTF_8)) {
+ if (line.startsWith("!use")) {
+ String[] parts = line.split(" ");
+ if (parts.length == 2) {
+ SqlTestFrameworkConfig cfg =
SqlTestFrameworkConfig.fromURL(parts[1]);
+ validateFrameworkConfig(cfg);
+ if
(MultiComponentSupplier.class.isAssignableFrom(cfg.componentSupplier)) {
+ metaSuppliers.add((Class<MultiComponentSupplier>)
cfg.componentSupplier);
+ }
+ }
+ }
+ }
+ return metaSuppliers;
+ }
+
+ protected void validateFrameworkConfig(SqlTestFrameworkConfig cfg)
Review Comment:
## Useless parameter
The parameter 'cfg' is never used.
[Show more
details](https://github.com/apache/druid/security/code-scanning/8767)
##########
sql/src/test/java/org/apache/druid/quidem/DruidQuidemTestBase.java:
##########
@@ -100,58 +116,175 @@
}
filter = new WildcardFileFilter(filterStr);
}
- druidQuidemRunner = new DruidQuidemRunner(createCommandHandler());
+ this.druidQuidemRunner = druidQuidemRunner;
}
- /** Creates a command handler. */
- protected CommandHandler createCommandHandler()
+ protected static class QuidemTestCaseConfiguration
{
- return new DruidQuidemCommandHandler();
+ final String fileName;
+ final String componentSupplierName;
+
+ public QuidemTestCaseConfiguration(String componentSupplierName, String
fileName)
+ {
+ this.fileName = fileName;
+ this.componentSupplierName = componentSupplierName;
+ }
+
+ public String getTestName()
+ {
+ if (componentSupplierName == null) {
+ return fileName;
+ } else {
+ return StringUtils.format("%s@%s", fileName, componentSupplierName);
+ }
+ }
+
+ @Override
+ public String toString()
+ {
+ return getTestName();
+ }
}
- @ParameterizedTest
- @MethodSource("getFileNames")
- public void test(String testFileName) throws Exception
+ public List<QuidemTestCaseConfiguration> getTestConfigs() throws IOException
+ {
+ List<QuidemTestCaseConfiguration> ret = new ArrayList<>();
+ List<String> fileNames = getFileNames();
+ for (String file : fileNames) {
+ try {
+ ret.addAll(getConfigurationsFor(file));
+ }
+ catch (Exception e) {
+ throw DruidException.defensive(e, "While processing configurations for
quidem file [%s]", file);
+ }
+ }
+ return ret;
+ }
+
+ private List<QuidemTestCaseConfiguration> getConfigurationsFor(String
testFileName) throws IOException
{
File inFile = new File(getTestRoot(), testFileName);
+ File outFile = new File(inFile.getParentFile(), inFile.getName() + ".out");
+
+ Set<Class<MultiComponentSupplier>> metaSuppliers =
collectMetaSuppliers(inFile);
+
+ List<Class<? extends QueryComponentSupplier>> testSuppliers = null;
+ switch (metaSuppliers.size()) {
+ case 0:
+ testSuppliers = Collections.singletonList(null);
+ break;
+ case 1:
+ Class<MultiComponentSupplier> metaSupplier =
metaSuppliers.iterator().next();
+ testSuppliers =
MultiComponentSupplier.getSuppliers(metaSuppliers.iterator().next());
+ break;
+ case 2:
+ throw DruidException.defensive("Multiple MetaComponentSuppliers found
[%s].", metaSuppliers);
+ }
+
+ List<QuidemTestCaseConfiguration> ret = new ArrayList<>();
+ for (Class<? extends QueryComponentSupplier> supplier : testSuppliers) {
Review Comment:
## Dereferenced variable may be null
Variable [testSuppliers](1) may be null at this access because of [this](2)
assignment.
[Show more
details](https://github.com/apache/druid/security/code-scanning/8769)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
