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.

Sign up for GitHub

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

fix: column.getReferences() returning incorrect column names for ColumnType.REFBACK #4569

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

fix: column.getReferences() returning incorrect column names for ColumnType.REFBACK #4569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e63013f
Fixed composite refback using dot separator when retrieving references
svandenhoek Dec 17, 2024
8bfc066
renamed test
svandenhoek Dec 17, 2024
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
18 changes: 9 additions & 9 deletions backend/molgenis-emx2-sql/src/main/java/org/molgenis/emx2/sql/SqlQuery.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public SqlQuery(SqlSchemaMetadata schema, String field, SelectColumn[] selection

/** Create alias that is short enough for postgresql to not complain */
public String alias(String label) {
if (!label.contains("-")) {
if (!label.contains(SUBSELECT_SEPARATOR)) {
// we only need aliases for subquery tables
return label;
}
Expand Down Expand Up @@ -154,7 +154,7 @@ private List<Field<?>> rowSelectFields(
List<Field<?>> fields = new ArrayList<>();
for (SelectColumn select : selection.getSubselect()) {
Column column = getColumnByName(table, select.getColumn());
String columnAlias = prefix.equals("") ? column.getName() : prefix + "-" + column.getName();
String columnAlias = prefix.equals("") ? column.getName() : prefix + SUBSELECT_SEPARATOR + column.getName();
if (column.isFile()) {
// check what they want to get, contents, mimetype, size, filename and/or extension
if (select.getSubselect().isEmpty() || select.has("id")) {
Expand All @@ -181,7 +181,7 @@ private List<Field<?>> rowSelectFields(
fields.addAll(
rowSelectFields(
column.getRefTable(),
tableAlias + "-" + column.getName(),
tableAlias + SUBSELECT_SEPARATOR + column.getName(),
columnAlias,
selection.getSubselect(column.getName())));
} else if (column.isRefback()) {
Expand Down Expand Up @@ -312,7 +312,7 @@ private Field<?> jsonSubselect(
Filter filters,
String[] searchTerms) {
checkHasViewPermission(table);
String subAlias = tableAlias + (parentColumn != null ? "-" + parentColumn.getName() : "");
String subAlias = tableAlias + (parentColumn != null ? SUBSELECT_SEPARATOR + parentColumn.getName() : "");
Collection<Field<?>> selection = jsonSubselectFields(table, subAlias, select);
return jsonField(
table, parentColumn, tableAlias, select, filters, searchTerms, subAlias, selection);
Expand Down Expand Up @@ -690,7 +690,7 @@ private Field<?> jsonAggregateSelect(
SelectColumn select,
Filter filters,
String[] searchTerms) {
String subAlias = tableAlias + (column != null ? "-" + column.getName() : "");
String subAlias = tableAlias + (column != null ? SUBSELECT_SEPARATOR + column.getName() : "");
List<Field<?>> fields = new ArrayList<>();
for (SelectColumn field : select.getSubselect()) {
if (COUNT_FIELD.equals(field.getColumn())) {
Expand Down Expand Up @@ -749,7 +749,7 @@ private Field<Object> jsonGroupBySelect(
Filter filter,
String[] searchTerms) {
DSLContext jooq = table.getJooq();
String subAlias = tableAlias + (column != null ? "-" + column.getName() : "");
String subAlias = tableAlias + (column != null ? SUBSELECT_SEPARATOR + column.getName() : "");

if (groupBy.getSubselect(COUNT_FIELD) == null && groupBy.getSubselect(SUM_FIELD) == null) {
throw new MolgenisException("COUNt or SUM is required when using group by");
Expand Down Expand Up @@ -928,7 +928,7 @@ private SelectJoinStep<org.jooq.Record> refJoins(
} else {
Column column = getColumnByName(table, filter.getColumn());
if (column.isReference() && !filter.getSubfilters().isEmpty()) {
String subAlias = tableAlias + "-" + column.getName();
String subAlias = tableAlias + SUBSELECT_SEPARATOR + column.getName();
if (!aliasList.contains(subAlias)) {
// to ensure only join once
aliasList.add(subAlias);
Expand All @@ -955,7 +955,7 @@ private SelectJoinStep<org.jooq.Record> refJoins(
// then do same as above
Column column = getColumnByName(table, select.getColumn());
if (column.isReference()) {
String subAlias = tableAlias + "-" + column.getName();
String subAlias = tableAlias + SUBSELECT_SEPARATOR + column.getName();
// only join if subselection extists
if (!aliasList.contains(subAlias) && !select.getSubselect().isEmpty()) {
aliasList.add(subAlias);
Expand Down Expand Up @@ -1109,7 +1109,7 @@ private Condition whereConditionsFilter(TableMetadata table, String tableAlias,
if (column.isReference()) {
conditions.add(
whereConditionsFilter(
column.getRefTable(), tableAlias + "-" + column.getName(), subfilter));
column.getRefTable(), tableAlias + SUBSELECT_SEPARATOR + column.getName(), subfilter));
} else if (column.isFile()) {
Filter sub = filters.getSubfilter("id");
if (sub != null && EQUALS.equals(sub.getOperator())) {
Expand Down
12 changes: 9 additions & 3 deletions backend/molgenis-emx2/src/main/java/org/molgenis/emx2/Column.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,6 @@ public Boolean isArray() {

/** will return self in case of single, and multiple in case of composite key wrapper */
public List<Reference> getReferences() {

// no ref
if (getRefTableName() == null) {
throw new MolgenisException(
Expand All @@ -461,6 +460,13 @@ public List<Reference> getReferences() {
+ " fails because that table has no primary key");
}

// Defines separator to be used.
// Default is COMPOSITE_REF_SEPARATOR, though REFBACK requires use of a subselect so to ensure
// function returns valid column name for a basic REFBACK to a composite key, overrides default
// behavior.
String separator =
(getColumnType().isRefback() ? SUBSELECT_SEPARATOR : COMPOSITE_REF_SEPARATOR);

// create the refs
Column refLink = getRefLinkColumn();
for (Column keyPart : pkeys) {
Expand All @@ -484,7 +490,7 @@ public List<Reference> getReferences() {
if (name == null) {
name = getName();
if (pkeys.size() > 1) {
name += COMPOSITE_REF_SEPARATOR + ref.getName();
name += separator + ref.getName();
}
}
refColumns.add(
Expand All @@ -511,7 +517,7 @@ public List<Reference> getReferences() {
// create the ref
String name = getName();
if (pkeys.size() > 1) {
name += COMPOSITE_REF_SEPARATOR + keyPart.getName();
name += separator + keyPart.getName();
}
refColumns.add(
new Reference(
Expand Down
1 change: 1 addition & 0 deletions backend/molgenis-emx2/src/main/java/org/molgenis/emx2/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public class Constants {
public static final String MG_USER_PREFIX = "MG_USER_";

public static final String COMPOSITE_REF_SEPARATOR = ".";
public static final String SUBSELECT_SEPARATOR = "-";
public static final String REF_LINK = "refLink";
public static final String REF_LABEL = "refLabel";
public static final String REF_LABEL_DEFAULT = "refLabelDefault";
Expand Down
29 changes: 29 additions & 0 deletions backend/molgenis-emx2/src/test/java/org/molgenis/emx2/TestColumn.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package org.molgenis.emx2;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.util.List;
import org.junit.jupiter.api.Test;

public class TestColumn {
Expand Down Expand Up @@ -49,4 +52,30 @@ public void validColumnName() {
new Column(
"a234567890123456789012345678901234567890123456789012345678901234")));
}

@Test
void testCompositeKeySeparator() {
Column pk1 = new Column("id1").setType(ColumnType.STRING);
Column pk2 = new Column("id2").setType(ColumnType.STRING);
List<Column> primaryKeys = List.of(pk1, pk2);

TableMetadata tableMetadata = mock(TableMetadata.class);
when(tableMetadata.getPrimaryKeyColumns()).thenReturn(primaryKeys);

Column column = mock(Column.class);
when(column.getReferences()).thenCallRealMethod();
when(column.getRefTableName()).thenReturn("MyRefTable");
when(column.getRefTable()).thenReturn(tableMetadata);
when(column.getRefLinkColumn()).thenReturn(null);
when(column.getName()).thenReturn("MyColname");

when(column.getColumnType()).thenReturn(ColumnType.REF);
List<String> actualRefs = column.getReferences().stream().map(Reference::getName).toList();
when(column.getColumnType()).thenReturn(ColumnType.REFBACK);
List<String> actualRefbacks = column.getReferences().stream().map(Reference::getName).toList();

assertAll(
() -> assertEquals(List.of("MyColname.id1", "MyColname.id2"), actualRefs),
() -> assertEquals(List.of("MyColname-id1", "MyColname-id2"), actualRefbacks));
}
}
Loading