This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/cayenne.git
commit 76d8e97b6113f136b629e108a366099c467c5b4f Author: Andrus Adamchik <[email protected]> AuthorDate: Mon Jul 13 18:42:42 2026 -0400 CAY-2977 DBImport confused by multi-key relationships post-cleanup --- RELEASE-NOTES.txt | 1 + .../dbsync/reverse/dbload/DbLoadDataStore.java | 45 ++--- .../cayenne/dbsync/reverse/dbload/ExportedKey.java | 209 +++------------------ .../dbsync/reverse/dbload/ExportedKeyLoader.java | 14 +- .../dbsync/reverse/dbload/ExportedKeySide.java | 81 ++++++++ .../dbsync/reverse/dbload/RelationshipLoader.java | 14 +- .../dbsync/reverse/dbload/ExportedKeyLoaderIT.java | 10 +- .../dbsync/reverse/dbload/ExportedKeyTest.java | 8 +- 8 files changed, 151 insertions(+), 231 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 02a53f58a..20e6ea637 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -40,6 +40,7 @@ CAY-2967 SQLTemplate/SQLSelect broken pagination CAY-2968 Vertical Inheritance: INSERT instead of UPDATE after updating flattened attribute CAY-2973 Exception trying to copy/paste a callback CAY-2976 Exception creating a relationship for an Incomplete ObjEntity +CAY-2977 DbImport confused by multi-key relationships ---------------------------------- Release: 5.0-M2 diff --git a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/DbLoadDataStore.java b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/DbLoadDataStore.java index 422f96f08..f6cb26aab 100644 --- a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/DbLoadDataStore.java +++ b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/DbLoadDataStore.java @@ -19,34 +19,32 @@ package org.apache.cayenne.dbsync.reverse.dbload; +import org.apache.cayenne.dbsync.model.DetectedDbEntity; +import org.apache.cayenne.map.DataMap; +import org.apache.cayenne.map.DbEntity; +import org.apache.cayenne.map.Procedure; + import java.util.HashMap; import java.util.Map; import java.util.Set; import java.util.TreeSet; -import org.apache.cayenne.map.DataMap; -import org.apache.cayenne.map.DbEntity; -import org.apache.cayenne.dbsync.model.DetectedDbEntity; -import org.apache.cayenne.map.Procedure; - /** - * Temporary storage for loaded from DB DbEntities and Procedures. - * DataMap is used but it's functionality is excessive and - * there can be unwanted side effects. - * But we can't get rid of it right now as parallel data structure - * for dbEntity, attributes, procedures etc.. must be created - * or some other work around should be implemented because - * some functionality relies on side effects (e.g. entity resolution - * in relationship) + * Temporary storage for loaded from DB DbEntities and Procedures. DataMap is used, but its functionality is excessive + * and there can be unwanted side effects. */ +// TODO: We can't get rid of it right now as parallel data structure for dbEntity, attributes, procedures etc. must be +// created or some other work around should be implemented because some functionality relies on side effects (e.g. +// entity resolution in relationship) public class DbLoadDataStore extends DataMap { - private Map<String, Set<ExportedKey>> exportedKeys = new HashMap<>(); - - private Map<String, DbEntity> upperCaseNames = new HashMap<>(); + private final Map<String, Set<ExportedKey>> exportedKeys; + private final Map<String, DbEntity> upperCaseNames; DbLoadDataStore() { super("__generated_by_dbloader__"); + exportedKeys = new HashMap<>(); + upperCaseNames = new HashMap<>(); } @Override @@ -56,7 +54,7 @@ public class DbLoadDataStore extends DataMap { @Override public void addDbEntity(DbEntity entity) { - if(!(entity instanceof DetectedDbEntity)) { + if (!(entity instanceof DetectedDbEntity)) { throw new IllegalArgumentException("Only DetectedDbEntity can be inserted in this map"); } super.addDbEntity(entity); @@ -64,11 +62,11 @@ public class DbLoadDataStore extends DataMap { } DbEntity addDbEntitySafe(DbEntity entity) { - if(!(entity instanceof DetectedDbEntity)) { + if (!(entity instanceof DetectedDbEntity)) { throw new IllegalArgumentException("Only DetectedDbEntity can be inserted in this map"); } DbEntity old = getDbEntity(entity.getName()); - if(old != null) { + if (old != null) { removeDbEntity(old.getName()); } addDbEntity(entity); @@ -77,7 +75,7 @@ public class DbLoadDataStore extends DataMap { void addProcedureSafe(Procedure procedure) { Procedure old = getProcedure(procedure.getName()); - if(old != null) { + if (old != null) { removeProcedure(old.getName()); } addProcedure(procedure); @@ -85,12 +83,7 @@ public class DbLoadDataStore extends DataMap { void addExportedKey(ExportedKey key) { // group by the FK constraint, so that all columns of a multi-column FK end up in a single relationship - Set<ExportedKey> exportedKeys = this.exportedKeys.get(key.getGroupKey()); - if (exportedKeys == null) { - exportedKeys = new TreeSet<>(); - this.exportedKeys.put(key.getGroupKey(), exportedKeys); - } - exportedKeys.add(key); + exportedKeys.computeIfAbsent(key.groupKey(), k -> new TreeSet<>()).add(key); } Set<Map.Entry<String, Set<ExportedKey>>> getExportedKeysEntrySet() { diff --git a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKey.java b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKey.java index 2ca48a9a7..478beb412 100644 --- a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKey.java +++ b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKey.java @@ -19,79 +19,41 @@ package org.apache.cayenne.dbsync.reverse.dbload; +import org.apache.cayenne.util.CompareToBuilder; + import java.sql.ResultSet; import java.sql.SQLException; import java.util.Objects; -import org.apache.cayenne.map.DbEntity; -import org.apache.cayenne.util.CompareToBuilder; -import org.apache.cayenne.util.Util; - /** * A representation of relationship between two tables in database. It can be used for creating names * for relationships. * * @since 4.0 */ -public class ExportedKey implements Comparable<ExportedKey> { - - private final KeyData pk; - private final KeyData fk; - private final short keySeq; +public record ExportedKey(ExportedKeySide pk, ExportedKeySide fk, short keySeq) implements Comparable<ExportedKey> { /** - * Extracts data from a resultset pointing to a exported key to - * ExportedKey class instance + * Extracts data from a resultset pointing to an exported key into an ExportedKey instance. * - * @param rs ResultSet pointing to a exported key, fetched using - * DataBaseMetaData.getExportedKeys(...) + * @param rs ResultSet pointing to an exported key, fetched using DataBaseMetaData.getExportedKeys(...) */ - ExportedKey(ResultSet rs) throws SQLException { - String pkCatalog = rs.getString("PKTABLE_CAT"); - String pkSchema = rs.getString("PKTABLE_SCHEM"); - String pkTable = rs.getString("PKTABLE_NAME"); - String pkColumn = rs.getString("PKCOLUMN_NAME"); - String pkName = rs.getString("PK_NAME"); - pk = new KeyData(pkCatalog, pkSchema, pkTable, pkColumn, pkName); - - String fkCatalog = rs.getString("FKTABLE_CAT"); - String fkSchema = rs.getString("FKTABLE_SCHEM"); - String fkTable = rs.getString("FKTABLE_NAME"); - String fkColumn = rs.getString("FKCOLUMN_NAME"); - String fkName = rs.getString("FK_NAME"); - fk = new KeyData(fkCatalog, fkSchema, fkTable, fkColumn, fkName); - - keySeq = rs.getShort("KEY_SEQ"); - } - - public KeyData getPk() { - return pk; - } - - public KeyData getFk() { - return fk; - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (obj == this) { - return true; - } - if (obj.getClass() != getClass()) { - return false; - } - ExportedKey rhs = (ExportedKey) obj; - return Objects.equals(pk, rhs.pk) - && Objects.equals(fk, rhs.fk) - && keySeq == rhs.keySeq; - } - - @Override - public int hashCode() { - return Objects.hash(pk, fk, keySeq); + static ExportedKey fromResultSet(ResultSet rs) throws SQLException { + ExportedKeySide pk = new ExportedKeySide( + rs.getString("PKTABLE_CAT"), + rs.getString("PKTABLE_SCHEM"), + rs.getString("PKTABLE_NAME"), + rs.getString("PKCOLUMN_NAME"), + rs.getString("PK_NAME")); + + ExportedKeySide fk = new ExportedKeySide( + rs.getString("FKTABLE_CAT"), + rs.getString("FKTABLE_SCHEM"), + rs.getString("FKTABLE_NAME"), + rs.getString("FKCOLUMN_NAME"), + rs.getString("FK_NAME")); + + return new ExportedKey(pk, fk, rs.getShort("KEY_SEQ")); } @Override @@ -109,133 +71,16 @@ public class ExportedKey implements Comparable<ExportedKey> { @Override public String toString() { - return getStrKey() + " # " + keySeq; - } - - String getStrKey() { - return pk + " <- " + fk; + return fk + " -> " + pk + " # " + keySeq; } /** * Returns a key that identifies the single FK constraint this row belongs to, so that all columns of a - * multi-column FK are grouped into one relationship. Uses the FK constraint name (FK_NAME) reported by the - * driver; falls back to the per-column {@link #getStrKey()} when the name is unavailable, preserving the - * historical behavior for drivers that don't report constraint names. + * multi-column FK are grouped into one relationship. */ - String getGroupKey() { - String fkName = fk.getName(); - if (Util.isEmptyString(fkName)) { - return getStrKey(); - } - return fk.getCatalog() + "." + fk.getSchema() + "." + fk.getTable() + "." + fkName - + " -> " + pk.getCatalog() + "." + pk.getSchema() + "." + pk.getTable(); - } - - public static class KeyData implements Comparable<KeyData> { - private final String catalog; - private final String schema; - private final String table; - private final String column; - private final String name; - - KeyData(String catalog, String schema, String table, String column, String name) { - this.catalog = catalog; - this.schema = schema; - this.table = table; - this.column = column; - this.name = name; - } - - public String getCatalog() { - return catalog; - } - - public String getSchema() { - return schema; - } - - public String getTable() { - return table; - } - - public String getColumn() { - return column; - } - - public String getName() { - return name; - } - - @Override - public String toString() { - return catalog + "." + schema + "." + table + "." + column; - } - - @Override - public int compareTo(KeyData rhs) { - Objects.requireNonNull(rhs); - if (rhs == this) { - return 0; - } - - return new CompareToBuilder() - .append(catalog, rhs.catalog) - .append(schema, rhs.schema) - .append(table, rhs.table) - .append(column, rhs.column) - .append(name, rhs.name) - .toComparison(); - } - - @Override - public boolean equals(Object obj) { - if (obj == null) { - return false; - } - if (obj == this) { - return true; - } - if (obj.getClass() != getClass()) { - return false; - } - KeyData rhs = (KeyData) obj; - return Objects.equals(catalog, rhs.catalog) - && Objects.equals(schema, rhs.schema) - && Objects.equals(table, rhs.table) - && Objects.equals(column, rhs.column) - && Objects.equals(name, rhs.name); - } - - @Override - public int hashCode() { - return Objects.hash(catalog, schema, table, column, name); - } - - /** - * Validate that entity is for this key (exists and has same catalog/schema) - * @param entity to validate - * @return is entity matches for this key - */ - public boolean validateEntity(DbEntity entity) { - if (entity == null) { - return false; - } - - if(Util.isEmptyString(catalog)) { - if(!Util.isEmptyString(entity.getCatalog())) { - return false; - } - } else { - if(!catalog.equals(entity.getCatalog())) { - return false; - } - } - - if(Util.isEmptyString(schema)) { - return Util.isEmptyString(entity.getSchema()); - } else { - return schema.equals(entity.getSchema()); - } - } + public String groupKey() { + return "%s.%s.%s.%s -> %s.%s.%s".formatted( + fk.catalog(), fk.schema(), fk.table(), fk.name(), + pk.catalog(), pk.schema(), pk.table()); } } diff --git a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyLoader.java b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyLoader.java index 8d6db95ec..5fc254cbf 100644 --- a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyLoader.java +++ b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyLoader.java @@ -57,17 +57,17 @@ class ExportedKeyLoader extends PerEntityLoader { @Override void processResultSet(DbEntity dbEntity, DbLoadDataStore map, ResultSet rs) throws SQLException { - ExportedKey key = new ExportedKey(rs); + ExportedKey key = ExportedKey.fromResultSet(rs); - DbEntity pkEntity = map.getDbEntity(key.getPk().getTable()); - if (!key.getPk().validateEntity(pkEntity)) { - LOGGER.info("Skip relation: '{}' because table '{}' is not found or in different catalog/schema", key, key.getPk().getTable()); + DbEntity pkEntity = map.getDbEntity(key.pk().table()); + if (!key.pk().validateEntity(pkEntity)) { + LOGGER.info("Skip relation: '{}' because table '{}' is not found or in different catalog/schema", key, key.pk().table()); return; } - DbEntity fkEntity = map.getDbEntity(key.getFk().getTable()); - if (!key.getFk().validateEntity(fkEntity)) { - LOGGER.info("Skip relation: '{}' because table '{}' is not found or in different catalog/schema", key, key.getFk().getTable()); + DbEntity fkEntity = map.getDbEntity(key.fk().table()); + if (!key.fk().validateEntity(fkEntity)) { + LOGGER.info("Skip relation: '{}' because table '{}' is not found or in different catalog/schema", key, key.fk().table()); return; } diff --git a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeySide.java b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeySide.java new file mode 100644 index 000000000..993e7897a --- /dev/null +++ b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeySide.java @@ -0,0 +1,81 @@ +/***************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + ****************************************************************/ + +package org.apache.cayenne.dbsync.reverse.dbload; + +import org.apache.cayenne.map.DbEntity; +import org.apache.cayenne.util.CompareToBuilder; +import org.apache.cayenne.util.Util; + +import java.util.Objects; + +/** + * One side (PK or FK) of an {@link ExportedKey} row. + * + * @since 5.0 + */ +public record ExportedKeySide(String catalog, String schema, String table, String column, String name) + implements Comparable<ExportedKeySide> { + + @Override + public String toString() { + return catalog + "." + schema + "." + table + "." + column; + } + + @Override + public int compareTo(ExportedKeySide rhs) { + Objects.requireNonNull(rhs); + if (rhs == this) { + return 0; + } + + return new CompareToBuilder() + .append(catalog, rhs.catalog) + .append(schema, rhs.schema) + .append(table, rhs.table) + .append(column, rhs.column) + .append(name, rhs.name) + .toComparison(); + } + + /** + * Validate that entity is for this key (exists and has same catalog/schema) + */ + public boolean validateEntity(DbEntity entity) { + if (entity == null) { + return false; + } + + if (Util.isEmptyString(catalog)) { + if (!Util.isEmptyString(entity.getCatalog())) { + return false; + } + } else { + if (!catalog.equals(entity.getCatalog())) { + return false; + } + } + + if (Util.isEmptyString(schema)) { + return Util.isEmptyString(entity.getSchema()); + } else { + return schema.equals(entity.getSchema()); + } + } +} diff --git a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/RelationshipLoader.java b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/RelationshipLoader.java index 855515729..76e3da376 100644 --- a/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/RelationshipLoader.java +++ b/cayenne-dbsync/src/main/java/org/apache/cayenne/dbsync/reverse/dbload/RelationshipLoader.java @@ -60,10 +60,10 @@ public class RelationshipLoader extends AbstractLoader { throw new IllegalStateException(); } - ExportedKey.KeyData PK = key.getPk(); - ExportedKey.KeyData FK = key.getFk(); - DbEntity pkEntity = map.getDbEntity(PK.getTable()); - DbEntity fkEntity = map.getDbEntity(FK.getTable()); + ExportedKeySide PK = key.pk(); + ExportedKeySide FK = key.fk(); + DbEntity pkEntity = map.getDbEntity(PK.table()); + DbEntity fkEntity = map.getDbEntity(FK.table()); if (pkEntity == null || fkEntity == null) { // Check for existence of this entities were made in creation of ExportedKey throw new IllegalStateException(); @@ -78,7 +78,7 @@ public class RelationshipLoader extends AbstractLoader { // TODO: dirty and non-transparent... using DbRelationshipDetected for the benefit of the merge package. // This info is available from joins.... DbRelationshipDetected reverseRelationship = new DbRelationshipDetected(); - reverseRelationship.setFkName(FK.getName()); + reverseRelationship.setFkName(FK.name()); reverseRelationship.setSourceEntity(fkEntity); reverseRelationship.setTargetEntityName(pkEntity); reverseRelationship.setToMany(false); @@ -163,8 +163,8 @@ public class RelationshipLoader extends AbstractLoader { for (ExportedKey exportedKey : exportedKeys) { // Create and append joins - String pkName = exportedKey.getPk().getColumn(); - String fkName = exportedKey.getFk().getColumn(); + String pkName = exportedKey.pk().column(); + String fkName = exportedKey.fk().column(); // skip invalid joins... DbAttribute pkAtt = pkEntity.getAttribute(pkName); diff --git a/cayenne-dbsync/src/test/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyLoaderIT.java b/cayenne-dbsync/src/test/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyLoaderIT.java index 84ad8fb8b..7f2d188c0 100644 --- a/cayenne-dbsync/src/test/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyLoaderIT.java +++ b/cayenne-dbsync/src/test/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyLoaderIT.java @@ -81,17 +81,17 @@ public class ExportedKeyLoaderIT extends BaseLoaderIT { ExportedKey artistIdFk = findArtistExportedKey(); assertNotNull(artistIdFk); - assertEquals("ARTIST", artistIdFk.getPk().getTable().toUpperCase()); - assertEquals("ARTIST_ID", artistIdFk.getPk().getColumn().toUpperCase()); + assertEquals("ARTIST", artistIdFk.pk().table().toUpperCase()); + assertEquals("ARTIST_ID", artistIdFk.pk().column().toUpperCase()); - assertEquals("PAINTING", artistIdFk.getFk().getTable().toUpperCase()); - assertEquals("ARTIST_ID", artistIdFk.getFk().getColumn().toUpperCase()); + assertEquals("PAINTING", artistIdFk.fk().table().toUpperCase()); + assertEquals("ARTIST_ID", artistIdFk.fk().column().toUpperCase()); } private ExportedKey findArtistExportedKey() { for(Map.Entry<String, Set<ExportedKey>> entry : store.getExportedKeysEntrySet()) { ExportedKey key = entry.getValue().iterator().next(); - if("ARTIST_ID".equalsIgnoreCase(key.getFk().getColumn())) { + if("ARTIST_ID".equalsIgnoreCase(key.fk().column())) { return key; } } diff --git a/cayenne-dbsync/src/test/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyTest.java b/cayenne-dbsync/src/test/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyTest.java index 28de385bc..9aa8c49be 100644 --- a/cayenne-dbsync/src/test/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyTest.java +++ b/cayenne-dbsync/src/test/java/org/apache/cayenne/dbsync/reverse/dbload/ExportedKeyTest.java @@ -34,8 +34,8 @@ public class ExportedKeyTest { @Test public void equalsKeyData() throws SQLException { - ExportedKey.KeyData keyData1 = new ExportedKey.KeyData("Catalog", null, "Table", "Column", "Name"); - ExportedKey.KeyData keyData2 = new ExportedKey.KeyData("Catalog", null, "Table", "Column", "Name"); + ExportedKeySide keyData1 = new ExportedKeySide("Catalog", null, "Table", "Column", "Name"); + ExportedKeySide keyData2 = new ExportedKeySide("Catalog", null, "Table", "Column", "Name"); assertTrue(keyData1.equals(keyData2)); assertTrue(keyData2.equals(keyData1)); @@ -60,7 +60,7 @@ public class ExportedKeyTest { when(rs1.getShort("KEY_SEQ")).thenReturn((short) 1); - ExportedKey keyData1 = new ExportedKey(rs1); + ExportedKey keyData1 = ExportedKey.fromResultSet(rs1); ResultSet rs2 = mock(ResultSet.class); when(rs2.getString("PKTABLE_CAT")).thenReturn("PKCatalog"); @@ -77,7 +77,7 @@ public class ExportedKeyTest { when(rs2.getShort("KEY_SEQ")).thenReturn((short)1); - ExportedKey keyData2 = new ExportedKey(rs2); + ExportedKey keyData2 = ExportedKey.fromResultSet(rs2); assertTrue(keyData1.equals(keyData2)); assertTrue(keyData2.equals(keyData1));
