-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(): jdbc custom dashboard implementation #6607
base: develop
Are you sure you want to change the base?
Conversation
Quality Gate failedFailed conditions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, I added some small suggestions but this is a good addition!
@@ -24,6 +25,7 @@ public abstract class AbstractMetricRepositoryTest { | |||
protected MetricRepositoryInterface metricRepository; | |||
|
|||
@Test | |||
@Disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:(
@@ -1,5 +1,5 @@ | |||
CREATE TABLE IF NOT EXISTS dashboards ( | |||
"key" VARCHAR(250) NOT NULL PRIMARY KEY, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert, this will fail existing installation as the file will be detected by Flyway as different (not 100% sure)
@@ -129,11 +134,18 @@ public Dashboard delete(String tenantId, String id) { | |||
|
|||
@Override | |||
public <F extends Enum<F>> ArrayListTotal<Map<String, Object>> generate(String tenantId, DataChart<?, DataFilter<F, ? extends ColumnDescriptor<F>>> dataChart, ZonedDateTime startDate, ZonedDateTime endDate, Pageable pageable) throws IOException { | |||
throw new NotImplementedException(); | |||
Map<Class<? extends QueryBuilderInterface<?>>, QueryBuilderInterface<?>> queryBuilderByHandledFields = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Map<Class<? extends QueryBuilderInterface<?>>, QueryBuilderInterface<?>> queryBuilderByHandledFields = new ConcurrentHashMap<>(); | |
Map<Class<? extends QueryBuilderInterface<?>>, QueryBuilderInterface<?>> queryBuilderByHandledFields = new HashMap<>(); |
ConcurrentHashMap
is only useful when used concurrently by multiple thread which is not the case here.
@@ -61,11 +64,28 @@ public abstract class AbstractJdbcExecutionRepository extends AbstractJdbcReposi | |||
private QueueInterface<Execution> executionQueue; | |||
private NamespaceUtils namespaceUtils; | |||
|
|||
@Getter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may have missed something, but I don't think the getter is used somewhere.
@Getter | ||
private final JdbcFilterService filterService; | ||
|
||
@Getter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may have missed something, but I don't think the getter is used somewhere.
In fact this should be a constant.
@@ -317,7 +336,44 @@ public Function<String, String> sortMapping() throws IllegalArgumentException { | |||
} | |||
|
|||
@Override | |||
public ArrayListTotal<Map<String, Object>> fetchData(String tenantId, DataFilter<Metrics.Fields, ? extends ColumnDescriptor<Metrics.Fields>> filter, ZonedDateTime startDate, ZonedDateTime endDate, Pageable pageable) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the 3 fetchData methods are the same on each DB.
Maybe you can move them to the parent class?
*/ | ||
protected <F extends Enum<F>> SelectSeekStepN<Record> orderBy(SelectHavingStep<Record> selectHavingStep, DataFilter<F, ? extends ColumnDescriptor<F>> descriptors) { | ||
List<SortField<?>> orderFields = new ArrayList<>(); | ||
if (descriptors.getOrderBy() != null && !descriptors.getOrderBy().isEmpty()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use !ListUtils.isEmpty()
which is null-safe
No description provided.