Merge pull request #36 from oracle/bugfix/KVSTORE-1923

mikebrey · web-flow · commit 9614e0c5ceb7 · 2023-06-28T09:51:07.000-06:00
[kvstore-1923] Handle table DDL mismatch
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,13 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/).
 
+## [1.7.0] [unreleased]
+### Added
+- Added the checks to verify entity definition matches with corresponding 
+  table in the database during table creation.
+
+### Changed
+
 ## [1.6.0]
 ### Added
 - Added support for composite primary keys.
diff --git a/config/checkstyle-suppressions.xml b/config/checkstyle-suppressions.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+
+<!DOCTYPE suppressions PUBLIC
+        "-//Checkstyle//DTD SuppressionFilter Configuration 1.0//EN"
+        "https://checkstyle.org/dtds/suppressions_1_0.dtd">
+
+<suppressions>
+  <suppress checks="^[a-z][a-zA-Z0-9]*"
+            files="TestTableCreation.java"/>
+</suppressions>
diff --git a/pom.xml b/pom.xml
@@ -298,6 +298,7 @@
                         <phase>validate</phase>
                         <configuration>
                             <configLocation>${project.basedir}/config/checkstyle.xml</configLocation>
+                            <suppressionsLocation>${project.basedir}/config/checkstyle-suppressions.xml</suppressionsLocation>
                             <encoding>UTF-8</encoding>
                             <consoleOutput>true</consoleOutput>
                             <failsOnError>true</failsOnError>
diff --git a/src/main/java/com/oracle/nosql/spring/data/core/NosqlTemplate.java b/src/main/java/com/oracle/nosql/spring/data/core/NosqlTemplate.java
@@ -93,7 +93,9 @@ public String getTableName(Class<?> domainClass) {
     @Override
     public boolean createTableIfNotExists(
         NosqlEntityInformation<?, ?> entityInformation) {
-        return doCreateTableIfNotExists(entityInformation);
+        boolean isTableExist = doCheckExistingTable(entityInformation);
+        // if table does not exist create
+        return (!isTableExist) ? doCreateTable(entityInformation) : true;
     }
 
     @SuppressWarnings("unchecked")
diff --git a/src/main/java/com/oracle/nosql/spring/data/core/NosqlTemplateBase.java b/src/main/java/com/oracle/nosql/spring/data/core/NosqlTemplateBase.java
@@ -6,17 +6,22 @@
  */
 package com.oracle.nosql.spring.data.core;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import oracle.nosql.driver.NoSQLException;
 import oracle.nosql.driver.NoSQLHandle;
 import oracle.nosql.driver.TableNotFoundException;
+import oracle.nosql.driver.TimeToLive;
 import oracle.nosql.driver.ops.DeleteRequest;
 import oracle.nosql.driver.ops.DeleteResult;
 import oracle.nosql.driver.ops.GetRequest;
 import oracle.nosql.driver.ops.GetResult;
+import oracle.nosql.driver.ops.GetTableRequest;
 import oracle.nosql.driver.ops.PrepareRequest;
 import oracle.nosql.driver.ops.PrepareResult;
 import oracle.nosql.driver.ops.PreparedStatement;
@@ -26,7 +31,10 @@
 import oracle.nosql.driver.ops.TableRequest;
 import oracle.nosql.driver.ops.TableResult;
 import oracle.nosql.driver.util.LruCache;
+import oracle.nosql.driver.values.ArrayValue;
 import oracle.nosql.driver.values.FieldValue;
+import oracle.nosql.driver.values.JsonOptions;
+import oracle.nosql.driver.values.JsonUtils;
 import oracle.nosql.driver.values.MapValue;
 
 import com.oracle.nosql.spring.data.NosqlDbFactory;
@@ -112,9 +120,7 @@ protected TableResult doTableRequest(NosqlEntityInformation<?, ?> entityInformat
         return tableRes;
     }
 
-    protected boolean doCreateTableIfNotExists(
-        NosqlEntityInformation<?, ?> entityInformation) {
-
+    protected String getCreateTableDDL(NosqlEntityInformation<?, ?> entityInformation) {
         String tableName = entityInformation.getTableName();
         String sql;
 
@@ -168,8 +174,12 @@ protected boolean doCreateTableIfNotExists(
                     entityInformation.getTtl().toString()));
         }
         sql = tableBuilder.toString();
+        return sql;
+    }
 
