diff --git a/grails-doc/src/en/guide/scaffolding.adoc b/grails-doc/src/en/guide/scaffolding.adoc index a938f20f7f9..b196a0fd2e6 100644 --- a/grails-doc/src/en/guide/scaffolding.adoc +++ b/grails-doc/src/en/guide/scaffolding.adoc @@ -188,7 +188,7 @@ All of this is what is known as "dynamic scaffolding" where the CRUD interface i NOTE: By default, the size of text areas in scaffolded views is defined in the CSS, so adding 'rows' and 'cols' attributes will have no effect. -Also, the standard scaffold views expect model variables of the form `InstanceList` for collections and `Instance` for single instances. It's tempting to use properties like 'books' and 'book', but those won't work. +Also, the standard scaffold views expect model variables of the form `List` and `Count` for collections, and `` for a single instance. The list view declares the count as `Number`, so it accepts the `Integer` or `Long` a controller supplies. ==== Static Scaffolding diff --git a/grails-doc/src/en/guide/theWebLayer/gsp/gspStaticCompilation.adoc b/grails-doc/src/en/guide/theWebLayer/gsp/gspStaticCompilation.adoc index f8d9870b9da..62449e9dddf 100644 --- a/grails-doc/src/en/guide/theWebLayer/gsp/gspStaticCompilation.adoc +++ b/grails-doc/src/en/guide/theWebLayer/gsp/gspStaticCompilation.adoc @@ -53,6 +53,12 @@ The variable [publisher] is undeclared. A page that declares no model has stated nothing, and reads what it is rendered with exactly as a dynamically compiled page does. This is what makes static compilation adoptable for views that were never written with it in mind: what the page does say is checked, and what it does not say still works. +A value the model supplies is converted to the declared type the way a Groovy assignment converts it, so a `Long` satisfies an `Integer` declaration and a `GString` satisfies a `String` one. A conversion that would change the value fails at render instead, naming the variable and both types: a `Long` too large for an `Integer`, or a `BigDecimal` with a fraction for a whole-number type. So does a value that cannot be converted at all, such as a `Date` for an `Integer`. + +Floating-point conversions preserve the exact binary value: widening a finite `Float` to `Double` is accepted, while `Double` to `Float` is rejected if it loses precision. Decimal conversions are also checked for exact equality, so `0.5G` can become a `Double`, but `0.1G` cannot be represented exactly by a `Double`. Non-finite values (`NaN` and infinities) cannot populate `BigDecimal` or `BigInteger` fields and fail with the same field-specific diagnostic. + +Precompiling the views does not catch these failures, because the value only exists once the page renders. + ==== Names Supplied by the Framework The names bound into every page do not need to be declared. Most carry their real types and are checked like anything else: diff --git a/grails-forge/grails-forge-core/src/main/resources/gsp/index.gsp b/grails-forge/grails-forge-core/src/main/resources/gsp/index.gsp index 4dcafd5728b..bb25b5a91dc 100644 --- a/grails-forge/grails-forge-core/src/main/resources/gsp/index.gsp +++ b/grails-forge/grails-forge-core/src/main/resources/gsp/index.gsp @@ -1,4 +1,5 @@ <%@ page import="grails.util.Environment"%> +<%@ page import="grails.web.mime.MimeType"%> <%@ page import="org.springframework.boot.SpringBootVersion"%> <%@ page import="org.springframework.core.SpringVersion"%> <%@ page import="org.springframework.util.ClassUtils"%> @@ -8,7 +9,7 @@ value="${pluginManager.allPlugins.toList() .withIndex() .collect { p, i -> [plugin: p, order: ((int) i) + 1] } - .sort { a, b -> a.plugin.name.toLowerCase() <=> b.plugin.name.toLowerCase() }}" + .sort { Map row -> ((grails.plugins.GrailsPlugin) row.plugin).name.toLowerCase() }}" /> @@ -455,11 +456,11 @@ + .sort { it.key.toLowerCase() }}"/>
@@ -566,7 +567,7 @@ .collect { l -> [name: (l.getClass().simpleName ?: l.getClass().name.tokenize('.').last()), packageName: (l.getClass().package?.name ?: ''), detail: l.toString()] } - .sort { a, b -> (a.name.toLowerCase() <=> b.name.toLowerCase()) ?: (a.detail <=> b.detail) }}"/> + .sort { Map a, Map b -> (a.name.toLowerCase() <=> b.name.toLowerCase()) ?: (a.detail <=> b.detail) }}"/> + .sort { MimeType a, MimeType b -> ((a.extension ?: '').toLowerCase() <=> (b.extension ?: '').toLowerCase()) ?: (a.name <=> b.name) } : []}"/>
diff --git a/grails-gsp/core/src/main/groovy/org/grails/gsp/GroovyPage.java b/grails-gsp/core/src/main/groovy/org/grails/gsp/GroovyPage.java index 0fd56700da5..e5033af2764 100644 --- a/grails-gsp/core/src/main/groovy/org/grails/gsp/GroovyPage.java +++ b/grails-gsp/core/src/main/groovy/org/grails/gsp/GroovyPage.java @@ -20,6 +20,7 @@ import java.io.Writer; import java.lang.reflect.Field; +import java.math.BigDecimal; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -33,6 +34,7 @@ import groovy.lang.MissingMethodException; import groovy.lang.Script; import org.codehaus.groovy.runtime.InvokerHelper; +import org.codehaus.groovy.runtime.typehandling.DefaultTypeTransformation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -179,17 +181,53 @@ public void initRun(Writer target, OutputContext outputContext, GroovyPageMetaIn private void applyModelFieldsFromBinding(Iterable modelFields) { for (Field field : modelFields) { + Object value = getProperty(field.getName()); + if (value == null) { + continue; + } + Object converted; try { - Object value = getProperty(field.getName()); - if (value != null) { - field.set(this, value); - } + converted = DefaultTypeTransformation.castToType(value, field.getType()); + } catch (RuntimeException e) { + throw new GroovyPagesException("Model field '" + field.getName() + "' is declared as " + + field.getType().getName() + " but the model supplied an instance of " + + value.getClass().getName() + ", which cannot be converted to it.", e, -1, getGroovyPageFileName()); + } + if (value instanceof Number && converted instanceof Number && !sameNumericValue((Number) value, (Number) converted)) { + throw new GroovyPagesException("Model field '" + field.getName() + "' is declared as " + + field.getType().getName() + ", which cannot hold the " + value.getClass().getName() + " " + + value + " the model supplied without changing it.", null, -1, getGroovyPageFileName()); + } + try { + field.set(this, converted); } catch (IllegalAccessException e) { throw new GroovyPagesException("Error setting model field '" + field.getName() + "'", e, -1, getGroovyPageFileName()); } } } + private static boolean sameNumericValue(Number original, Number converted) { + if (isFloatingPoint(original) && isFloatingPoint(converted)) { + // Every float is exactly representable as a double, including NaN and infinities. + return Double.compare(original.doubleValue(), converted.doubleValue()) == 0; + } + try { + return exactDecimalValue(original).compareTo(exactDecimalValue(converted)) == 0; + } catch (NumberFormatException e) { + // A non-finite floating-point value cannot equal a finite decimal or integer. + return false; + } + } + + private static BigDecimal exactDecimalValue(Number number) { + // Decimal strings round floating-point values and can hide a loss of precision. + return isFloatingPoint(number) ? new BigDecimal(number.doubleValue()) : new BigDecimal(number.toString()); + } + + private static boolean isFloatingPoint(Number number) { + return number instanceof Float || number instanceof Double; + } + public Object raw(Object value) { if (rawEncoder == null) { return InvokerHelper.invokeMethod(value, "encodeAsRaw", null); diff --git a/grails-gsp/core/src/test/groovy/org/grails/gsp/GspCompileStaticSpec.groovy b/grails-gsp/core/src/test/groovy/org/grails/gsp/GspCompileStaticSpec.groovy index 1b583e59d60..5dc89bdb9cc 100644 --- a/grails-gsp/core/src/test/groovy/org/grails/gsp/GspCompileStaticSpec.groovy +++ b/grails-gsp/core/src/test/groovy/org/grails/gsp/GspCompileStaticSpec.groovy @@ -19,8 +19,11 @@ package org.grails.gsp +import java.util.concurrent.atomic.AtomicLong + import grails.core.gsp.GrailsTagLibClass import org.grails.core.gsp.DefaultGrailsTagLibClass +import org.codehaus.groovy.runtime.typehandling.GroovyCastException import org.grails.taglib.TagLibraryLookup import spock.lang.Specification @@ -238,6 +241,112 @@ Date d4=new Date(123L) rendered == '123-456-789-123' } + def "model field is applied when the model supplies the declared type"() { + given: + def template = '''@{ model="Long sampleCount"}${sampleCount}''' + when: + def rendered = renderTemplate(template, [sampleCount: 42L], true) + then: + rendered == '42' + } + + def "a Number model field accepts every numeric type a controller may supply"() { + given: + def template = '''@{ model="Number sampleCount"}${sampleCount}''' + when: + def rendered = renderTemplate(template, [sampleCount: supplied], true) + then: + rendered == '42' + where: + // scaffolded controllers supply Long via the generated service and Integer + // via RestfulController.countResources() + supplied << [42 as Integer, 42L, 42 as Short, 42 as BigInteger] + } + + def "a model value is converted to the declared type the way a Groovy assignment converts it"() { + given: + def template = """@{ model="${declared} sampleCount"}\${sampleCount}""" + when: + def rendered = renderTemplate(template, [sampleCount: supplied], true) + then: + rendered == expected + where: + declared | supplied | expected + 'Integer' | 42L | '42' + 'int' | 42L | '42' + 'Long' | 42 | '42' + 'long' | (42 as Short) | '42' + 'Integer' | 42.0G | '42' + 'String' | "${40 + 2}" | '42' + 'Double' | 0.1f | Double.toString((double) 0.1f) + 'double' | 1.1f | Double.toString((double) 1.1f) + 'Float' | 0.5d | '0.5' + 'float' | -0.0d | '-0.0' + 'Double' | Float.NaN | 'NaN' + 'Float' | Double.POSITIVE_INFINITY | 'Infinity' + 'Double' | Double.NEGATIVE_INFINITY | '-Infinity' + 'BigDecimal' | 0.5d | '0.5' + 'Double' | 0.5G | '0.5' + 'Long' | new AtomicLong(42L) | '42' + } + + def "a conversion that would change the value names the field and both types"() { + given: + def template = """@{ model="${declared} sampleCount"}\${sampleCount}""" + when: + renderTemplate(template, [sampleCount: supplied], true) + then: + GroovyPagesException e = thrown() + e.message.contains("Model field 'sampleCount'") + e.message.contains(declaredName) + e.message.contains(supplied.getClass().name) + where: + declared | supplied | declaredName + 'Integer' | 3_000_000_000L | 'java.lang.Integer' + 'int' | 3_000_000_000L | 'int' + 'Integer' | 42.9G | 'java.lang.Integer' + 'Short' | 70_000 | 'java.lang.Short' + 'Float' | 0.1d | 'java.lang.Float' + 'float' | 1.1d | 'float' + 'Double' | Long.MAX_VALUE | 'java.lang.Double' + 'Float' | 16_777_217 | 'java.lang.Float' + 'Double' | 9_007_199_254_740_993G | 'java.lang.Double' + 'Double' | 0.1G | 'java.lang.Double' + 'BigDecimal' | 0.1d | 'java.math.BigDecimal' + 'Integer' | Double.NaN | 'java.lang.Integer' + 'Double' | new BigDecimal('1E400') | 'java.lang.Double' + 'Double' | Float.NEGATIVE_INFINITY | 'java.lang.Double' + } + + def "a model value that cannot be converted names the field and both types"() { + given: + def template = '''@{ model="Integer sampleCount"}${sampleCount}''' + when: + renderTemplate(template, [sampleCount: new Date()], true) + then: + GroovyPagesException e = thrown() + e.message.contains("Model field 'sampleCount'") + e.message.contains('java.lang.Integer') + e.message.contains('java.util.Date') + e.cause instanceof GroovyCastException + } + + def "a non-finite value that cannot be converted names the field and both types"() { + given: + def template = """@{ model="${declared} sampleCount"}\${sampleCount}""" + when: + renderTemplate(template, [sampleCount: supplied], true) + then: + GroovyPagesException e = thrown() + e.message.contains("Model field 'sampleCount'") + e.message.contains("java.math.${declared}") + e.message.contains(supplied.getClass().name) + e.cause instanceof NumberFormatException + where: + [declared, supplied] << [['BigDecimal', 'BigInteger'], + [Double.NaN, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY]].combinations() + } + def renderTemplate(templateSource, model, expectedCompileStaticMode, printSource = false) { def t = gpte.createTemplate(templateSource, "template${templateSource.hashCode()}") assert t.metaInfo.compilationException == null diff --git a/grails-profiles/web/skeleton/grails-app/views/index.gsp b/grails-profiles/web/skeleton/grails-app/views/index.gsp index 4dcafd5728b..bb25b5a91dc 100644 --- a/grails-profiles/web/skeleton/grails-app/views/index.gsp +++ b/grails-profiles/web/skeleton/grails-app/views/index.gsp @@ -1,4 +1,5 @@ <%@ page import="grails.util.Environment"%> +<%@ page import="grails.web.mime.MimeType"%> <%@ page import="org.springframework.boot.SpringBootVersion"%> <%@ page import="org.springframework.core.SpringVersion"%> <%@ page import="org.springframework.util.ClassUtils"%> @@ -8,7 +9,7 @@ value="${pluginManager.allPlugins.toList() .withIndex() .collect { p, i -> [plugin: p, order: ((int) i) + 1] } - .sort { a, b -> a.plugin.name.toLowerCase() <=> b.plugin.name.toLowerCase() }}" + .sort { Map row -> ((grails.plugins.GrailsPlugin) row.plugin).name.toLowerCase() }}" /> @@ -455,11 +456,11 @@ + .sort { it.key.toLowerCase() }}"/>
@@ -566,7 +567,7 @@ .collect { l -> [name: (l.getClass().simpleName ?: l.getClass().name.tokenize('.').last()), packageName: (l.getClass().package?.name ?: ''), detail: l.toString()] } - .sort { a, b -> (a.name.toLowerCase() <=> b.name.toLowerCase()) ?: (a.detail <=> b.detail) }}"/> + .sort { Map a, Map b -> (a.name.toLowerCase() <=> b.name.toLowerCase()) ?: (a.detail <=> b.detail) }}"/> + .sort { MimeType a, MimeType b -> ((a.extension ?: '').toLowerCase() <=> (b.extension ?: '').toLowerCase()) ?: (a.name <=> b.name) } : []}"/>
diff --git a/grails-scaffolding/src/test/groovy/grails/plugin/scaffolding/ScaffoldedIndexViewModelSpec.groovy b/grails-scaffolding/src/test/groovy/grails/plugin/scaffolding/ScaffoldedIndexViewModelSpec.groovy new file mode 100644 index 00000000000..6c949b9e401 --- /dev/null +++ b/grails-scaffolding/src/test/groovy/grails/plugin/scaffolding/ScaffoldedIndexViewModelSpec.groovy @@ -0,0 +1,63 @@ +/* + * 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 grails.plugin.scaffolding + +import grails.codegen.model.ModelBuilder +import grails.core.gsp.GrailsTagLibClass +import groovy.text.GStringTemplateEngine +import org.grails.gsp.GroovyPagesTemplateEngine +import org.grails.taglib.TagLibraryLookup +import spock.lang.Specification + +class ScaffoldedIndexViewModelSpec extends Specification implements ModelBuilder { + + void "the scaffolded index view holds the count either scaffolding path supplies"() { + given: 'the index view expanded from its template, the way dynamic scaffolding expands it' + String expanded = new GStringTemplateEngine() + .createTemplate(new File('src/main/templates/scaffolding/index.gsp')) + .make(model(ScaffoldedIndexBook).asMap()) + .toString() + String modelDirective = expanded.readLines().find { it.startsWith('@{ model=') } + + and: 'a page carrying only that declaration, so rendering it needs no tag library' + GroovyPagesTemplateEngine engine = new GroovyPagesTemplateEngine() + engine.afterPropertiesSet() + engine.tagLibraryLookup = new TagLibraryLookup() { + @Override + protected void putTagLib(Map tags, String name, GrailsTagLibClass taglib) { + tags.put(name, taglib.newInstance()) + } + } + def page = engine.createTemplate(modelDirective + '${scaffoldedIndexBookCount}', "scaffoldedIndex${supplied}") + + when: + StringWriter out = new StringWriter() + page.make([scaffoldedIndexBookList: [], scaffoldedIndexBookCount: supplied]).writeTo(new PrintWriter(out, true)) + + then: + modelDirective + out.toString() == supplied.toString() + + where: 'RestfulController supplies an Integer, a generated service a Long, and a Long count can pass Integer.MAX_VALUE' + supplied << [3, 3L, 3_000_000_000L] + } +} + +class ScaffoldedIndexBook { +} diff --git a/grails-test-examples/gsp-compile-static/build.gradle b/grails-test-examples/gsp-compile-static/build.gradle index 656a3cea450..72baeb56d5a 100644 --- a/grails-test-examples/gsp-compile-static/build.gradle +++ b/grails-test-examples/gsp-compile-static/build.gradle @@ -50,6 +50,20 @@ tasks.named('integrationTest') { dependsOn tasks.named('compileGroovyPages') } +// Every generated application starts from the web profile's welcome page, and nothing else compiles it +// statically, so it is staged beside this application's own pages and compiled with them. The forge copy +// is pinned to the profile copy by GrailsGspSpec, so compiling one covers both. +def stageViewsWithWelcomePage = tasks.register('stageViewsWithWelcomePage', Sync) { + duplicatesStrategy = DuplicatesStrategy.EXCLUDE + into layout.buildDirectory.dir('generated/views-with-welcome-page') + from layout.projectDirectory.dir('grails-app/views') + from rootProject.layout.projectDirectory.file('grails-profiles/web/skeleton/grails-app/views/index.gsp') +} +tasks.named('compileGroovyPages') { + source = layout.buildDirectory.dir('generated/views-with-welcome-page').get() + dependsOn stageViewsWithWelcomePage +} + apply { from rootProject.layout.projectDirectory.file('gradle/functional-test-config.gradle') from rootProject.layout.projectDirectory.file('gradle/grails-extension-gradle-config.gradle')