Skip to content

Commit

Permalink
Less usages of schema.getName() (#107)
Browse files Browse the repository at this point in the history
Co-authored-by: Alexander Lavrukov <[email protected]>
  • Loading branch information
lavrukov and Alexander Lavrukov authored Dec 6, 2024
1 parent 4b1165b commit 360e394
Show file tree
Hide file tree
Showing 15 changed files with 70 additions and 53 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ protected Schema(Schema<?> schema, String subSchemaFieldPath) {
this.changefeeds = schema.changefeeds;
}

public String getTypeName() {
return getType().getSimpleName();
}

private void validateFieldNames() {
flattenFields().stream().collect(toMap(JavaField::getName, Function.identity(), ((x, y) -> {
throw new IllegalArgumentException("fields with same name `%s` detected: `{%s}` and `{%s}`"
Expand Down Expand Up @@ -268,13 +272,15 @@ public final NamingStrategy getNamingStrategy() {
}

/**
* DEPRECATED: old method, use correct instance of {@link TableDescriptor}
* Returns the name of the table for data binding.
* <p>
* If the {@link Table} annotation is present, the field {@code name} should be used to
* specify the table name.
*
* @return the table name for data binding
*/
@Deprecated
public final String getName() {
return staticName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public <T> T toSchema(Schema<T> schema) {
} catch (ConversionException e) {
throw e;
} catch (Exception e) {
String message = format("Could not convert %s: %s", schema.getName(), e.getMessage());
String message = format("Could not convert %s: %s", schema.getTypeName(), e.getMessage());
throw new ConversionException(message, e);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import tech.ydb.yoj.repository.db.EntitySchema;
import tech.ydb.yoj.repository.db.Range;
import tech.ydb.yoj.repository.db.Table;
import tech.ydb.yoj.repository.db.TableDescriptor;
import tech.ydb.yoj.repository.db.ViewSchema;
import tech.ydb.yoj.repository.db.cache.FirstLevelCache;
import tech.ydb.yoj.repository.db.exception.IllegalTransactionIsolationLevelException;
Expand All @@ -34,11 +35,13 @@
public class InMemoryTable<T extends Entity<T>> implements Table<T> {
private final Class<T> type;
private final EntitySchema<T> schema;
private final TableDescriptor<T> tableDescriptor;
private final InMemoryRepositoryTransaction transaction;

public InMemoryTable(DbMemory<T> memory) {
this.type = memory.type();
this.schema = EntitySchema.of(type);
this.tableDescriptor = TableDescriptor.from(schema);
this.transaction = memory.transaction();
}

Expand Down Expand Up @@ -330,16 +333,16 @@ public <KEY> List<T> find(
.filter(i -> i.getIndexName().equals(indexName))
.findAny()
.orElseThrow(() -> new IllegalArgumentException(
"Entity `%s` doesn't have index `%s`".formatted(schema.getName(), indexName)
"Table `%s` doesn't have index `%s`".formatted(tableDescriptor.toDebugString(), indexName)
));

Set<String> indexKeys = Set.copyOf(globalIndex.getFieldNames());
Set<String> missingInIndexKeys = Sets.difference(keyFields, indexKeys);

Preconditions.checkArgument(
missingInIndexKeys.isEmpty(),
"Index `%s` of entity `%s` doesn't contain key(s): [%s]".formatted(
indexName, schema.getName(), String.join(", ", missingInIndexKeys)
"Index `%s` of table `%s` doesn't contain key(s): [%s]".formatted(
indexName, tableDescriptor.toDebugString(), String.join(", ", missingInIndexKeys)
)
);
Preconditions.checkArgument(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,24 @@
import tech.ydb.yoj.databind.schema.Schema;
import tech.ydb.yoj.repository.db.Entity;
import tech.ydb.yoj.repository.db.EntitySchema;
import tech.ydb.yoj.repository.db.TableDescriptor;
import tech.ydb.yoj.repository.ydb.yql.YqlType;

import java.util.HashMap;
import java.util.Map;

public final class BulkMapperImpl<E extends Entity<E>> implements BulkMapper<E> {
private final TableDescriptor<E> tableDescriptor;
private final EntitySchema<E> srcSchema;

public BulkMapperImpl(EntitySchema<E> srcSchema) {
public BulkMapperImpl(TableDescriptor<E> tableDescriptor, EntitySchema<E> srcSchema) {
this.tableDescriptor = tableDescriptor;
this.srcSchema = srcSchema;
}

@Override
public String getTableName(String tableSpace) {
return tableSpace + srcSchema.getName();
return tableSpace + tableDescriptor.tableName();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package tech.ydb.yoj.repository.ydb.merge;

import lombok.NonNull;
import lombok.Value;
import lombok.With;
import org.slf4j.Logger;
Expand Down Expand Up @@ -36,7 +35,7 @@ public class ByEntityYqlQueriesMerger implements YqlQueriesMerger {
Statement.QueryType.DELETE_ALL));
private static final Map<TransitionKey, MergingState> transitionMap = createTransitionMap();

private final Map<TableMetadata, TableState> states = new HashMap<>();
private final Map<TableDescriptor<?>, TableState> states = new HashMap<>();
private final RepositoryCache cache;

ByEntityYqlQueriesMerger(RepositoryCache cache) {
Expand All @@ -48,7 +47,8 @@ public void onNext(YdbRepository.Query<?> query) {
Statement.QueryType queryType = query.getStatement().getQueryType();
check(SUPPORTED_QUERY_TYPES.contains(queryType), "Unsupported query type: " + queryType);

TableState tableState = states.computeIfAbsent(new TableMetadata(getEntityClass(query), getTableName(query)), __ -> new TableState());
TableDescriptor<?> tableDescriptor = convertQueryToYqlStatement(query).getTableDescriptor();
TableState tableState = states.computeIfAbsent(tableDescriptor, __ -> new TableState());
if (queryType == Statement.QueryType.DELETE_ALL) {
tableState.entityStates.clear();
tableState.deleteAll = query;
Expand Down Expand Up @@ -193,10 +193,6 @@ private static Class getEntityClass(YdbRepository.Query query) {
return convertQueryToYqlStatement(query).getInSchemaType();
}

private static String getTableName(YdbRepository.Query query) {
return convertQueryToYqlStatement(query).getTableName();
}

private static YqlStatement convertQueryToYqlStatement(YdbRepository.Query query) {
return (YqlStatement) query.getStatement();
}
Expand Down Expand Up @@ -251,14 +247,8 @@ public boolean isEmpty() {

@Value
private static class TransitionKey {
private MergingState state;
private Statement.QueryType nextQueryType;
}

@Value
private static class TableMetadata {
private @NonNull Class<?> entityClass;
private @NonNull String tableName;
MergingState state;
Statement.QueryType nextQueryType;
}

private enum MergingState {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import tech.ydb.yoj.repository.db.Entity;
import tech.ydb.yoj.repository.db.EntityIdSchema;
import tech.ydb.yoj.repository.db.EntitySchema;
import tech.ydb.yoj.repository.db.TableDescriptor;
import tech.ydb.yoj.repository.ydb.statement.ResultSetReader;
import tech.ydb.yoj.repository.ydb.yql.YqlType;

Expand All @@ -14,11 +15,13 @@
import java.util.Map;

public final class EntityIdKeyMapper<E extends Entity<E>, ID extends Entity.Id<E>, RESULT> implements ReadTableMapper<ID, RESULT> {
private final TableDescriptor<E> tableDescriptor;
private final EntitySchema<E> srcSchema;
private final Schema<RESULT> dstSchema;
private final ResultSetReader<RESULT> resultSetReader;

public EntityIdKeyMapper(EntitySchema<E> srcSchema, Schema<RESULT> dstSchema) {
public EntityIdKeyMapper(TableDescriptor<E> tableDescriptor, EntitySchema<E> srcSchema, Schema<RESULT> dstSchema) {
this.tableDescriptor = tableDescriptor;
this.srcSchema = srcSchema;
this.dstSchema = dstSchema;
this.resultSetReader = new ResultSetReader<>(dstSchema);
Expand All @@ -45,7 +48,7 @@ public List<String> getColumns() {

@Override
public String getTableName(String tableSpace) {
return tableSpace + srcSchema.getName();
return tableSpace + tableDescriptor.tableName();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ public QueryType getQueryType() {

@Override
public String toDebugString(PARAMS params) {
return "deleteAll(" + schema.getName() + ")";
return "deleteAll(" + tableDescriptor.toDebugString() + ")";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ public QueryType getQueryType() {

@Override
public String toDebugString(PARAMS params) {
return "findAll(" + schema.getName() + ")";
return "findAll(" + tableDescriptor.toDebugString() + ")";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public static <K, T extends Entity<T>, RESULT> FindInStatement<Set<K>, T, RESULT
@Nullable Integer limit
) {
Schema<K> keySchema = getKeySchemaFromValues(keys);
Set<String> keyFields = collectKeyFieldsFromKeys(schema, indexName, keySchema, keys);
Set<String> keyFields = collectKeyFieldsFromKeys(tableDescriptor, schema, indexName, keySchema, keys);

return new FindInStatement<>(
tableDescriptor, schema, resultSchema, keySchema, keyFields, indexName, filter, orderBy, limit
Expand Down Expand Up @@ -219,11 +219,13 @@ private static <V> Schema<V> getKeySchemaFromValues(Iterable<V> keys) {
return ObjectSchema.of((Class<V>) key.getClass());
}

private static <V> Set<String> collectKeyFieldsFromKeys(
Schema<?> entitySchema,
private static <E extends Entity<E>, K> Set<String> collectKeyFieldsFromKeys(
TableDescriptor<E> tableDescriptor,
Schema<E> entitySchema,
String indexName,
Schema<V> keySchema,
Iterable<V> keys) {
Schema<K> keySchema,
Iterable<K> keys
) {
Set<Set<String>> nonNullFieldsSet = Streams.stream(keys)
.map(key -> nonNullKeyFieldNames(keySchema, key))
.collect(toUnmodifiableSet());
Expand All @@ -238,7 +240,7 @@ private static <V> Set<String> collectKeyFieldsFromKeys(
.filter(index -> indexName.equals(index.getIndexName()))
.findAny()
.orElseThrow(() -> new IllegalArgumentException(
"Entity `%s` doesn't have index `%s`".formatted(entitySchema.getName(), indexName)
"Table `%s` doesn't have index `%s`".formatted(tableDescriptor.toDebugString(), indexName)
));

// 2. all key fields are index key fields
Expand All @@ -247,8 +249,8 @@ private static <V> Set<String> collectKeyFieldsFromKeys(

Preconditions.checkArgument(
missingInIndexKeys.isEmpty(),
"Index `%s` of entity `%s` doesn't contain key(s): [%s]".formatted(
indexName, entitySchema.getName(), String.join(", ", missingInIndexKeys)
"Index `%s` of table `%s` doesn't contain key(s): [%s]".formatted(
indexName, tableDescriptor.toDebugString(), String.join(", ", missingInIndexKeys)
)
);

Expand All @@ -268,8 +270,8 @@ private static <V> Set<String> collectKeyFieldsFromKeys(

Preconditions.checkArgument(
entityFieldType.equals(keyFieldType.getValue()),
"Entity `%s` has column `%s` of type `%s`, but corresponding key field is `%s`".formatted(
entitySchema.getName(), keyFieldType.getKey(), entityFieldType, keyFieldType.getValue()
"Table `%s` has column `%s` of type `%s`, but corresponding key field is `%s`".formatted(
tableDescriptor.toDebugString(), keyFieldType.getKey(), entityFieldType, keyFieldType.getValue()
)
);
}
Expand Down Expand Up @@ -316,7 +318,7 @@ private void validateOrderByFields() {
Preconditions.checkArgument(
missingColumns.isEmpty(),
"Result schema of '%s' does not contain field(s): [%s] by which the result is ordered: %s".formatted(
resultSchema.getType().getSimpleName(), String.join(", ", missingColumns), orderBy
resultSchema.getTypeName(), String.join(", ", missingColumns), orderBy
)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public RESULT readResult(List<ValueProtos.Column> columnList, ValueProtos.Value
return resultSchema.newInstance(cells);
} catch (Exception e) {
throw new ConversionException(
format("Could not convert %s%s: %s", resultSchema.getName(), id(cells), e.getMessage()),
format("Could not convert %s%s: %s", resultSchema.getTypeName(), id(cells), e.getMessage()),
e
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import com.google.common.base.Preconditions;
import com.google.protobuf.NullValue;
import lombok.NonNull;
import lombok.Getter;
import tech.ydb.proto.ValueProtos;
import tech.ydb.yoj.databind.schema.Schema;
import tech.ydb.yoj.repository.db.Entity;
Expand Down Expand Up @@ -38,15 +38,16 @@ public abstract class YqlStatement<PARAMS, ENTITY extends Entity<ENTITY>, RESULT
protected final EntitySchema<ENTITY> schema;
protected final Schema<RESULT> resultSchema;
protected final ResultSetReader<RESULT> resultSetReader;
protected final String tableName;
@Getter
protected final TableDescriptor<ENTITY> tableDescriptor;

public YqlStatement(
TableDescriptor<ENTITY> tableDescriptor, EntitySchema<ENTITY> schema, Schema<RESULT> resultSchema
) {
this.schema = schema;
this.resultSchema = resultSchema;
this.resultSetReader = new ResultSetReader<>(resultSchema);
this.tableName = tableDescriptor.tableName();
this.tableDescriptor = tableDescriptor;
}

@Override
Expand Down Expand Up @@ -136,10 +137,6 @@ public Class<ENTITY> getInSchemaType() {
return schema.getType();
}

public @NonNull String getTableName() {
return tableName;
}

protected Collection<YqlStatementParam> getParams() {
return emptyList();
}
Expand All @@ -164,7 +161,7 @@ protected String nameEqVars() {
}

protected String table(String tablespace) {
return escape(tablespace + tableName);
return escape(tablespace + tableDescriptor.tableName());
}

protected String escape(String value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,28 +201,28 @@ public void deleteAll() {

@Override
public void bulkUpsert(List<T> input, BulkParams params) {
var mapper = new BulkMapperImpl<>(schema);
var mapper = new BulkMapperImpl<>(tableDescriptor, schema);
executor.bulkUpsert(mapper, input, params);
}

@Override
public <ID extends Entity.Id<T>> Stream<T> readTable(ReadTableParams<ID> params) {
ReadTableMapper<ID, T> mapper = new EntityIdKeyMapper<>(schema, schema);
ReadTableMapper<ID, T> mapper = new EntityIdKeyMapper<>(tableDescriptor, schema, schema);
return readTableStream(mapper, params)
.map(T::postLoad);
}

@Override
public <ID extends Entity.Id<T>> Stream<ID> readTableIds(ReadTableParams<ID> params) {
EntityIdSchema<ID> idSchema = schema.getIdSchema();
ReadTableMapper<ID, ID> mapper = new EntityIdKeyMapper<>(schema, idSchema);
ReadTableMapper<ID, ID> mapper = new EntityIdKeyMapper<>(tableDescriptor, schema, idSchema);
return readTableStream(mapper, params);
}

@Override
public <V extends ViewId<T>, ID extends Id<T>> Stream<V> readTable(Class<V> viewClass, ReadTableParams<ID> params) {
ViewSchema<V> viewSchema = ViewSchema.of(viewClass);
ReadTableMapper<ID, V> mapper = new EntityIdKeyMapper<>(schema, viewSchema);
ReadTableMapper<ID, V> mapper = new EntityIdKeyMapper<>(tableDescriptor, schema, viewSchema);
return readTableStream(mapper, params);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,9 @@ public <T extends Entity<T>> String toYql(@NonNull EntitySchema<T> schema) {
return field.flatten()
.map(this::fieldToYql)
.reduce(rel::combine)
.orElseThrow(() -> new IllegalStateException("No DB fields found for " + fieldPath + " in " + schema.getName()));
.orElseThrow(() -> new IllegalStateException(
"No DB fields found for " + fieldPath + " in " + schema.getTypeName()
));
}

private String fieldToYql(EntitySchema.JavaField field) {
Expand Down Expand Up @@ -711,7 +713,9 @@ public <T extends Entity<T>> String toYql(@NonNull EntitySchema<T> schema) {
return schema.getField(fieldPath).flatten()
.map(dbField -> format("`%s` %s", dbField.getName(), type.yql))
.reduce(type::combine)
.orElseThrow(() -> new IllegalStateException("No DB fields found for " + fieldPath + " in " + schema.getName()));
.orElseThrow(() -> new IllegalStateException(
"No DB fields found for " + fieldPath + " in " + schema.getTypeName()
));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,9 @@ public <T extends Entity<T>> String toYql(@NonNull EntitySchema<T> schema) {
}
}

throw new IllegalStateException(String.format("Unable to find index [%s] in table [%s]",
index, schema.getName()));
throw new IllegalStateException(
"Unable to find index [%s] for entity [%s]".formatted(index, schema.getTypeName())
);
}

@Override
Expand Down
Loading

0 comments on commit 360e394

Please sign in to comment.