codeconsole commented on code in PR #16142:
URL: https://github.com/apache/grails-core/pull/16142#discussion_r3779216708
##########
grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageParser.java:
##########
@@ -1380,6 +1391,54 @@ public void setLastModified(long lastModified) {
this.lastModified = lastModified;
}
+ /**
+ * Computes the checksum recorded in the {@code SOURCE_CHECKSUM} constant
of a generated page.
+ * <p>
+ * Both the compiler that writes the constant and the runtime that
compares against it use this method, so
+ * that the two can never disagree on how a GSP source is digested. The
stream is read to the end but is
+ * not closed; closing it remains the caller's responsibility.
+ *
+ * @param source the raw bytes of the GSP source
+ * @return the checksum as a lower-case hex string
+ * @throws IOException if the source cannot be read
+ * @since 7.0.16
+ */
+ public static String checksumOf(InputStream source) throws IOException {
Review Comment:
Done in `e4055c2cb7` and `7920a56816`. `checksumOf(byte[])`, and both stream
wrappers are gone — the `InputStream` constructor just delegates to the
`String` one, which I'd missed. Raw-bytes invariant is in the javadoc.
##########
grails-doc/src/en/guide/theWebLayer/gsp/makingChangesToADeployedApplication.adoc:
##########
@@ -46,8 +46,10 @@ There are also some system properties to control GSP
reloading:
|===
|Name|Description|Default
|grails.gsp.enable.reload|system property for enabling the GSP reload mode
(alternative to adding it in the file-based application configuration|
-|grails.gsp.reload.interval|interval between checking the lastmodified time of
the gsp source file, unit is milliseconds|5000
-|grails.gsp.reload.granularity|the number of milliseconds leeway to give
before deciding a file is out of date. this is needed because different
roundings usually cause a 1000ms difference in lastmodified times|1000
+|grails.gsp.reload.interval|interval between checks of the gsp source file,
unit is milliseconds|5000
+|grails.gsp.reload.granularity|the number of milliseconds leeway to give
before deciding a file is out of date. this is needed because different
roundings usually cause a 1000ms difference in lastmodified times. Applies only
to pages compared by modification time — see below|2000
|===
GSP reloading is supported for precompiled GSPs since Grails 1.3.5.
+
+A precompiled GSP records a checksum of the page it was compiled from, and is
reloaded when the source no longer matches that checksum. Comparing content
rather than modification times means a page that was copied or checked out
afresh — and so carries a new modification time but the same content — is not
needlessly recompiled, and an edit is detected however close together two
writes fall. Pages compiled at runtime are still compared by modification time,
and so is any page precompiled by Grails 7.0 or earlier.
Review Comment:
Applied, bounded at 7.1.6.
##########
grails-gsp/core/src/test/groovy/org/grails/gsp/compiler/GroovyPageCompilerReproducibilitySpec.groovy:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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 org.grails.gsp.compiler
+
+import spock.lang.Specification
+import spock.lang.TempDir
+
+import org.grails.gsp.GroovyPageMetaInfo
+
+/**
+ * Precompiled GSPs must not vary with the modification time of their source.
+ *
+ * Git records no modification times, so every fresh clone or CI checkout
gives each .gsp a new one. Baking
+ * that into the generated class made otherwise identical jars differ on every
checkout, and because
+ * {@code LAST_MODIFIED} was a compile-time constant the difference survived
Gradle's compile-classpath
+ * normalization, so every downstream task missed the build cache.
+ */
+class GroovyPageCompilerReproducibilitySpec extends Specification {
+
+ private static final String PAGE_CONTENT = '<html><body><g:if
test="${flag}">Hello</g:if></body></html>'
+
+ @TempDir
+ File tempDir
+
+ private File viewsDir
+ private File page
+
+ void setup() {
+ this.viewsDir = new File(this.tempDir, 'views')
+ this.page = new File(this.viewsDir, 'index.gsp')
+ this.page.parentFile.mkdirs()
+ this.page.text = PAGE_CONTENT
+ }
+
+ void 'a source compiled at two different modification times produces
identical classes'() {
+ when: 'the same page is compiled twice, as two checkouts of one commit
would'
+ this.page.setLastModified(1_000_000_000_000L)
Review Comment:
Applied at both sites.
##########
grails-gsp/core/src/test/groovy/org/grails/gsp/compiler/GroovyPageCompilerReproducibilitySpec.groovy:
##########
@@ -0,0 +1,109 @@
+/*
+ * 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 org.grails.gsp.compiler
+
+import spock.lang.Specification
+import spock.lang.TempDir
+
+import org.grails.gsp.GroovyPageMetaInfo
+
+/**
+ * Precompiled GSPs must not vary with the modification time of their source.
+ *
+ * Git records no modification times, so every fresh clone or CI checkout
gives each .gsp a new one. Baking
+ * that into the generated class made otherwise identical jars differ on every
checkout, and because
+ * {@code LAST_MODIFIED} was a compile-time constant the difference survived
Gradle's compile-classpath
+ * normalization, so every downstream task missed the build cache.
+ */
+class GroovyPageCompilerReproducibilitySpec extends Specification {
+
+ private static final String PAGE_CONTENT = '<html><body><g:if
test="${flag}">Hello</g:if></body></html>'
+
+ @TempDir
+ File tempDir
+
+ private File viewsDir
+ private File page
+
+ void setup() {
+ this.viewsDir = new File(this.tempDir, 'views')
+ this.page = new File(this.viewsDir, 'index.gsp')
+ this.page.parentFile.mkdirs()
+ this.page.text = PAGE_CONTENT
+ }
+
+ void 'a source compiled at two different modification times produces
identical classes'() {
+ when: 'the same page is compiled twice, as two checkouts of one commit
would'
+ this.page.setLastModified(1_000_000_000_000L)
+ byte[] first = compileToBytes('first')
+
+ and:
+ this.page.setLastModified(1_700_000_000_000L)
+ byte[] second = compileToBytes('second')
+
+ then: 'the jar built from them is byte-identical, so downstream tasks
keep their cache hits'
+ first == second
+ }
+
+ void 'a compiled page records a checksum of its source instead of a
modification time'() {
+ when:
+ GroovyPageMetaInfo metaInfo = compileToMetaInfo('recorded')
+
+ then: 'the checksum identifies the content'
+ metaInfo.sourceChecksum ==~ /[0-9a-f]{64}/
+
+ and: 'no modification time is baked in for a fresh checkout to
invalidate'
+ metaInfo.lastModified == 0L
+ }
+
+ void 'an edited source produces a different checksum'() {
+ given:
+ GroovyPageMetaInfo before = compileToMetaInfo('before')
+
+ when:
+ this.page.text = '<html><body>something else entirely</body></html>'
+ GroovyPageMetaInfo after = compileToMetaInfo('after')
+
+ then: 'the runtime can still tell that the page changed'
+ before.sourceChecksum != after.sourceChecksum
+ }
+
+ private Map compile(File targetDir) {
+ targetDir.mkdirs()
+ GroovyPageCompiler compiler = new GroovyPageCompiler()
+ compiler.viewsDir = this.viewsDir
+ compiler.srcFiles = [this.page]
+ compiler.targetDir = targetDir
+ compiler.generatedGroovyPagesDirectory = new File(this.tempDir,
'generated').tap { mkdirs() }
+ compiler.compile()
+ }
+
+ private byte[] compileToBytes(String name) {
+ File targetDir = new File(this.tempDir, name)
+ Map results = compile(targetDir)
+ new File(targetDir, "${results.values().first()}.class").bytes
+ }
+
+ private GroovyPageMetaInfo compileToMetaInfo(String name) {
+ File targetDir = new File(this.tempDir, name)
+ Map results = compile(targetDir)
+ ClassLoader loader = new URLClassLoader([targetDir.toURI().toURL()] as
URL[], getClass().classLoader)
+ new GroovyPageMetaInfo(loader.loadClass(results.values().first() as
String))
Review Comment:
Applied — now `withCloseable`.
##########
grails-gsp/core/src/test/groovy/org/grails/gsp/GroovyPageMetaInfoReloadSpec.groovy:
##########
@@ -0,0 +1,180 @@
+/*
+ * 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 org.grails.gsp
+
+import java.security.PrivilegedAction
+
+import spock.lang.Specification
+import spock.lang.TempDir
+
+import org.springframework.core.io.FileSystemResource
+import org.springframework.core.io.Resource
+
+import org.grails.gsp.compiler.GroovyPageParser
+
+/**
+ * Reload-staleness behaviour of {@link GroovyPageMetaInfo}.
+ *
+ * A page compiled by {@code GroovyPageCompiler} records a checksum of its
source rather than the source's
+ * modification time, which git does not preserve across a checkout. Staleness
is therefore decided by
+ * comparing content, falling back to the timestamp for pages compiled before
the checksum existed.
+ *
+ * Each feature uses a fresh {@code GroovyPageMetaInfo}, because the result of
a check is cached for
+ * {@code grails.gsp.reload.interval} milliseconds.
+ */
+class GroovyPageMetaInfoReloadSpec extends Specification {
+
+ private static final String PAGE_CONTENT = '<html><body>hi</body></html>'
+
+ @TempDir
+ File tempDir
+
+ private Resource sourcePage(String content = PAGE_CONTENT) {
+ File page = new File(this.tempDir, 'index.gsp')
+ page.text = content
+ new FileSystemResource(page)
+ }
+
+ private static String checksumOf(Resource resource) {
+ resource.inputStream.withStream { InputStream input ->
GroovyPageParser.checksumOf(input) }
+ }
+
+ private static PrivilegedAction<Resource> callableFor(Resource resource) {
+ { -> resource } as PrivilegedAction
+ }
+
+ void 'a page whose recorded checksum matches its source is not reported as
stale'() {
+ given: 'a precompiled page recording the checksum of the source on
disk'
+ Resource resource = sourcePage()
+ GroovyPageMetaInfo metaInfo = new GroovyPageMetaInfo()
+ metaInfo.sourceChecksum = checksumOf(resource)
+
+ expect:
+ !metaInfo.shouldReload(callableFor(resource))
+ }
+
+ void 'a page whose source no longer matches its recorded checksum is
reported as stale'() {
+ given: 'a precompiled page whose source has since been edited'
+ Resource resource = sourcePage()
+ GroovyPageMetaInfo metaInfo = new GroovyPageMetaInfo()
+ metaInfo.sourceChecksum = checksumOf(resource)
+ resource.getFile().text = '<html><body>edited</body></html>'
+
+ expect:
+ metaInfo.shouldReload(callableFor(resource))
+ }
+
+ void 'a source that was touched but not edited is not reported as stale'()
{
+ given: 'a page whose source carries a modification time nothing like
the one it was compiled at'
+ Resource resource = sourcePage()
+ GroovyPageMetaInfo metaInfo = new GroovyPageMetaInfo()
+ metaInfo.sourceChecksum = checksumOf(resource)
+ resource.getFile().setLastModified(resource.getFile().lastModified() +
86_400_000L)
Review Comment:
Applied at all three sites.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
