From aa64880e5b05947e409c03a43513bbc5ed2ff1f3 Mon Sep 17 00:00:00 2001 From: delchev Date: Fri, 11 Sep 2026 15:14:35 +0300 Subject: [PATCH] ide-template: an authored default is read the same way in every generated language (#7293) One authored `defaultValue:` is written into several generated languages - the repository's Java literal and the item dialog's JavaScript seed - and `AuthoredDefaults` exists to state that every one of them has to read the authored text the same way, or the value the column holds and the value the form offers stop agreeing. They did not. `JsLiterals` unquotes first, so the SQL-quoted shape a working DB DEFAULT needs reads as the bare one; `JavaLiterals` unquoted in the `String` arm only. So `defaultValue: "'20'"` on an integer column seeded 20 in the dialog and emitted `entity.Quantity = Integer.valueOf("'20'")` in the repository - a NumberFormatException on every create that relied on the default, in the create path every writer passes through. - Every arm of `JavaLiterals.defaultValueExpression` reads the authored text through `AuthoredDefaults.unquote`, once, up front. - The boolean reader does the unquoting itself, in `AuthoredDefaults`, rather than in the Java half: `'true'` now reads as `true` in BOTH languages, where unquoting it in one would have reintroduced the very disagreement being fixed. - Each numeric arm validates the text by running the factory the generated expression calls (`Integer::valueOf`, `BigDecimal::new`, ...), so an unparsable default is refused while the author is generating, naming the property, its value and its type - the `resolveMultiselect` precedent in the same class. This replaces the old "fails that one create" rationale, which understated the cost: the literal is assigned in the repository's create path, so `Integer.valueOf("8.0")` failed every create of that entity, not one field. Every default in the repository's own fixtures and ITs parses, so nothing that generates today stops generating. Tests: the `'20'` case beside the `JsLiterals` one and the refusal case in `JavaLiteralsTest`, the quoted-boolean parity case in `JsLiteralsTest`, and the invariant end to end in `ModelParameterProcessorTest` - both literals asserted together, since agreeing is the whole point. Verified: `ide-template` unit suite (147), `ModelGenerationIT`, `IntentEngineIT` (80), `IntentEmissionCoverageIT` - which compiles the generated client Java - `formatter:validate` with the cache wiped, and the `-P release` javadoc build. Fixes #7293 Co-Authored-By: Claude Opus 5 --- .../service/model/AuthoredDefaults.java | 8 +- .../template/service/model/JavaLiterals.java | 62 ++++++++++---- .../model/ModelParameterProcessor.java | 8 +- .../service/model/JavaLiteralsTest.java | 82 ++++++++++++------- .../service/model/JsLiteralsTest.java | 2 + .../model/ModelParameterProcessorTest.java | 44 ++++++++++ 6 files changed, 159 insertions(+), 47 deletions(-) diff --git a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/AuthoredDefaults.java b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/AuthoredDefaults.java index 01519012c74..cef0736f552 100644 --- a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/AuthoredDefaults.java +++ b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/AuthoredDefaults.java @@ -28,11 +28,17 @@ private AuthoredDefaults() {} /** * Whether an authored boolean default reads as true. * + *

+ * Read through {@link #unquote(String)}, so the SQL-quoted shape a working DB DEFAULT needs + * ({@code 'true'}) reads as the bare one - and reads the same way in every generated language, + * which is this class's whole reason to exist. + * * @param defaultValue the authored default * @return true when it does */ static boolean readsAsTrue(String defaultValue) { - return "true".equals(defaultValue) || "TRUE".equals(defaultValue) || "1".equals(defaultValue); + String value = unquote(defaultValue); + return "true".equals(value) || "TRUE".equals(value) || "1".equals(value); } /** diff --git a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/JavaLiterals.java b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/JavaLiterals.java index 1073353bbc8..2190bedd294 100644 --- a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/JavaLiterals.java +++ b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/JavaLiterals.java @@ -9,6 +9,8 @@ */ package org.eclipse.dirigible.components.ide.template.service.model; +import java.util.function.Consumer; + /** * Java literals for values a model carries as text. * @@ -69,31 +71,63 @@ public static String escape(String value) { * the property has no default that a Java literal can stand in for. * *

- * A numeric default is parsed from its authored text rather than inlined as a numeric literal, so - * an author's {@code "8.0"} on an integer column fails that one create instead of failing the whole - * generated build. A string default is read in either authoring shape ({@link AuthoredDefaults}), - * and both yield the string the column would hold. A date/time or binary column has no literal: its - * DEFAULT is emitted verbatim into the DDL and is typically a SQL expression ({@code CURRENT_DATE}, - * {@code now()}). + * Every arm reads the authored text through {@link AuthoredDefaults}, so both authoring shapes - + * bare and SQL-quoted - yield the value the column would hold. Reading it in only one arm is how + * {@code defaultValue: "'20'"} on an integer column seeded {@code 20} in the item dialog and + * emitted {@code Integer.valueOf("'20'")} in the repository, a {@code NumberFormatException} on + * every create that relied on the default (dirigible #7293). + * + *

+ * A numeric default is parsed from its text rather than inlined as a numeric literal - the same + * parse the generated expression performs, which is why it is run HERE: an unparsable numeric + * default is refused while the author is generating, naming the property, instead of compiling into + * an expression that throws on every create of that entity. A date/time or binary column has no + * literal: its DEFAULT is emitted verbatim into the DDL and is typically a SQL expression + * ({@code CURRENT_DATE}, {@code now()}). * * @param javaClass the property's Java class, as the parameter graph resolved it * @param defaultValue the authored default, as the model carries it + * @param property the property the default is authored on, for the refusal message * @return the Java expression, or null when there is none + * @throws IllegalArgumentException when a numeric property's default is not a value of its type */ - public static String defaultValueExpression(String javaClass, String defaultValue) { + public static String defaultValueExpression(String javaClass, String defaultValue, String property) { if (javaClass == null || defaultValue == null || defaultValue.isEmpty()) { return null; } + String value = AuthoredDefaults.unquote(defaultValue); return switch (javaClass) { - case "java.math.BigDecimal" -> "new java.math.BigDecimal(\"" + escape(defaultValue) + "\")"; - case "Double" -> "Double.valueOf(\"" + escape(defaultValue) + "\")"; - case "Float" -> "Float.valueOf(\"" + escape(defaultValue) + "\")"; - case "Long" -> "Long.valueOf(\"" + escape(defaultValue) + "\")"; - case "Integer" -> "Integer.valueOf(\"" + escape(defaultValue) + "\")"; - case "Short" -> "Short.valueOf(\"" + escape(defaultValue) + "\")"; + case "java.math.BigDecimal" -> numericExpression("new java.math.BigDecimal", value, javaClass, property, + java.math.BigDecimal::new); + case "Double" -> numericExpression("Double.valueOf", value, javaClass, property, Double::valueOf); + case "Float" -> numericExpression("Float.valueOf", value, javaClass, property, Float::valueOf); + case "Long" -> numericExpression("Long.valueOf", value, javaClass, property, Long::valueOf); + case "Integer" -> numericExpression("Integer.valueOf", value, javaClass, property, Integer::valueOf); + case "Short" -> numericExpression("Short.valueOf", value, javaClass, property, Short::valueOf); case "Boolean" -> AuthoredDefaults.readsAsTrue(defaultValue) ? "Boolean.TRUE" : "Boolean.FALSE"; - case "String" -> "\"" + escape(AuthoredDefaults.unquote(defaultValue)) + "\""; + case "String" -> "\"" + escape(value) + "\""; default -> null; }; } + + /** + * A numeric default as the factory call the generated code applies it through, refusing a text the + * very same factory cannot read. + * + * @param factory the factory the expression calls + * @param value the authored default, unquoted + * @param javaClass the property's Java class, for the refusal message + * @param property the property the default is authored on, for the refusal message + * @param parse the factory itself, run here on the authored text + * @return the factory call + */ + private static String numericExpression(String factory, String value, String javaClass, String property, Consumer parse) { + try { + parse.accept(value); + } catch (NumberFormatException ex) { + throw new IllegalArgumentException("Property [" + property + "] declares the default [" + value + + "], which is not a value of its type [" + javaClass + "] - every create applying it would fail.", ex); + } + return factory + "(\"" + escape(value) + "\")"; + } } diff --git a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessor.java b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessor.java index b60fe2e09e0..b8b1abd1432 100644 --- a/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessor.java +++ b/components/ide/ide-template/src/main/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessor.java @@ -412,7 +412,7 @@ private static void processProperty(Map property, Map property, Map property) { + private static void resolveDefaultValueLiterals(Map property, Map entity) { String defaultValue = str(property, "dataDefaultValue"); if (defaultValue == null || defaultValue.isEmpty()) { return; @@ -460,7 +461,8 @@ private static void resolveDefaultValueLiterals(Map property) { if (Boolean.TRUE.equals(property.get("dataPrimaryKey")) || Boolean.TRUE.equals(property.get("dataAutoIncrement"))) { return; } - String expression = JavaLiterals.defaultValueExpression(str(property, "dataTypeJavaClass"), defaultValue); + String expression = JavaLiterals.defaultValueExpression(str(property, "dataTypeJavaClass"), defaultValue, + str(entity, "name") + "." + str(property, "name")); if (expression != null) { property.put("dataDefaultValueJavaLiteral", expression); } diff --git a/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/JavaLiteralsTest.java b/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/JavaLiteralsTest.java index bdf4d7bd983..e7855dcc680 100644 --- a/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/JavaLiteralsTest.java +++ b/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/JavaLiteralsTest.java @@ -13,6 +13,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * Tests the Java literals a model value is written into a generated source as. @@ -50,36 +52,58 @@ void escapesTheControlCharactersThatWouldEndTheLine() { } /** - * A numeric default is parsed from its authored text rather than inlined, so an author's "8.0" on - * an integer column fails that one create instead of failing the whole generated build. + * A numeric default is parsed from its authored text rather than inlined as a numeric literal - + * which keeps BigDecimal exact and needs no per-type suffix. */ @Test void parsesANumericDefaultFromItsAuthoredText() { - assertEquals("new java.math.BigDecimal(\"20.00\")", JavaLiterals.defaultValueExpression("java.math.BigDecimal", "20.00")); - assertEquals("Double.valueOf(\"1.5\")", JavaLiterals.defaultValueExpression("Double", "1.5")); - assertEquals("Float.valueOf(\"1.5\")", JavaLiterals.defaultValueExpression("Float", "1.5")); - assertEquals("Long.valueOf(\"7\")", JavaLiterals.defaultValueExpression("Long", "7")); - assertEquals("Integer.valueOf(\"7\")", JavaLiterals.defaultValueExpression("Integer", "7")); - assertEquals("Short.valueOf(\"7\")", JavaLiterals.defaultValueExpression("Short", "7")); - assertEquals("Integer.valueOf(\"8.0\")", JavaLiterals.defaultValueExpression("Integer", "8.0")); + assertEquals("new java.math.BigDecimal(\"20.00\")", JavaLiterals.defaultValueExpression("java.math.BigDecimal", "20.00", "E.P")); + assertEquals("Double.valueOf(\"1.5\")", JavaLiterals.defaultValueExpression("Double", "1.5", "E.P")); + assertEquals("Float.valueOf(\"1.5\")", JavaLiterals.defaultValueExpression("Float", "1.5", "E.P")); + assertEquals("Long.valueOf(\"7\")", JavaLiterals.defaultValueExpression("Long", "7", "E.P")); + assertEquals("Integer.valueOf(\"7\")", JavaLiterals.defaultValueExpression("Integer", "7", "E.P")); + assertEquals("Short.valueOf(\"7\")", JavaLiterals.defaultValueExpression("Short", "7", "E.P")); + assertEquals("Integer.valueOf(\"-7\")", JavaLiterals.defaultValueExpression("Integer", "-7", "E.P")); } /** - * A numeric default is the one place a malformed value cannot even be escaped into something that - * parses - so it must still compile, and fail at that one create. + * The defect: the SQL-quoted shape was read in the String arm only, so an integer column's + * {@code '20'} seeded 20 in the item dialog and emitted {@code Integer.valueOf("'20'")} in the + * repository - a NumberFormatException on every create that relied on the default (#7293). */ @Test - void escapesAMalformedNumericDefaultTooRatherThanBreakingTheCompile() { - assertEquals("Integer.valueOf(\"7\\\"\")", JavaLiterals.defaultValueExpression("Integer", "7\"")); + void readsANumericDefaultInEitherAuthoringShape() { + assertEquals("Integer.valueOf(\"20\")", JavaLiterals.defaultValueExpression("Integer", "'20'", "E.P")); + assertEquals("Long.valueOf(\"20\")", JavaLiterals.defaultValueExpression("Long", "'20'", "E.P")); + assertEquals("new java.math.BigDecimal(\"20.00\")", JavaLiterals.defaultValueExpression("java.math.BigDecimal", "'20.00'", "E.P")); + } + + /** + * A text the property's own factory cannot read is refused while the author is generating, naming + * the property - it used to compile into an expression that threw on every create of that entity. + */ + @Test + void refusesANumericDefaultThatIsNotAValueOfItsType() { + IllegalArgumentException refusal = + assertThrows(IllegalArgumentException.class, () -> JavaLiterals.defaultValueExpression("Integer", "8.0", "Order.Lines")); + assertTrue(refusal.getMessage() + .contains("Order.Lines"), + "the refusal must name the property, got: " + refusal.getMessage()); + assertThrows(IllegalArgumentException.class, () -> JavaLiterals.defaultValueExpression("Integer", "7\"", "E.P")); + assertThrows(IllegalArgumentException.class, () -> JavaLiterals.defaultValueExpression("Integer", "N/A", "E.P")); + assertThrows(IllegalArgumentException.class, () -> JavaLiterals.defaultValueExpression("java.math.BigDecimal", "1 or 2", "E.P")); + assertThrows(IllegalArgumentException.class, () -> JavaLiterals.defaultValueExpression("Long", "nextval('s')", "E.P")); } @Test void readsABooleanDefaultInEveryAuthoredShape() { - assertEquals("Boolean.TRUE", JavaLiterals.defaultValueExpression("Boolean", "true")); - assertEquals("Boolean.TRUE", JavaLiterals.defaultValueExpression("Boolean", "TRUE")); - assertEquals("Boolean.TRUE", JavaLiterals.defaultValueExpression("Boolean", "1")); - assertEquals("Boolean.FALSE", JavaLiterals.defaultValueExpression("Boolean", "false")); - assertEquals("Boolean.FALSE", JavaLiterals.defaultValueExpression("Boolean", "0")); + assertEquals("Boolean.TRUE", JavaLiterals.defaultValueExpression("Boolean", "true", "E.P")); + assertEquals("Boolean.TRUE", JavaLiterals.defaultValueExpression("Boolean", "TRUE", "E.P")); + assertEquals("Boolean.TRUE", JavaLiterals.defaultValueExpression("Boolean", "1", "E.P")); + assertEquals("Boolean.FALSE", JavaLiterals.defaultValueExpression("Boolean", "false", "E.P")); + assertEquals("Boolean.FALSE", JavaLiterals.defaultValueExpression("Boolean", "0", "E.P")); + assertEquals("Boolean.TRUE", JavaLiterals.defaultValueExpression("Boolean", "'true'", "E.P")); + assertEquals("Boolean.FALSE", JavaLiterals.defaultValueExpression("Boolean", "'false'", "E.P")); } /** @@ -88,24 +112,24 @@ void readsABooleanDefaultInEveryAuthoredShape() { */ @Test void readsAStringDefaultInEitherAuthoringShape() { - assertEquals("\"DRAFT\"", JavaLiterals.defaultValueExpression("String", "DRAFT")); - assertEquals("\"DRAFT\"", JavaLiterals.defaultValueExpression("String", "'DRAFT'")); - assertEquals("\"'\"", JavaLiterals.defaultValueExpression("String", "'")); - assertEquals("\"6\\\"\"", JavaLiterals.defaultValueExpression("String", "6\"")); - assertEquals("\"6\\\"\"", JavaLiterals.defaultValueExpression("String", "'6\"'")); + assertEquals("\"DRAFT\"", JavaLiterals.defaultValueExpression("String", "DRAFT", "E.P")); + assertEquals("\"DRAFT\"", JavaLiterals.defaultValueExpression("String", "'DRAFT'", "E.P")); + assertEquals("\"'\"", JavaLiterals.defaultValueExpression("String", "'", "E.P")); + assertEquals("\"6\\\"\"", JavaLiterals.defaultValueExpression("String", "6\"", "E.P")); + assertEquals("\"6\\\"\"", JavaLiterals.defaultValueExpression("String", "'6\"'", "E.P")); } @Test void hasNoExpressionForATypeWhoseDefaultIsASqlExpression() { - assertNull(JavaLiterals.defaultValueExpression("java.time.LocalDate", "CURRENT_DATE")); - assertNull(JavaLiterals.defaultValueExpression("java.time.Instant", "now()")); - assertNull(JavaLiterals.defaultValueExpression("byte[]", "x")); + assertNull(JavaLiterals.defaultValueExpression("java.time.LocalDate", "CURRENT_DATE", "E.P")); + assertNull(JavaLiterals.defaultValueExpression("java.time.Instant", "now()", "E.P")); + assertNull(JavaLiterals.defaultValueExpression("byte[]", "x", "E.P")); } @Test void hasNoExpressionWithoutADefault() { - assertNull(JavaLiterals.defaultValueExpression("String", null)); - assertNull(JavaLiterals.defaultValueExpression("String", "")); - assertNull(JavaLiterals.defaultValueExpression(null, "DRAFT")); + assertNull(JavaLiterals.defaultValueExpression("String", null, "E.P")); + assertNull(JavaLiterals.defaultValueExpression("String", "", "E.P")); + assertNull(JavaLiterals.defaultValueExpression(null, "DRAFT", "E.P")); } } diff --git a/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/JsLiteralsTest.java b/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/JsLiteralsTest.java index 6aa7db1a203..758b249e575 100644 --- a/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/JsLiteralsTest.java +++ b/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/JsLiteralsTest.java @@ -59,6 +59,8 @@ void seedsEachWidgetInTheShapeTheDraftHolds() { assertEquals("true", JsLiterals.defaultValueExpression("CHECKBOX", false, "TRUE")); assertEquals("true", JsLiterals.defaultValueExpression("CHECKBOX", false, "1")); assertEquals("false", JsLiterals.defaultValueExpression("CHECKBOX", false, "false")); + assertEquals("true", JsLiterals.defaultValueExpression("CHECKBOX", false, "'true'")); + assertEquals("false", JsLiterals.defaultValueExpression("CHECKBOX", false, "'false'")); assertEquals("20.00", JsLiterals.defaultValueExpression("TEXTBOX", true, "20.00")); assertEquals("'DRAFT'", JsLiterals.defaultValueExpression("TEXTBOX", false, "DRAFT")); // A dropdown's FK stays a string even though it reads as a number, so it matches an option's diff --git a/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessorTest.java b/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessorTest.java index abcf9e12576..9e016d8020a 100644 --- a/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessorTest.java +++ b/components/ide/ide-template/src/test/java/org/eclipse/dirigible/components/ide/template/service/model/ModelParameterProcessorTest.java @@ -167,6 +167,50 @@ void carriesTheAuthoredDefaultAsAnEscapedJavaScriptSeed() { assertEquals("20", rate.get("dataDefaultValueJsLiteral")); } + /** + * The defect: one authored default reaches several generated languages, and the SQL-quoted shape + * was read in the Java String arm only - so an integer column's {@code '20'} seeded 20 in the item + * dialog and emitted {@code Integer.valueOf("'20'")} in the repository, a NumberFormatException on + * every create that relied on it (#7293). The two literals are asserted together because agreeing + * is the whole invariant. + */ + @Test + void readsAQuotedNumericDefaultTheSameWayInBothLanguages() { + Map quantity = property("Quantity", "INTEGER"); + quantity.put("dataDefaultValue", "'20'"); + Map rate = property("VatRate", "DECIMAL"); + rate.put("dataDefaultValue", "'20.00'"); + Map billable = property("Billable", "BOOLEAN"); + billable.put("widgetType", "CHECKBOX"); + billable.put("dataDefaultValue", "'true'"); + ModelParameterProcessor.process(model(entity("Line", "Lines", quantity, rate, billable)), parameters()); + + assertEquals("Integer.valueOf(\"20\")", quantity.get("dataDefaultValueJavaLiteral")); + assertEquals("20", quantity.get("dataDefaultValueJsLiteral")); + assertEquals("new java.math.BigDecimal(\"20.00\")", rate.get("dataDefaultValueJavaLiteral")); + assertEquals("20.00", rate.get("dataDefaultValueJsLiteral")); + assertEquals("Boolean.TRUE", billable.get("dataDefaultValueJavaLiteral")); + assertEquals("true", billable.get("dataDefaultValueJsLiteral")); + } + + /** + * A numeric default the property's own type cannot read is refused while the author is generating, + * naming the property - it used to compile into an expression that threw on every create. + */ + @Test + void refusesANumericDefaultThatIsNotAValueOfItsType() { + Map quantity = property("Quantity", "INTEGER"); + quantity.put("dataDefaultValue", "8.0"); + Map model = model(entity("Line", "Lines", quantity)); + + IllegalArgumentException refusal = + assertThrows(IllegalArgumentException.class, () -> ModelParameterProcessor.process(model, parameters())); + + assertTrue(refusal.getMessage() + .contains("Line.Quantity"), + "the refusal must name the property, got: " + refusal.getMessage()); + } + /** * The key's presence is what the template reads as "this property has a default to seed", so a * property with none must leave it absent rather than null.