Skip to content

Commit

Permalink
Added some tests to validate the generated code against actual DBs
Browse files Browse the repository at this point in the history
  • Loading branch information
Tim203 committed Sep 12, 2024
1 parent ca3df84 commit 6c45c11
Show file tree
Hide file tree
Showing 34 changed files with 667 additions and 287 deletions.
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,7 @@
!database/mongo/src/**
!database/sql
!database/sql/src
!database/sql/src/**
!database/sql/src/**

# Ignore the test databases, which uh, should ideally be deleted after tests
core/database*
15 changes: 12 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,20 @@ However not everything is the same across dialects.
### Missing functionality for specific dialects

#### TestEntity deleteByAAndB(String, String)
This behaviour doesn't work on MySQL (not MariaDB) and H2.
This is because every dialect but MySQL and H2 support 'REPLACING' or a variant of it.
This currently doesn't work on MySQL (not MariaDB), H2 and Oracle Database.
For MySQL and H2 this is caused by the lack of support for 'REPLACING' or a variant of it.
For those two dialect we have to fetch a record first and then delete it (inside a transaction).
For Oracle Database the reason is that unlike the other 'REPLACING' implementations this dialect
does not accept a wildcard.
The current codebase is not flexible enough to do these wildly different behaviours per dialect,
but will be supported in the future.
but will be supported in the future.

#### any deleteFirst() and any deleteTop*()
Anything with a limit projection in delete currently doesn't work for Oracle Database, PostgreSQL and SQLite.
For SQLite this is a flag that can be enabled during compile, but it's disabled by default.
For Oracle and Postgres delete with a limit doesn't exist.
The current codebase is not flexible enough to do these wildly different behaviours per dialect,
but will be supported in the future.

### Different data types across dialects
Not every data type has the same name / is available on each dialect.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@

import com.squareup.javapoet.MethodSpec;
import org.geysermc.databaseutils.processor.query.QueryContext;
import org.geysermc.databaseutils.processor.query.section.projection.ProjectionKeywordCategory;
import org.geysermc.databaseutils.processor.type.RepositoryGenerator;
import org.geysermc.databaseutils.processor.util.InvalidRepositoryException;

final class DeleteAction extends Action {
DeleteAction() {
super("delete", true, true, true, false);
super("delete", true, true, true, false, ProjectionKeywordCategory.LIMIT);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ public ProjectionSection projection() {
return result.projection();
}

public boolean hasProjection() {
return projection() != null;
}

public boolean hasProjectionColumnName() {
return projection() != null && projection().columnName() != null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private QueryContext analyseValidateAndCreate() {
}

if (readResult.projection() != null) {
validateColumnNames(readResult.projection().projections(), SectionType.PROJECTION, null);
validateProjectionColumnName(readResult.projection().projections(), SectionType.PROJECTION);

var handledCategories = new ArrayList<ProjectionKeywordCategory>();
for (@NonNull ProjectionFactor projection : readResult.projection().projections()) {
Expand Down Expand Up @@ -187,17 +187,26 @@ private QueryContext analyseValidateAndCreate() {
return queryContext;
}

private void validateProjectionColumnName(List<ProjectionFactor> factors, SectionType type) {
for (ProjectionFactor factor : factors) {
CharSequence columnName = factor.columnName();
if (columnName != null) {
var column = info.columnFor(columnName);
if (column == null) {
throw new InvalidRepositoryException(
"Could not find column %s for entity %s", columnName, info.name());
}
}
}
}

private <T extends VariableFactor> void validateColumnNames(
List<? extends Factor> factors, SectionType type, BiConsumer<T, ColumnInfo> customValidation) {
var variableLast = true;
for (Factor factor : factors) {
CharSequence columnName = null;
if (factor instanceof VariableFactor variable) {
columnName = variable.columnName();
} else if (factor instanceof ProjectionFactor projection) {
if (projection.columnName() != null) {
columnName = projection.columnName();
}
}

if (columnName != null) {
Expand All @@ -208,7 +217,7 @@ private <T extends VariableFactor> void validateColumnNames(
}
variableLast = true;
if (customValidation != null) {
//noinspection DataFlowIssue,unchecked
//noinspection unchecked
customValidation.accept((T) factor, column);
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.geysermc.databaseutils.processor.query.section.projection.ProjectionKeyword;
import org.geysermc.databaseutils.processor.query.section.projection.keyword.DistinctProjectionKeyword;
import org.geysermc.databaseutils.processor.query.section.projection.keyword.FirstProjectionKeyword;
import org.geysermc.databaseutils.processor.query.section.projection.keyword.TopProjectionKeyword;

public record ProjectionSection(List<@NonNull ProjectionFactor> projections) {
public ProjectionSection(@NonNull List<@NonNull ProjectionFactor> projections) {
Expand Down Expand Up @@ -53,6 +54,17 @@ public boolean first() {
return false;
}

public int limit() {
for (var factor : projections) {
if (factor.keyword() instanceof FirstProjectionKeyword) {
return 1;
} else if (factor.keyword() instanceof TopProjectionKeyword top) {
return top.limit();
}
}
return -1;
}

/**
* Returns all the projection keywords that are not:
* <ul>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,6 @@
/*
* Copyright (c) 2024 GeyserMC <https://geysermc.org>
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*
* @author GeyserMC
* Copyright (c) 2024 GeyserMC
* Licensed under the MIT license
* @link https://github.com/GeyserMC/DatabaseUtils
*/
package org.geysermc.databaseutils.processor.query.section.projection.keyword;
Expand All @@ -46,6 +27,9 @@ public int limit() {
}