-        TableRequest tableReq = new TableRequest().setStatement(sql)
+    protected boolean doCreateTable(NosqlEntityInformation<?, ?> entityInformation) {
+        String ddl = getCreateTableDDL(entityInformation);
+        TableRequest tableReq = new TableRequest().setStatement(ddl)
             .setTableLimits(entityInformation.getTableLimits(nosqlDbFactory));
 
         TableResult tableRes = doTableRequest(entityInformation, tableReq);
@@ -178,6 +188,179 @@ protected boolean doCreateTableIfNotExists(
         return tableState == TableResult.State.ACTIVE;
     }
 
+    protected boolean doCheckExistingTable(NosqlEntityInformation<?, ?> entityInformation) {
+        final String colField = "fields";
+        final String colNameField = "name";
+        final String colTypeField = "type";
+        final String shardField = "shardKey";
+        final String primaryField = "primaryKey";
+        final String ttlField = "ttl";
+        final String identityField = "identity";
+
+        List<String> errors = new ArrayList<>();
+
+        TableResult tableResult = null;
+        String jsonSchema = null;
+        try {
+            tableResult = doGetTable(entityInformation);
+
+            // table does not exist return false
+            if (tableResult == null) {
+                return false;
+            }
+
+            /* If table already exist in the database compare and throw error if
+               mismatch*/
+            jsonSchema = tableResult.getSchema();
+            MapValue tableSchema = JsonUtils.createValueFromJson(jsonSchema,
+                            new JsonOptions().setMaintainInsertionOrder(true)).
+                    asMap();
+
+            ArrayValue tableColumns = tableSchema.get(colField).asArray();
+            ArrayValue tableShardKeys =
+                    tableSchema.get(shardField).asArray();
+            ArrayValue tablePrimaryKeys =
+                    tableSchema.get(primaryField).asArray();
+
+            Map<String, String> tableShardMap = new LinkedHashMap<>();
+            Map<String, String> tableNonShardMap = new LinkedHashMap<>();
+            Map<String, String> tableOthersMap = new LinkedHashMap<>();
+
+            // extract table details into maps
+            for (int i = 0; i < tableColumns.size(); i++) {
+                MapValue column = tableColumns.get(i).asMap();
+                String colName =
+                        column.getString(colNameField).toLowerCase();
+                String columnType =
+                        column.getString(colTypeField).toLowerCase();
+
+                if (i < tableShardKeys.size()) {
+                    tableShardMap.put(colName, columnType);
+                } else if (i < tablePrimaryKeys.size()) {
+                    tableNonShardMap.put(colName, columnType);
+                } else {
+                    tableOthersMap.put(colName, columnType);
+                }
+            }
+
+            // extract entity details into maps
+            Map<String, FieldValue.Type> shardKeys = entityInformation.
+                    getShardKeys();
+            Map<String, FieldValue.Type> nonShardKeys = entityInformation.
+                    getNonShardKeys();
+
+            Map<String, String> entityShardMap = new LinkedHashMap<>();
+            shardKeys.forEach((k, v) -> entityShardMap.put(
+                    k.toLowerCase(), v.name().toLowerCase()));
+
+            Map<String, String> entityNonShardMap = new LinkedHashMap<>();
+            nonShardKeys.forEach((k, v) -> entityNonShardMap.put(
+                    k.toLowerCase(), v.name().toLowerCase()));
+
+            Map<String, String> entityOthersMap = new LinkedHashMap<>();
+            entityOthersMap.put(JSON_COLUMN.toLowerCase(), "json");
+
+            // convert maps to String. String format is {k1 v1, k2 v2 ...}
+            String tableShards = "{" + tableShardMap.entrySet().stream()
+                    .map(e -> e.getKey() + " " + e.getValue()).
+                    collect(Collectors.joining(",")) + "}";
+            String tableNonShards = "{" + tableNonShardMap.entrySet()
+                    .stream().map(e -> e.getKey() + " " + e.getValue()).
+                    collect(Collectors.joining(",")) + "}";
+
+            String entityShards = "{" + entityShardMap.entrySet().stream()
+                    .map(e -> e.getKey() + " " + e.getValue()).
+                    collect(Collectors.joining(",")) + "}";
+            String entityNonShards = "{" + entityNonShardMap.entrySet()
+                    .stream().map(e -> e.getKey() + " " + e.getValue()).
+                    collect(Collectors.joining(",")) + "}";
+
+            String msg;
+            // check shard keys and types match
+            if (!tableShards.equals(entityShards)) {
+                msg = String.format("Shard primary keys mismatch: " +
+                        "table=%s, entity=%s.", tableShards, entityShards);
+                errors.add(msg);
+            }
+
+            // check non-shard keys and types match
+            if (!tableNonShards.equals(entityNonShards)) {
+                msg = String.format("Non-shard primary keys mismatch: " +
+                                "table=%s, entity=%s.", tableNonShards,
+                        entityNonShards);
+                errors.add(msg);
+            }
+
+            // check kv_json_ column exist and it's type is JSON
+            if (!tableOthersMap.containsKey(JSON_COLUMN.toLowerCase())) {
+                msg = String.format("'%s' column does not exist in the table",
+                        JSON_COLUMN);
+                errors.add(msg);
+            } else if (!tableOthersMap.get(JSON_COLUMN.toLowerCase()).
+                    equalsIgnoreCase("json")) {
+                msg = String.format("'%s' column type is not JSON in the " +
+                        "table", JSON_COLUMN);
+                errors.add(msg);
+            }
+
+            // check identity same
+            FieldValue identity = tableSchema.get(identityField);
+            if (identity != null && !entityInformation.isAutoGeneratedId()) {
+                errors.add("Identity information mismatch.");
+
+            } else if (identity == null && entityInformation.isAutoGeneratedId() &&
+                    entityInformation.getIdNosqlType() != FieldValue.Type.STRING) {
+                errors.add("Identity information mismatch.");
+            }
+
+            // TTL warning
+            FieldValue ttlValue = tableSchema.get(ttlField);
+            TimeToLive ttl = entityInformation.getTtl();
+            // TTL is present in database but not in the entity
+            if (ttlValue != null && ttl != null &&
+                    !ttl.toString().equalsIgnoreCase(ttlValue.getString())) {
+                LOG.warn("TTL of the table in database is different from " +
+                        "the TTL of the entity " +
+                        entityInformation.getJavaType().getName());
+            } else if (ttlValue == null && ttl != null && ttl.getValue() != 0) {
+                // TTL is present in entity but not in the database
+                LOG.warn("TTL of the table in database is different from " +
+                        "the TTL of the entity " +
+                        entityInformation.getJavaType().getName());
+            }
+        } catch (NullPointerException | ClassCastException ex) {
+            // something is wrong in parsing json schema
+            String msg = String.format("Could not validate schema of the " +
+                            "table %s in the database against entity %s : " +
+                            "%s",
+                    entityInformation.getTableName(),
+                    entityInformation.getJavaType().getName(),
+                    ex.getMessage());
+
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("JSON Schema of the table is " + jsonSchema);
+            }
+            LOG.warn(msg);
+        }
+
+        if (!errors.isEmpty()) {
+            StringBuilder sb = new StringBuilder();
+            sb.append("The following mismatch have been found between the " +
+                    "entity class ");
+            sb.append(entityInformation.getJavaType().getName());
+            sb.append(" definition and the existing table ");
+            sb.append(entityInformation.getTableName());
+            sb.append(" in the database:\n");
+            errors.forEach(err -> sb.append(err).append("\n"));
+            sb.append("To fix this errors, either make the entity class to" +
+                    " match the table definition or use NosqlTable.name" +
+                    " annotation to use a different table.");
+            throw new IllegalArgumentException(sb.toString());
+        }
+        // no mismatch between table and entity return true
+        return true;
+    }
+
     protected DeleteResult doDelete(
         NosqlEntityInformation<?, ?> entityInformation,
         MapValue primaryKey) {
@@ -229,16 +412,7 @@ protected PutResult doPut(NosqlEntityInformation<?, ?> entityInformation,
 
         PutResult putRes;
         try {
-            try {
-                putRes = nosqlClient.put(putReq);
-            } catch (TableNotFoundException tnfe) {
-                if (entityInformation.isAutoCreateTable()) {
-                    doCreateTableIfNotExists(entityInformation);
-                    putRes = nosqlClient.put(putReq);
-                } else {
-                    throw tnfe;
-                }
-            }
+            putRes = nosqlClient.put(putReq);
         } catch (NoSQLException nse) {
             LOG.error("Put: table: {} key: {}", putReq.getTableName(),
                 row.get(entityInformation.getIdColumnName()));
@@ -378,6 +552,18 @@ protected <T> Iterable<MapValue> doExecuteMapValueQuery(NosqlQuery query,
         return doQuery(qReq);
     }
 
+    protected TableResult doGetTable(NosqlEntityInformation<?, ?> entityInformation) {
+        try {
+            GetTableRequest request = new GetTableRequest();
+            request.setTableName(entityInformation.getTableName());
+            return nosqlClient.getTable(request);
+        } catch (TableNotFoundException tne) {
+            return null;
+        } catch (NoSQLException nse) {
+            throw MappingNosqlConverter.convert(nse);
+        }
+    }
+
     private PreparedStatement getPreparedStatement(
         NosqlEntityInformation<?, ?> entityInformation, String query) {
         PreparedStatement preparedStatement;
diff --git a/src/main/java/com/oracle/nosql/spring/data/core/ReactiveNosqlTemplate.java b/src/main/java/com/oracle/nosql/spring/data/core/ReactiveNosqlTemplate.java
@@ -78,8 +78,10 @@ public String getTableName(Class<?> domainClass) {
     public Mono<Boolean> createTableIfNotExists(
         NosqlEntityInformation<?, ?> entityInformation) {
         Assert.notNull(entityInformation, "Entity information should not be null");
-
-        return Mono.just(doCreateTableIfNotExists(entityInformation));
+        boolean isTableExist = doCheckExistingTable(entityInformation);
+        // if table does not exist create
+        return (!isTableExist) ? Mono.just(doCreateTable(entityInformation)) :
+                Mono.just(true);
     }
 
     /**
diff --git a/src/main/java/com/oracle/nosql/spring/data/repository/support/NosqlEntityInformation.java b/src/main/java/com/oracle/nosql/spring/data/repository/support/NosqlEntityInformation.java
@@ -463,8 +463,8 @@ public Map<String, FieldValue.Type> getNonShardKeys() {
     }
 
     private static class ProcessPrimaryKeys {
-        private Map<String, FieldValue.Type> shardKeys;
-        private Map<String, FieldValue.Type> nonShardKeys;
+        private final Map<String, FieldValue.Type> shardKeys;
+        private final Map<String, FieldValue.Type> nonShardKeys;
 
         public ProcessPrimaryKeys(Field idField) {
             shardKeys = new LinkedHashMap<>();
@@ -477,8 +477,8 @@ private void process(Field idField) {
 
             if (isCompositeKeyType(idField.getType())) {
                 //composite key
-                Map<Integer, SortedSet<String>> shardMap = new TreeMap<>();
-                Map<Integer, SortedSet<String>> nonShardMap = new TreeMap<>();
+                TreeMap<Integer, SortedSet<String>> shardMap = new TreeMap<>();
+                TreeMap<Integer, SortedSet<String>> nonShardMap = new TreeMap<>();
 
                 for (Field primaryKey : idField.getType().getDeclaredFields()) {
                     if (!primaryKey.isAnnotationPresent(Transient.class) &&
@@ -578,10 +578,8 @@ private void process(Field idField) {
                 }
 
                 if (!nonShardMap.isEmpty()) {
-                    int shardMaxOrder =
-                            (int) ((TreeMap<?, ?>) shardMap).lastKey();
-                    int nonShardMinOrder =
-                            (int) ((TreeMap<?, ?>) nonShardMap).firstKey();
+                    int shardMaxOrder = shardMap.lastKey();
+                    int nonShardMinOrder = nonShardMap.firstKey();
 
                     if (shardMaxOrder != -1 && nonShardMinOrder <= shardMaxOrder) {
                         throw new IllegalArgumentException("Order of non " +
diff --git a/src/test/java/com/oracle/nosql/spring/data/test/TestTTL.java b/src/test/java/com/oracle/nosql/spring/data/test/TestTTL.java
@@ -47,6 +47,11 @@ public void setup() {
         template.dropTableIfExists(EntityWith10DaysTTL.class.getSimpleName());
         template.dropTableIfExists(EntityWithDefaultTTL.class.getSimpleName());
         template.dropTableIfExists(EntityWithNegativeTTL.class.getSimpleName());
+
+        template.createTableIfNotExists(template.
+                getNosqlEntityInformation(EntityWith10DaysTTL.class));
+        template.createTableIfNotExists(template.
+                getNosqlEntityInformation(EntityWithDefaultTTL.class));
     }
 
     @After
diff --git a/src/test/java/com/oracle/nosql/spring/data/test/composite/MachineApp.java b/src/test/java/com/oracle/nosql/spring/data/test/composite/MachineApp.java
@@ -53,7 +53,8 @@ public static void staticSetup() throws ClassNotFoundException {
     @Before
     public void setup() {
         template.dropTableIfExists(Machine.class.getSimpleName());
-
+        template.createTableIfNotExists(template.
+                getNosqlEntityInformation(Machine.class));
         machineCache = new HashMap<>();
         List<IpAddress> routeAddress = new ArrayList<>();
         routeAddress.add(new IpAddress("127.0.0.1"));
diff --git a/src/test/java/com/oracle/nosql/spring/data/test/composite/MachineAppWithoutAnnotation.java b/src/test/java/com/oracle/nosql/spring/data/test/composite/MachineAppWithoutAnnotation.java
@@ -51,6 +51,8 @@ public static void staticSetup() throws ClassNotFoundException {
     @Before
     public void setup() {
         template.dropTableIfExists(MachineWithoutAnnotation.class.getSimpleName());
+        template.createTableIfNotExists(template.
+                getNosqlEntityInformation(MachineWithoutAnnotation.class));
     }
 
     @After
diff --git a/src/test/java/com/oracle/nosql/spring/data/test/composite/ReactiveMachineApp.java b/src/test/java/com/oracle/nosql/spring/data/test/composite/ReactiveMachineApp.java
diff --git a/src/test/java/com/oracle/nosql/spring/data/test/composite/TestTableCreation.java b/src/test/java/com/oracle/nosql/spring/data/test/composite/TestTableCreation.java

Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,8 @@ public static void staticSetup() throws ClassNotFoundException {`
`51`	`51`	`@Before`
`52`	`52`	`public void setup() {`
`53`	`53`	`template.dropTableIfExists(MachineWithoutAnnotation.class.getSimpleName());`
	`54`	`+ template.createTableIfNotExists(template.`
	`55`	`+ getNosqlEntityInformation(MachineWithoutAnnotation.class));`
`54`	`56`	`}`
`55`	`57`
`56`	`58`	`@After`