#2186 - Invalid generated table and column names of @ManyToMany relation with allQuotedIdentifiers=true

ebean-api/src/main/java/io/ebean/config/AbstractNamingConvention.java

@@ -7,6 +7,8 @@ import javax.persistence.DiscriminatorValue;
 import javax.persistence.Inheritance;
 import javax.persistence.Table;
 
+import static io.ebean.util.StringHelper.isNull;
+
 /**
  * Provides some base implementation for NamingConventions.
  *
@@ -78,10 +80,16 @@ public abstract class AbstractNamingConvention implements NamingConvention {
 
   @Override
   public String getSequenceName(String rawTableName, String pkColumn) {
-    final String tableNameUnquoted = databasePlatform.unQuote(rawTableName);
+    TableName tableName = new TableName(rawTableName);
+    String seqName = seqName(pkColumn, tableName.getName());
+    return tableName.withCatalogAndSchema(seqName);
+  }
+
+  private String seqName(String pkColumn, String tableName) {
+    final String tableNameUnquoted = unQuote(tableName);
     String seqName = sequenceFormat.replace("{table}", tableNameUnquoted);
-    pkColumn = (pkColumn == null) ? "" : databasePlatform.unQuote(pkColumn);
-    return seqName.replace("{column}", pkColumn);
+    pkColumn = (pkColumn == null) ? "" : unQuote(pkColumn);
+    return quoteIdentifiers(seqName.replace("{column}", pkColumn));
   }
 
   /**
@@ -216,15 +224,13 @@ public abstract class AbstractNamingConvention implements NamingConvention {
         || AnnotationUtil.has(supCls, DiscriminatorValue.class);
   }
 
-
   @Override
   public TableName getM2MJoinTableName(TableName lhsTable, TableName rhsTable) {
-
     StringBuilder buffer = new StringBuilder();
-    buffer.append(lhsTable.getName());
+    buffer.append(unQuote(lhsTable.getName()));
     buffer.append("_");
 
-    String rhsTableName = rhsTable.getName();
+    String rhsTableName = unQuote(rhsTable.getName());
     if (rhsTableName.indexOf('_') < rhsPrefixLength) {
       // trim off a xx_ prefix if there is one
       rhsTableName = rhsTableName.substring(rhsTableName.indexOf('_') + 1);
@@ -238,7 +244,13 @@ public abstract class AbstractNamingConvention implements NamingConvention {
       buffer.setLength(maxTableNameLength);
     }
 
-    return new TableName(lhsTable.getCatalog(), lhsTable.getSchema(), buffer.toString());
+    String tableName = quoteIdentifiers(buffer.toString());
+    return new TableName(lhsTable.getCatalog(), lhsTable.getSchema(), tableName);
+  }
+
+  @Override
+  public String deriveM2MColumn(String tableName, String dbColumn) {
+    return quoteIdentifiers(unQuote(tableName) +"_" + unQuote(dbColumn));
   }
 
   /**
@@ -255,14 +267,29 @@ public abstract class AbstractNamingConvention implements NamingConvention {
     return null;
   }
 
+  @Override
+  public String getTableName(String catalog, String schema, String name) {
+    StringBuilder sb = new StringBuilder();
+    if (!isNull(catalog)) {
+      sb.append(quoteIdentifiers(catalog)).append(".");
+    }
+    if (!isNull(schema)) {
+      sb.append(quoteIdentifiers(schema)).append(".");
+    }
+    return sb.append(quoteIdentifiers(name)).toString();
+  }
+
   /**
-   * Replace back ticks (if they are used) with database platform specific
-   * quoted identifiers.
+   * Replace back ticks (if they are used) with database platform specific quoted identifiers.
    */
   protected String quoteIdentifiers(String s) {
     return databasePlatform.convertQuotedIdentifiers(s);
   }
 
+  private String unQuote(String val) {
+    return databasePlatform.unQuote(val);
+  }
+
   /**
    * Checks string is null or empty .
    */