public void limit(int limit) {
if (limit <= 0) {
throw new IllegalArgumentException("Limit must be greater than 0");
}
this.limit = limit;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public CharSequence firstName() {
return name(0);
}

public boolean hasValueParameters() {
public boolean hasParameters() {
return !element.getParameters().isEmpty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.geysermc.databaseutils.processor.query.section.factor.OrFactor;
import org.geysermc.databaseutils.processor.query.section.factor.VariableByFactor;
import org.geysermc.databaseutils.processor.query.section.factor.VariableOrderByFactor;
import org.geysermc.databaseutils.processor.query.section.projection.keyword.FirstProjectionKeyword;
import org.geysermc.databaseutils.processor.query.section.projection.keyword.SkipProjectionKeyword;
import org.geysermc.databaseutils.processor.query.section.projection.keyword.TopProjectionKeyword;
import org.geysermc.databaseutils.processor.type.RepositoryGenerator;
Expand Down Expand Up @@ -171,6 +172,8 @@ public void addDelete(QueryContext context, MethodSpec.Builder spec) {

if (needsUpdatedCount) {
spec.addStatement("__count = this.collection.deleteOne($L).getDeletedCount()", filter);
} else if (context.returnInfo().isSelf()) {
spec.addStatement("return this.collection.findOneAndDelete($L)", filter);
} else {
spec.addStatement("this.collection.deleteOne($L)", filter);
}
Expand All @@ -195,7 +198,7 @@ public void addDelete(QueryContext context, MethodSpec.Builder spec) {
spec.addStatement("this.collection.bulkWrite(__bulkOperations)");
}
} else {
if (needsUpdatedCount) {
if (!context.hasProjection() && needsUpdatedCount) {
spec.addStatement(
"__count = (int) this.collection.deleteMany($L).getDeletedCount()",
createFilter(context.bySectionFactors()));
Expand All @@ -204,7 +207,7 @@ public void addDelete(QueryContext context, MethodSpec.Builder spec) {
if (context.returnInfo().isSelf()) {
spec.addStatement("return this.collection.findOneAndDelete($L)", filter);
return;
} else if (context.returnInfo().isSelfCollection()) {
} else if (context.returnInfo().isSelfCollection() || needsUpdatedCount) {
spec.addStatement("var __session = this.mongoClient.startSession()");
spec.beginControlFlow("try");
spec.addStatement("__session.startTransaction()");
Expand All @@ -213,7 +216,7 @@ public void addDelete(QueryContext context, MethodSpec.Builder spec) {
"var __find = this.collection.find(__session, $L)$L$L",
filter,
createSort(context),
createProjection(context));
createProjection(context, true));

spec.addStatement("var __toDelete = new $T<$T>()", ArrayList.class, Bson.class);
spec.beginControlFlow("for (var __found : __find)");
Expand All @@ -238,7 +241,17 @@ public void addDelete(QueryContext context, MethodSpec.Builder spec) {
spec.endControlFlow();

spec.addStatement("__session.commitTransaction()");
spec.addStatement("return __find");

if (needsUpdatedCount) {
if (context.typeUtils().isType(Boolean.class, context.returnType())) {
spec.addStatement("return __deletedCount > 0");
} else {
spec.addStatement("return (int) __deletedCount"); // todo make more flexible
}
} else {
spec.addStatement("return __find");
}

spec.nextControlFlow("catch ($T __exception)", Exception.class);
spec.addStatement("__session.abortTransaction()");
spec.addStatement("throw __exception");
Expand Down Expand Up @@ -346,6 +359,10 @@ private int createFilterSection(List<Factor> filterOrder, int index, CodeBlock.B
}

private CodeBlock createProjection(QueryContext context) {
return createProjection(context, false);
}

private CodeBlock createProjection(QueryContext context, boolean needsCollection) {
var builder = CodeBlock.builder();

if (context.projection() != null) {
Expand All @@ -358,6 +375,11 @@ private CodeBlock createProjection(QueryContext context) {
builder.add(".skip($L)", keyword.offset());
continue;
}
// todo are there other situations I'm missing?
if (projection instanceof FirstProjectionKeyword && needsCollection) {
builder.add(".limit(1)");
continue;
}
throw new InvalidRepositoryException("Unsupported projection %s", projection.name());
}

Expand All @@ -369,6 +391,8 @@ private CodeBlock createProjection(QueryContext context) {

if (context.returnInfo().isCollection()) {
builder.add(".into(new $T<>())", context.typeUtils().collectionImplementationFor(context.returnType()));
} else if (needsCollection) {
builder.add(".into(new $T<>())", ArrayList.class);
} else {
builder.add(".first()");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ public QueryBuilder add(String queryPart, boolean parameters, List<String> colum
return this;
}

public QueryBuilder addRaw(String queryPart, String... format) {
return addRaw(queryPart, List.of(format));
public QueryBuilder addRaw(String queryPart, List<String> format) {
return addRaw(queryPart, format.toArray(String[]::new));
}

public QueryBuilder addRaw(String queryPart, List<String> format) {
public QueryBuilder addRaw(String queryPart, String... format) {
if (!query.isEmpty()) {
query.append(' ');
}
query.append(queryPart.formatted(format.toArray()));
query.append(queryPart.formatted((Object[]) format));
return this;
}

Expand All @@ -83,6 +83,18 @@ public QueryBuilder addRawBefore(String before, String queryPart, String... form
return this;
}

/**
* Optionally add before, if before is not found then {@link #addRaw(String, String...)} is called
*/
public QueryBuilder addRawOptionalBefore(String before, String queryPart, String... format) {
int index = query.indexOf(before);
if (index == -1) {
return addRaw(queryPart, format);
}
query.insert(index, queryPart.formatted((Object[]) format) + (query.isEmpty() ? "" : " "));
return this;
}

public QueryBuilder addEndRaw(String queryPart, String... format) {
if (!endQuery.isEmpty()) {
endQuery.append(' ');
Expand All @@ -91,6 +103,14 @@ public QueryBuilder addEndRaw(String queryPart, String... format) {
return this;
}

public QueryBuilder addEndStartRaw(String queryPart, String... format) {
if (!endQuery.isEmpty()) {
endQuery.insert(0, ' ');
}
endQuery.insert(0, queryPart.formatted((Object[]) format));
return this;
}

public QueryBuilder addColumn(CharSequence columnName, boolean parameter) {
var parameterName = parameter ? queryContext.parametersInfo().name(parameterIndex++) : null;
columns.add(new QueryBuilderColumn(queryContext.columnFor(columnName), parameterName));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,17 @@ public void addDelete(QueryContext context, MethodSpec.Builder spec) {
var builder = new QueryBuilder(context).addRaw("delete from %s", context.tableName());
if (context.hasBySection()) {
builder.add("where %s", this::createWhereForFactors);
} else {
} else if (context.parametersInfo().hasParameters()) {
builder.add("where %s", this::createWhereForKeys);
}

if (context.hasProjection()) {
var limit = context.projection().limit();
if (limit != -1) {
builder.addRaw("limit " + limit);
}
}

boolean notSelfToSelf =
!context.parametersInfo().isSelf() && context.returnInfo().isAnySelf();
if (!notSelfToSelf) {
Expand All @@ -124,12 +131,20 @@ public void addDelete(QueryContext context, MethodSpec.Builder spec) {

spec.addStatement("String __sql");
spec.beginControlFlow(
"if (this.dialect == $T.POSTGRESQL || this.dialect == $T.ORACLE_DATABASE || this.dialect == $T.SQLITE || this.dialect == $T.MARIADB)",
SqlDialect.class,
"if (this.dialect == $T.POSTGRESQL || this.dialect == $T.SQLITE || this.dialect == $T.MARIADB)",
SqlDialect.class,
SqlDialect.class,
SqlDialect.class);
spec.addStatement("__sql = $S", builder.copy().addEndRaw("returning *"));
spec.nextControlFlow("else if (this.dialect == $T.ORACLE_DATABASE)", SqlDialect.class);
// todo this needs prepareStatement(__sql, new String[] {columnNames})
// var oracleDbSqlBuilder = builder.copy()
// .addEndRaw(
// "returning %s into %s",
// mapAndJoin(context.entityInfo().columns(), ColumnInfo::name),
// join(repeat("?", context.columns().size())));
// spec.addStatement("__sql = $S", oracleDbSqlBuilder);
spec.addStatement("throw new $T($S)", IllegalStateException.class, "This behaviour is not yet implemented!");
spec.nextControlFlow("else if (this.dialect == $T.SQL_SERVER)", SqlDialect.class);
spec.addStatement("__sql = $S", builder.copy().addRawBefore("where", "output deleted.*"));
spec.nextControlFlow(
Expand Down Expand Up @@ -271,7 +286,7 @@ private void addBySectionData(
builder.dialectDepending() ? "__sql" : '"' + builder.query() + '"');

CharSequence parameterName = "";
if (context.parametersInfo().hasValueParameters()) {
if (context.parametersInfo().hasParameters()) {
parameterName = context.parametersInfo().firstName();
}

Expand Down
Loading

0 comments on commit 6c45c11

Please sign in to comment.