-
Notifications
You must be signed in to change notification settings - Fork 1
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
Making user work #250
Making user work #250
Changes from all commits
291c2a9
d47f160
4b197e1
e59b8ec
f89a077
691ab60
757ece3
007df35
b5ca1de
5d8b88a
4a0ad90
a6c589d
11edf9c
2358293
5826069
0c60d6a
e25cf5c
654f982
001f498
7d8cea4
69d2660
081f25e
6e632b2
1231ec5
c23063c
7807307
6098790
b2f1d14
e6a2628
ea487c4
6c06381
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,32 @@ | ||||||
package org.folio.fqm.repository; | ||||||
|
||||||
import lombok.RequiredArgsConstructor; | ||||||
import lombok.SneakyThrows; | ||||||
import lombok.extern.log4j.Log4j2; | ||||||
|
||||||
import org.folio.fqm.exception.EntityTypeNotFoundException; | ||||||
import org.folio.querytool.domain.dto.BooleanType; | ||||||
import org.folio.querytool.domain.dto.EntityTypeColumn; | ||||||
import org.folio.querytool.domain.dto.EntityTypeSource; | ||||||
import org.folio.querytool.domain.dto.EntityTypeSourceJoin; | ||||||
import org.folio.querytool.domain.dto.ValueWithLabel; | ||||||
import org.jooq.DSLContext; | ||||||
import org.jooq.Condition; | ||||||
import org.jooq.Field; | ||||||
|
||||||
import org.springframework.beans.factory.annotation.Qualifier; | ||||||
import org.springframework.stereotype.Repository; | ||||||
|
||||||
import com.fasterxml.jackson.databind.ObjectMapper; | ||||||
|
||||||
import org.folio.querytool.domain.dto.EntityType; | ||||||
|
||||||
import java.util.ArrayList; | ||||||
import java.util.List; | ||||||
import java.util.Optional; | ||||||
import java.util.Set; | ||||||
import java.util.UUID; | ||||||
|
||||||
import static org.apache.commons.collections4.CollectionUtils.isEmpty; | ||||||
import static org.jooq.impl.DSL.field; | ||||||
import static org.jooq.impl.DSL.or; | ||||||
|
@@ -110,6 +137,63 @@ public void replaceEntityTypeDefinitions(List<EntityType> entityTypes) { | |||||
}); | ||||||
} | ||||||
|
||||||
public Optional<EntityType> getCompositeEntityTypeDefinition(UUID entityTypeId) { | ||||||
log.info("Getting COMPOSITE definition for entity type ID: {}", entityTypeId); | ||||||
Field<String> definitionField = field("definition", String.class); | ||||||
|
||||||
EntityType entityType = jooqContext | ||||||
.select(definitionField) | ||||||
.from(table(TABLE_NAME)) | ||||||
.where(field(ID_FIELD_NAME).eq(entityTypeId)) | ||||||
.fetchOptional(definitionField) | ||||||
.map(this::unmarshallEntityType) | ||||||
.map(currentEntityType -> { | ||||||
String customFieldsEntityTypeId = currentEntityType.getCustomFieldEntityTypeId(); | ||||||
if (customFieldsEntityTypeId != null) { | ||||||
currentEntityType.getColumns().addAll(fetchColumnNamesForCustomFields(UUID.fromString(customFieldsEntityTypeId))); | ||||||
} | ||||||
return currentEntityType; | ||||||
}).orElseThrow(); | ||||||
|
||||||
// Build entity type from sources and joins | ||||||
// var source1 = entityType.getSources().get(0); | ||||||
// buildCompositeEntityTypeDefinition(entityType); | ||||||
|
||||||
return Optional.of(entityType); | ||||||
} | ||||||
|
||||||
private void buildCompositeEntityTypeDefinition(EntityType entityType) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method's name isn't very clear to me. When I see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could be wrong, but at a quick glance, it looks like this method can/should be |
||||||
System.out.println("BUILD COMPOSITE DEFINITION"); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These |
||||||
List<EntityTypeSourceJoin> entityTypeSourceJoins = new ArrayList<>(); | ||||||
List<EntityTypeSource> sources = entityType.getSources(); | ||||||
entityType.setFromClause("BUILD COMPOSITE DEFINITION"); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personal opinion: we should avoid using the |
||||||
StringBuilder fromClause = new StringBuilder(); | ||||||
for (int i = 0; i < sources.size(); i++) { | ||||||
EntityTypeSource source = sources.get(i); | ||||||
System.out.println("Building source: " + source.getAlias()); | ||||||
// TODO: below if-block checks if source is the first in the array, since there is nothing to join if it is. | ||||||
// But this needs to be done better | ||||||
if (i == 0) { // TODO: think about replacing this with -- if (source.getJoin() == null) | ||||||
fromClause.append(source.getAlias()); | ||||||
} else { | ||||||
if (source.getType().equals("db")) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This way, we handles nulls and avoid a potential NPE |
||||||
System.out.println("Building db source"); | ||||||
EntityTypeSourceJoin join = source.getJoin(); | ||||||
String joinType = join.getType(); | ||||||
String joinCondition = join.getCondition(); | ||||||
Comment on lines
+181
to
+183
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just stating the obvious here since this is implied by the comment above, but if |
||||||
String alias = source.getAlias(); | ||||||
fromClause.append(" " + joinType + " " + alias + " ON " + joinCondition); | ||||||
} else if (source.getType().equals("entity-type")) { | ||||||
System.out.println("Building entity-type source"); | ||||||
// get entity type definition | ||||||
// get sources from that one recursively | ||||||
} | ||||||
} | ||||||
} | ||||||
String fromClauseString = fromClause.toString(); | ||||||
entityType.setFromClause(fromClauseString); | ||||||
} | ||||||
|
||||||
private List<EntityTypeColumn> fetchColumnNamesForCustomFields(UUID entityTypeId) { | ||||||
log.info("Getting columns for entity type ID: {}", entityTypeId); | ||||||
EntityType entityTypeDefinition = getEntityTypeDefinition(entityTypeId) | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,17 @@ | ||
package org.folio.fqm.repository; | ||
|
||
import lombok.RequiredArgsConstructor; | ||
import lombok.extern.log4j.Log4j2; | ||
import org.folio.fqm.exception.EntityTypeNotFoundException; | ||
import org.folio.fqm.model.IdsWithCancelCallback; | ||
import org.folio.fqm.service.EntityTypeFlatteningService; | ||
import org.folio.fqm.service.FqlToSqlConverterService; | ||
import org.folio.fqm.utils.IdColumnUtils; | ||
import org.folio.fqm.utils.StreamHelper; | ||
import org.folio.fql.model.Fql; | ||
import org.folio.querytool.domain.dto.EntityType; | ||
import org.folio.querytool.domain.dto.EntityTypeDefaultSort; | ||
import org.folio.querytool.domain.dto.EntityTypeSource; | ||
import org.jooq.Condition; | ||
import org.jooq.Cursor; | ||
import org.jooq.DSLContext; | ||
|
@@ -27,19 +30,20 @@ | |
import java.util.stream.Stream; | ||
|
||
import static org.apache.commons.lang3.ObjectUtils.isEmpty; | ||
import static org.folio.fqm.repository.EntityTypeRepository.ID_FIELD_NAME; | ||
import static org.folio.fqm.utils.IdColumnUtils.RESULT_ID_FIELD; | ||
import static org.jooq.impl.DSL.field; | ||
import static org.jooq.impl.DSL.select; | ||
import static org.jooq.impl.DSL.table; | ||
|
||
@Repository | ||
@RequiredArgsConstructor(onConstructor = @__(@Autowired)) | ||
@Log4j2 | ||
public class IdStreamer { | ||
|
||
@Qualifier("readerJooqContext") private final DSLContext jooqContext; | ||
private final EntityTypeRepository entityTypeRepository; | ||
@Qualifier("readerJooqContext") | ||
private final DSLContext jooqContext; | ||
private final QueryDetailsRepository queryDetailsRepository; | ||
private final EntityTypeFlatteningService entityTypeFlatteningService; | ||
|
||
/** | ||
* Executes the given Fql Query and stream the result Ids back. | ||
|
@@ -49,7 +53,9 @@ public int streamIdsInBatch(UUID entityTypeId, | |
Fql fql, | ||
int batchSize, | ||
Consumer<IdsWithCancelCallback> idsConsumer) { | ||
EntityType entityType = getEntityType(entityTypeId); | ||
EntityType entityType = entityTypeFlatteningService | ||
.getFlattenedEntityType(entityTypeId, true) | ||
.orElseThrow(() -> new EntityTypeNotFoundException(entityTypeId)); | ||
Condition sqlWhereClause = FqlToSqlConverterService.getSqlCondition(fql.fqlCondition(), entityType); | ||
return this.streamIdsInBatch(entityType, sortResults, sqlWhereClause, batchSize, idsConsumer); | ||
} | ||
|
@@ -63,12 +69,12 @@ public int streamIdsInBatch(UUID queryId, | |
Consumer<IdsWithCancelCallback> idsConsumer) { | ||
UUID entityTypeId = queryDetailsRepository.getEntityTypeId(queryId); | ||
EntityType entityType = getEntityType(entityTypeId); | ||
Condition condition = field(ID_FIELD_NAME).in( | ||
Condition condition = field("\"source1\".id").in( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't forget to do something with this string before we merge this :) |
||
select(RESULT_ID_FIELD) | ||
.from(table("query_results")) | ||
.where(field("query_id").eq(queryId)) | ||
); | ||
return streamIdsInBatch(entityType, sortResults, condition, batchSize, idsConsumer); | ||
return streamIdsInBatch(entityType, sortResults, condition, batchSize, idsConsumer); // TODO: fix this | ||
} | ||
|
||
public List<List<String>> getSortedIds(String derivedTableName, | ||
|
@@ -91,19 +97,29 @@ public List<List<String>> getSortedIds(String derivedTableName, | |
} | ||
|
||
private int streamIdsInBatch(EntityType entityType, | ||
boolean sortResults, | ||
Condition sqlWhereClause, | ||
int batchSize, | ||
Consumer<IdsWithCancelCallback> idsConsumer) { | ||
boolean sortResults, | ||
Condition sqlWhereClause, | ||
int batchSize, | ||
Consumer<IdsWithCancelCallback> idsConsumer) { | ||
String finalJoinClause = entityTypeFlatteningService.getJoinClause(entityType); | ||
Field<String[]> idValueGetter = IdColumnUtils.getResultIdValueGetter(entityType); | ||
String finalWhereClause = sqlWhereClause.toString(); | ||
for (EntityTypeSource source : entityType.getSources()) { | ||
String toReplace = ":" + source.getAlias(); | ||
String alias = "\"" + source.getAlias() + "\""; | ||
finalWhereClause = finalWhereClause.replace(toReplace, alias); | ||
} | ||
Comment on lines
+106
to
+111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should do this replacement by using SQL parameters, if at all possible. Doing it manually is generally pretty dangerous and bug-prone. The code for handling |
||
System.out.println("Final where clause: " + finalWhereClause); | ||
try ( | ||
// NEW WAY | ||
Cursor<Record1<String[]>> idsCursor = jooqContext.dsl() | ||
.select(field(idValueGetter)) | ||
.from(entityType.getFromClause()) | ||
.where(sqlWhereClause) | ||
.from(finalJoinClause) | ||
.where(finalWhereClause) | ||
.orderBy(getSortFields(entityType, sortResults)) | ||
.fetchSize(batchSize) | ||
.fetchLazy(); | ||
|
||
Stream<String[]> idStream = idsCursor | ||
.stream() | ||
.map(row -> row.getValue(idValueGetter)); | ||
|
@@ -136,7 +152,7 @@ private static SortField<Object> toSortField(EntityTypeDefaultSort entityTypeDef | |
} | ||
|
||
private EntityType getEntityType(UUID entityTypeId) { | ||
return entityTypeRepository.getEntityTypeDefinition(entityTypeId) | ||
return entityTypeFlatteningService.getFlattenedEntityType(entityTypeId, true) | ||
.orElseThrow(() -> new EntityTypeNotFoundException(entityTypeId)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
package org.folio.fqm.repository; | ||
|
||
import static org.folio.fqm.repository.EntityTypeRepository.ID_FIELD_NAME; | ||
import static org.jooq.impl.DSL.field; | ||
|
||
import java.sql.SQLException; | ||
|
@@ -14,6 +13,7 @@ | |
import org.folio.fql.model.Fql; | ||
import org.folio.fqm.exception.FieldNotFoundException; | ||
import org.folio.fqm.exception.EntityTypeNotFoundException; | ||
import org.folio.fqm.service.EntityTypeFlatteningService; | ||
import org.folio.fqm.service.FqlToSqlConverterService; | ||
import org.folio.fqm.utils.IdColumnUtils; | ||
import org.folio.fqm.utils.SqlFieldIdentificationUtils; | ||
|
@@ -22,6 +22,7 @@ | |
|
||
import org.apache.commons.collections4.CollectionUtils; | ||
import org.folio.querytool.domain.dto.EntityTypeColumn; | ||
import org.folio.querytool.domain.dto.EntityTypeSource; | ||
import org.jooq.Condition; | ||
import org.jooq.DSLContext; | ||
import org.jooq.Field; | ||
|
@@ -42,7 +43,7 @@ | |
public class ResultSetRepository { | ||
|
||
@Qualifier("readerJooqContext") private final DSLContext jooqContext; | ||
private final EntityTypeRepository entityTypeRepository; | ||
private final EntityTypeFlatteningService entityTypeFlatteningService; | ||
|
||
public List<Map<String, Object>> getResultSet(UUID entityTypeId, | ||
List<String> fields, | ||
|
@@ -81,8 +82,12 @@ public List<Map<String, Object>> getResultSet(UUID entityTypeId, | |
whereClause = whereClause.and(field(idColumnValueGetter).in(idColumnValues)); | ||
} | ||
} | ||
|
||
String fromClause = entityTypeFlatteningService.getJoinClause(entityType); | ||
|
||
// Have to get FROM clause | ||
var result = jooqContext.select(fieldsToSelect) | ||
.from(entityType.getFromClause()) | ||
.from(fromClause) | ||
.where(whereClause) | ||
.fetch(); | ||
return recordToMap(result); | ||
|
@@ -104,11 +109,28 @@ public List<Map<String, Object>> getResultSet(UUID entityTypeId, Fql fql, List<S | |
} | ||
|
||
Condition condition = FqlToSqlConverterService.getSqlCondition(fql.fqlCondition(), entityType); | ||
// TODO: might want to put next 5 lines in its own method in EntityTypeFlatteningService | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree 100%, especially since this same code shows up in |
||
// also is it even necessary? | ||
String finalWhereClause = condition.toString(); | ||
for (EntityTypeSource source : entityType.getSources()) { | ||
String toReplace = ":" + source.getAlias(); | ||
String alias = "\"" + source.getAlias() + "\""; | ||
finalWhereClause = finalWhereClause.replace(toReplace, alias); | ||
} | ||
var fieldsToSelect = getSqlFields(entityType, fields); | ||
var sortCriteria = hasIdColumn(entityType) ? field(ID_FIELD_NAME) : DSL.noField(); | ||
String idFieldWithAlias = ""; | ||
String idColumnName = entityType | ||
.getColumns() | ||
.stream() | ||
.filter(EntityTypeColumn::getIsIdColumn) | ||
.findFirst() | ||
.orElseThrow() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should throw a specific exception, or at least a generic one with a clear error message |
||
.getName(); | ||
var sortCriteria = hasIdColumn(entityType) ? field(idColumnName) : DSL.noField(); // TODO: new changes break sorting | ||
String fromClause = entityTypeFlatteningService.getJoinClause(entityType); | ||
var result = jooqContext.select(fieldsToSelect) | ||
.from(entityType.getFromClause()) | ||
.where(condition) | ||
.from(fromClause) | ||
.where(finalWhereClause) | ||
.and(afterIdCondition) | ||
.orderBy(sortCriteria) | ||
.limit(limit) | ||
|
@@ -126,13 +148,13 @@ private List<Field<Object>> getSqlFields(EntityType entityType, List<String> fie | |
} | ||
|
||
private EntityType getEntityType(UUID entityTypeId) { | ||
return entityTypeRepository.getEntityTypeDefinition(entityTypeId) | ||
return entityTypeFlatteningService.getFlattenedEntityType(entityTypeId, true) | ||
.orElseThrow(() -> new EntityTypeNotFoundException(entityTypeId)); | ||
} | ||
|
||
private boolean hasIdColumn(EntityType entityType) { | ||
return entityType.getColumns().stream() | ||
.anyMatch(col -> ID_FIELD_NAME.equals(col.getName())); | ||
.anyMatch(col -> col.getIsIdColumn()); | ||
} | ||
|
||
private List<Map<String, Object>> recordToMap(Result<Record> result) { | ||
|
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.
Since the code above is commented out, this method is unused