From aaaecb42e939ac1a389968d02e41072e42448d05 Mon Sep 17 00:00:00 2001 From: Carlos Ruiz Date: Mon, 9 Sep 2024 15:37:08 +0200 Subject: [PATCH] IDEMPIERE-2981 Implement JSON Field type + IDEMPIERE-6227 - fixes (#2449) * IDEMPIERE-2981 Implement JSON Field type - Fix the JSON Editor, is sending to the database the value non prettified * - Fix issue adding a JSON column with log migration script enabled When logging migration script the Convert layer is applied, and we didn't implement Convert for the JSON column * IDEMPIERE-6227 Cannot add a column with spaces on the default value (Convert layer logging migration script) --- .../adempiere/webui/editor/WJsonEditor.java | 13 ++++-- .../src/org/compiere/db/DB_Oracle.java | 2 + .../compiere/dbPort/Convert_PostgreSQL.java | 46 +++++++++++++++---- .../test/base/Convert_PostgreSQLTest.java | 26 +++++++++++ 4 files changed, 76 insertions(+), 11 deletions(-) diff --git a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/editor/WJsonEditor.java b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/editor/WJsonEditor.java index 505fc4ef09..b2db7f9c05 100644 --- a/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/editor/WJsonEditor.java +++ b/org.adempiere.ui.zk/WEB-INF/src/org/adempiere/webui/editor/WJsonEditor.java @@ -1,5 +1,6 @@ package org.adempiere.webui.editor; +import org.adempiere.webui.event.ValueChangeEvent; import org.compiere.model.GridField; import org.compiere.util.Util; @@ -31,9 +32,15 @@ public class WJsonEditor extends WStringEditor { @Override public void setValue(Object value) { super.setValue(value); - - if (value != null && !Util.isEmpty(value.toString())) - getComponent().setValue(Util.prettifyJSONString(value.toString())); + + String prettyValue = null; + if (value != null && !Util.isEmpty(value.toString())) { + prettyValue = Util.prettifyJSONString(value.toString()); + if (! prettyValue.equals(value)) { + ValueChangeEvent vce = new ValueChangeEvent(this, getColumnName(), value, prettyValue); + super.fireValueChange(vce); + } + } } } diff --git a/org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java b/org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java index 238152a2d7..76c419f507 100644 --- a/org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java +++ b/org.compiere.db.oracle.provider/src/org/compiere/db/DB_Oracle.java @@ -1169,6 +1169,8 @@ public class DB_Oracle implements AdempiereDatabase sql.append(sqlDefault); // Constraint + if (column.getAD_Reference_ID() == DisplayType.JSON) + sql.append(" CONSTRAINT ").append(column.getAD_Table().getTableName()).append("_").append(column.getColumnName()).append("_isjson CHECK (").append(column.getColumnName()).append(" IS JSON)"); // Null Values if (column.isMandatory() && defaultValue != null && defaultValue.length() > 0) diff --git a/org.compiere.db.postgresql.provider/src/org/compiere/dbPort/Convert_PostgreSQL.java b/org.compiere.db.postgresql.provider/src/org/compiere/dbPort/Convert_PostgreSQL.java index 13f8bd3a80..5531f1bd72 100644 --- a/org.compiere.db.postgresql.provider/src/org/compiere/dbPort/Convert_PostgreSQL.java +++ b/org.compiere.db.postgresql.provider/src/org/compiere/dbPort/Convert_PostgreSQL.java @@ -103,6 +103,7 @@ public class Convert_PostgreSQL extends Convert_SQL92 { } else { + statement = convertAddJson(statement); statement = convertWithConvertMap(statement); statement = convertSimilarTo(statement); statement = DB_PostgreSQL.removeNativeKeyworkMarker(statement); @@ -1105,10 +1106,16 @@ public class Convert_PostgreSQL extends Convert_SQL92 { begin_default = rest.toUpperCase().indexOf( " DEFAULT ") + 9; defaultvalue = rest.substring(begin_default); - int nextspace = defaultvalue.indexOf(' '); - if (nextspace > -1) { - rest = defaultvalue.substring(nextspace); - defaultvalue = defaultvalue.substring(0, defaultvalue.indexOf(' ')); + String endDefaultChar = " "; + int shift = 0; + if (defaultvalue.startsWith("'")) { + endDefaultChar = "'"; + shift = 1; + } + int endDefault = defaultvalue.substring(shift).indexOf(endDefaultChar) + shift; + if (endDefault > -1+shift) { + rest = defaultvalue.substring(endDefault+shift); + defaultvalue = defaultvalue.substring(0, endDefault+shift); } else { rest = ""; } @@ -1167,10 +1174,16 @@ public class Convert_PostgreSQL extends Convert_SQL92 { begin_default = rest.toUpperCase().indexOf( " DEFAULT ") + 9; defaultvalue = rest.substring(begin_default); - int nextspace = defaultvalue.indexOf(' '); - if (nextspace > -1) { - rest = defaultvalue.substring(nextspace); - defaultvalue = defaultvalue.substring(0, defaultvalue.indexOf(' ')); + String endDefaultChar = " "; + int shift = 0; + if (defaultvalue.startsWith("'")) { + endDefaultChar = "'"; + shift = 1; + } + int endDefault = defaultvalue.substring(shift).indexOf(endDefaultChar) + shift; + if (endDefault > -1+shift) { + rest = defaultvalue.substring(endDefault+shift); + defaultvalue = defaultvalue.substring(0, endDefault+shift); } else { rest = ""; } @@ -1220,4 +1233,21 @@ public class Convert_PostgreSQL extends Convert_SQL92 { return sqlStatement; } + + /** + * For JSON columns Oracle uses CLOB ... CONSTRAINT ... CHECK IS JSON + * while oracle uses JSONB, no constraint + * @param statement + * @return + */ + private String convertAddJson(String statement) { + if (statement.toUpperCase().matches(".*\\bCLOB\\b.*\\bCONSTRAINT\\b.*CHECK\\b.*\\bIS JSON\\).*")) { + // remove the CONSTRAINT ... IS JSON part + statement = statement.replaceAll("(?i)\\bCONSTRAINT\\b.*CHECK\\b.*\\(.*\\bIS JSON\\)", ""); + // change type CLOB to JSONB + statement = statement.replaceAll("(?i)\\bCLOB\\b", "JSONB"); + } + return statement; + } + } // Convert diff --git a/org.idempiere.test/src/org/idempiere/test/base/Convert_PostgreSQLTest.java b/org.idempiere.test/src/org/idempiere/test/base/Convert_PostgreSQLTest.java index 9d647f516d..ae576f06cf 100644 --- a/org.idempiere.test/src/org/idempiere/test/base/Convert_PostgreSQLTest.java +++ b/org.idempiere.test/src/org/idempiere/test/base/Convert_PostgreSQLTest.java @@ -201,6 +201,32 @@ public final class Convert_PostgreSQLTest extends AbstractTestCase { sqe = "ALTER TABLE C_InvoiceTax ADD COLUMN Created TIMESTAMP DEFAULT statement_timestamp() NOT NULL"; r = DB.getDatabase().convertStatement(sql); assertEquals(sqe, r.trim()); + + sql = "ALTER TABLE M_FreightCategory MODIFY Description VARCHAR2(255 CHAR) DEFAULT 'Test Default with Spaces'"; + sqe = "INSERT INTO t_alter_column values('m_freightcategory','Description','VARCHAR(255)',null,'Test Default with Spaces')"; + r = DB.getDatabase().convertStatement(sql); + assertEquals(sqe, r.trim()); + + sql = "ALTER TABLE M_FreightCategory ADD Description VARCHAR2(255 CHAR) DEFAULT 'Test Default with Spaces'"; + sqe = "ALTER TABLE M_FreightCategory ADD COLUMN Description VARCHAR(255) DEFAULT 'Test Default with Spaces'"; + r = DB.getDatabase().convertStatement(sql); + assertEquals(sqe, r.trim()); + + sql = "ALTER TABLE M_FreightCategory ADD JsonData CLOB DEFAULT NULL CONSTRAINT M_FreightCategory_JsonData_isjson CHECK (JsonData IS JSON)"; + sqe = "ALTER TABLE M_FreightCategory ADD COLUMN JsonData JSONB DEFAULT NULL"; + r = DB.getDatabase().convertStatement(sql); + assertEquals(sqe, r.trim()); + + sql = "ALTER TABLE M_FreightCategory ADD JsonData CLOB DEFAULT '{ \"color\": \"red\" }' CONSTRAINT M_FreightCategory_JsonData_isjson CHECK (JsonData IS JSON) NOT NULL"; + sqe = "ALTER TABLE M_FreightCategory ADD COLUMN JsonData JSONB DEFAULT '{ \"color\": \"red\" }' NOT NULL"; + r = DB.getDatabase().convertStatement(sql); + assertEquals(sqe, r.trim()); + + sql = "ALTER TABLE M_FreightCategory MODIFY JsonData CLOB DEFAULT '{ \"color\": \"red\" }' CONSTRAINT M_FreightCategory_JsonData_isjson CHECK (JsonData IS JSON)"; + sqe = "INSERT INTO t_alter_column values('m_freightcategory','JsonData','JSONB',null,'{ \"color\": \"red\" }')"; + r = DB.getDatabase().convertStatement(sql); + assertEquals(sqe, r.trim()); + } finally { Ini.setProperty(P_POSTGRE_SQL_NATIVE, originalNative); }