Skip to content
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

Less usages of schema.getName() #107

Merged
merged 2 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's maintain a bit more compatibility by keeping getTableName() and just returning tableDescriptor.tableName() from this method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get this information from TableDescriptor in caller method (or from schema). This method have only one usage in our project, and there is better to fix it. I prefer to make smaller interface and remove this method

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] in table [%s]".formatted(index, schema.getTypeName())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use: in table [%s] -> for entity [%s] (because we have no access to table descriptor here)

);
}

@Override
Expand Down
Loading
Loading