ebean-api/src/main/java/io/ebean/config/MatchingNamingConvention.java

@@ -39,11 +39,6 @@ public class MatchingNamingConvention extends AbstractNamingConvention {
     return new TableName(quoteIdentifiers(getCatalog()), quoteIdentifiers(getSchema()), quoteIdentifiers(beanClass.getSimpleName()));
   }
 
-  @Override
-  public String getPropertyFromColumn(Class<?> beanClass, String dbColumnName) {
-    return dbColumnName;
-  }
-
   @Override
   public String getForeignKey(String prefix, String fkProperty) {
     prefix = databasePlatform.unQuote(prefix);

ebean-api/src/main/java/io/ebean/config/NamingConvention.java

@@ -53,6 +53,16 @@ public interface NamingConvention {
    */
   TableName getM2MJoinTableName(TableName lhsTable, TableName rhsTable);
 
+  /**
+   * Derive a DB Column from a FK table and column.
+   */
+  String deriveM2MColumn(String tableName, String dbColumn);
+
+  /**
+   * Return the full table name taking into account quoted identifiers.
+   */
+  String getTableName(String catalog, String schema, String name);
+
   /**
    * Return the column name given the property name.
    *
@@ -60,18 +70,6 @@ public interface NamingConvention {
    */
   String getColumnFromProperty(Class<?> beanClass, String propertyName);
 
-  /**
-   * Return the property name from the column name.
-   * <p>
-   * This is used to help mapping of raw SQL queries onto bean properties.
-   * </p>
-   *
-   * @param beanClass    the bean class
-   * @param dbColumnName the db column name
-   * @return the property name from the column name
-   */
-  String getPropertyFromColumn(Class<?> beanClass, String dbColumnName);
-
   /**
    * Return the sequence name given the table name (for DB's that use sequences).
    * <p>

ebean-api/src/main/java/io/ebean/config/TableName.java

@@ -20,7 +20,7 @@ public final class TableName {
   /**
    * The name.
    */
-  private String name;
+  private final String name;
 
   /**
    * Construct with the given catalog schema and table name.
@@ -29,7 +29,6 @@ public final class TableName {
    * </p>
    */
   public TableName(String catalog, String schema, String name) {
-    super();
     this.catalog = catalog != null ? catalog.trim() : null;
     this.schema = schema != null ? schema.trim() : null;
     this.name = name != null ? name.trim() : null;
@@ -110,14 +109,11 @@ public final class TableName {
    * @return the qualified name
    */
   public String getQualifiedName() {
-
     StringBuilder buffer = new StringBuilder();
-
     // Add catalog
     if (catalog != null) {
       buffer.append(catalog);
     }
-
     // Add schema
     if (schema != null) {
       if (buffer.length() > 0) {
@@ -125,31 +121,27 @@ public final class TableName {
       }
       buffer.append(schema);
     }
-
     if (buffer.length() > 0) {
       buffer.append(".");
     }
-    buffer.append(name);
-
-    return buffer.toString();
+    return buffer.append(name).toString();
   }
 
   /**
    * Append a catalog and schema prefix if they exist to the string builder.
    */
-  public void appendCatalogAndSchema(StringBuilder buffer) {
-    if (catalog != null) {
-      buffer.append(catalog).append(".");
-    }
+  public String withCatalogAndSchema(String name) {
     if (schema != null) {
-      buffer.append(schema).append(".");
+      name = schema + "." + name;
     }
+    if (catalog != null) {
+      name = catalog + "." + name;
+    }
+    return name;
   }
 
   /**
    * Checks if is table name is valid i.e. it has at least a name.
-   *
-   * @return true, if is valid
    */
   public boolean isValid() {
     return name != null && !name.isEmpty();

ebean-api/src/main/java/io/ebean/config/UnderscoreNamingConvention.java

@@ -60,18 +60,6 @@ public class UnderscoreNamingConvention extends AbstractNamingConvention {
     return toUnderscoreFromCamel(propertyName);
   }
 
-  /**
-   * Converts underscore based column name to Camel case property name.
-   *
-   * @param beanClass    the bean class
-   * @param dbColumnName the db column name
-   * @return the property from column
-   */
-  @Override
-  public String getPropertyFromColumn(Class<?> beanClass, String dbColumnName) {
-    return toCamelFromUnderscore(dbColumnName);
-  }
-
   /**
    * Return true if the result will be upper case.
    * <p>

ebean-core/src/main/java/io/ebeaninternal/server/deploy/parse/AnnotationAssocManys.java

@@ -38,8 +38,6 @@ import javax.persistence.OrderBy;
 import javax.persistence.OrderColumn;
 import java.util.Set;
 
-import static io.ebean.util.StringHelper.isNull;
-
 /**
  * Read the deployment annotation for Assoc Many beans.
  */
@@ -346,18 +344,6 @@ class AnnotationAssocManys extends AnnotationAssoc {
     return append(joinTable.catalog(), joinTable.schema(), joinTable.name());
   }
 
-  private String append(String catalog, String schema, String name) {
-    StringBuilder sb = new StringBuilder();
-    if (!isNull(catalog)) {
-      sb.append(catalog).append(".");
-    }
-    if (!isNull(schema)) {
-      sb.append(schema).append(".");
-    }
-    sb.append(name);
-    return sb.toString();
-  }
-
   /**
    * Return the full table name
    */
@@ -368,6 +354,13 @@ class AnnotationAssocManys extends AnnotationAssoc {
     return append(collectionTable.catalog(), collectionTable.schema(), collectionTable.name());
   }
 
+  /**
+   * Return the full table name taking into account quoted identifiers.
+   */
+  private String append(String catalog, String schema, String name) {
+    return namingConvention.getTableName(catalog, schema, name);
+  }
+
   /**
    * Define intersection table and foreign key columns for ManyToMany.
    * <p>
@@ -414,8 +407,8 @@ class AnnotationAssocManys extends AnnotationAssoc {
       BeanProperty localId = localTable.getIdProperty();
       if (localId != null) {
         // add the source to intersection join columns
-        String fkCol = localTableName + "_" + localId.getDbColumn();
-        intJoin.addJoinColumn(new DeployTableJoinColumn(localId.getDbColumn(), namingConvention.getColumnFromProperty(null, fkCol)));
+        String fkCol = namingConvention.deriveM2MColumn(localTableName, localId.getDbColumn());
+        intJoin.addJoinColumn(new DeployTableJoinColumn(localId.getDbColumn(), fkCol));
       }
     }
 
@@ -424,8 +417,8 @@ class AnnotationAssocManys extends AnnotationAssoc {
       BeanProperty otherId = otherTable.getIdProperty();
       if (otherId != null) {
         // set the intersection to dest table join columns
-        final String fkCol = otherTableName + "_" + otherId.getDbColumn();
-        destJoin.addJoinColumn(new DeployTableJoinColumn(namingConvention.getColumnFromProperty(null, fkCol), otherId.getDbColumn()));
+        String fkCol = namingConvention.deriveM2MColumn(otherTableName, otherId.getDbColumn());
+        destJoin.addJoinColumn(new DeployTableJoinColumn(fkCol, otherId.getDbColumn()));
       }
     }
 

ebean-core/src/test/java/io/ebean/config/MatchingNamingConventionTest.java

@@ -11,7 +11,7 @@ import static org.junit.Assert.assertNull;
 
 public class MatchingNamingConventionTest {
 
-  private MatchingNamingConvention namingConvention;
+  private final MatchingNamingConvention namingConvention;
 
   public MatchingNamingConventionTest() {
     this.namingConvention = new MatchingNamingConvention();
@@ -30,6 +30,50 @@ public class MatchingNamingConventionTest {
     return nc;
   }
 
+  @Test
+  public void getTableName() {
+    assertThat(namingConvention.getTableName("a", "b", "c")).isEqualTo("a.b.c");
+    assertThat(namingConvention.getTableName("", "b", "c")).isEqualTo("b.c");
+    assertThat(namingConvention.getTableName("", "", "c")).isEqualTo("c");
+    assertThat(namingConvention.getTableName("a", "", "c")).isEqualTo("a.c");
+  }
+
+  @Test
+  public void getTableName_when_allQuoted() {
+    MatchingNamingConvention nc = createMatchingNamingConventionAllQuoted();
+
+    assertThat(nc.getTableName("a", "b", "c")).isEqualTo("[a].[b].[c]");
+    assertThat(nc.getTableName("", "b", "c")).isEqualTo("[b].[c]");
+    assertThat(nc.getTableName("", "", "c")).isEqualTo("[c]");
+    assertThat(nc.getTableName("a", "", "c")).isEqualTo("[a].[c]");
+  }
+
+  @Test
+  public void getM2MJoinTableName() {
+    TableName t0 = new TableName("One");
+    TableName t1 = new TableName("Two");
+    assertThat(namingConvention.getM2MJoinTableName(t0, t1).toString()).isEqualTo("One_Two");
+  }
+
+  @Test
+  public void getM2MJoinTableName_when_allQuoted() {
+    MatchingNamingConvention nc = createMatchingNamingConventionAllQuoted();
+    TableName t0 = new TableName("[One]");
+    TableName t1 = new TableName("[Two]");
+    assertThat(nc.getM2MJoinTableName(t0, t1).toString()).isEqualTo("[One_Two]");
+  }
+
+  @Test
+  public void deriveM2MColumn() {
+    assertThat(namingConvention.deriveM2MColumn("One", "Two")).isEqualTo("One_Two");
+  }
+
+  @Test
+  public void deriveM2MColumn_when_allQuoted() {
+    MatchingNamingConvention nc = createMatchingNamingConventionAllQuoted();
+    assertThat(nc.deriveM2MColumn("[One]", "[Two]")).isEqualTo("[One_Two]");
+  }
+
   @Test
   public void getColumnFromProperty_when_allQuoted() {
 
@@ -50,11 +94,16 @@ public class MatchingNamingConventionTest {
     assertNull(tableName.getSchema());
   }
 
-
   @Test
   public void getSequenceName() {
     MatchingNamingConvention nc = createMatchingNamingConventionAllQuoted();
-    assertEquals("Customer_seq", nc.getSequenceName("[Customer]", null));
+    assertThat(nc.getSequenceName("[Customer]", null)).isEqualTo("[Customer_seq]");
+  }
+
+  @Test
+  public void getSequenceName_when_quotedSchema() {
+    MatchingNamingConvention nc = createMatchingNamingConventionAllQuoted();
+    assertThat(nc.getSequenceName("[dbo].[Customer]", null)).isEqualTo("[dbo].[Customer_seq]");
   }
 
   @Test

ebean-core/src/test/java/org/tests/config/TestTableName.java

@@ -5,8 +5,28 @@ import io.ebean.config.TableName;
 import org.junit.Assert;
 import org.junit.Test;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 public class TestTableName extends BaseTestCase {
 
+  @Test
+  public void withCatalogAndSchema() {
+    TableName t = new TableName("a.b.c");
+    assertThat(t.withCatalogAndSchema("foo")).isEqualTo("a.b.foo");
+  }
+
+  @Test
+  public void withCatalogAndSchema_when_quoted() {
+    TableName t = new TableName("[a].[b].[c]");
+    assertThat(t.withCatalogAndSchema("foo")).isEqualTo("[a].[b].foo");
+
+    TableName noCat = new TableName("[b].[c]");
+    assertThat(noCat.withCatalogAndSchema("foo")).isEqualTo("[b].foo");
+
+    TableName tabOnly = new TableName("[c]");
+    assertThat(tabOnly.withCatalogAndSchema("foo")).isEqualTo("foo");
+  }
+
   @Test
   public void test() {