jamesfredley commented on code in PR #16631:
URL: https://github.com/apache/grails-core/pull/16631#discussion_r4383278981


##########
grails-datamapping-core/src/main/groovy/org/apache/grails/data/core/querytail/QueryTailRequestAttribution.groovy:
##########
@@ -0,0 +1,729 @@
+/*
+ *  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.apache.grails.data.core.querytail
+
+import java.lang.reflect.Method
+import java.nio.charset.StandardCharsets
+import java.util.concurrent.atomic.AtomicLong
+import java.util.regex.Pattern
+
+import groovy.transform.CompileStatic
+import groovy.transform.PackageScope
+
+import jakarta.servlet.http.HttpServletRequest
+
+import org.springframework.util.ClassUtils
+import org.springframework.web.context.request.RequestAttributes
+import org.springframework.web.context.request.RequestContextHolder
+import org.springframework.web.context.request.ServletRequestAttributes
+
+/**
+ * Attributes a captured statement to the web request on the current thread. 
Grails keeps the
+ * request, and the controller and action it resolved, on the thread that 
serves it, so a statement
+ * run by an interceptor, the action or the view all land on the same page. 
The first statement of
+ * a request gives it an id that every later statement of that request 
repeats, which is how the
+ * {@code query-tail} command counts statements per request. Each capture file 
keeps its own request
+ * ids, so a request recorded to another file cannot consume or reuse one of 
its ids. Work on any other
+ * thread is attributed to the thread by name.
+ *
+ * <p>The request is recorded as its method and path, with the values in the 
path replaced by
+ * {@code ?}. Once Grails has chosen the controller, the path is read against 
the URL mapping it
+ * matched: a segment the mapping spells out is kept, as is a variable segment 
that names the
+ * controller, action or namespace, and every other variable segment is 
replaced. A request a Spring
+ * MVC handler serves is recorded as the pattern its handler mapping matched, 
whose variables are
+ * replaced. When the path cannot be read against the mapping, the values of 
the parameters the URL
+ * mapping took from it and those of its query are replaced, found
+ * whether or not the client percent-encoded them, and any segment that is a 
number, a UUID or a long
+ * hexadecimal token, so that an id, a token or a key carried in a path never 
reaches the capture
+ * file. A segment a value is glued into, as a mapping such as {@code 
/reset/${token}_confirm}
+ * produces, is replaced whole, whether the value shows as sent or decoded. 
That also holds for a value
+ * spanning several segments, with literal text before its first segment or 
after its last, and for every path
+ * parameter after a semicolon, such as a {@code jsessionid} the container 
rewrote the URL with.
+ * Until the URL mapping has run, as for a statement of a
+ * filter that runs before the controller is chosen, or for a request that 
never reaches one, no
+ * segment's role is known and every segment is replaced; only the controller, 
or the pattern, the
+ * framework set on the request says that it has run, never a parameter, which 
a client can send.
+ * A request is described once and its line reused for its later statements, 
until its path or the
+ * handler it reached changes, as a forward changes them.</p>
+ *
+ * <p>Work a request starts on another thread that carries the request with 
it, as Grails promises
+ * and {@code @Async} methods do, counts for the request. A statement that 
runs once the request
+ * has completed is attributed to its thread.</p>
+ *
+ * <p>Spring Web is optional: without it on the classpath every statement is 
attributed to its
+ * thread.</p>
+ *
+ * @since 9.0
+ */
+@CompileStatic
+class QueryTailRequestAttribution {
+
+    /** The prefix of the request attribute holding the id each attribution 
gave a request. */
+    public static final String REQUEST_ID_ATTRIBUTE = 
'org.apache.grails.data.core.querytail.REQUEST_ID'
+
+    /** The request attribute Grails sets the controller name in. */
+    public static final String CONTROLLER_NAME_ATTRIBUTE = 
'org.grails.CONTROLLER_NAME_ATTRIBUTE'
+
+    /** The request attribute Grails sets the controller namespace in. */
+    public static final String CONTROLLER_NAMESPACE_ATTRIBUTE = 
'org.grails.CONTROLLER_NAMESPACE_ATTRIBUTE'
+
+    /** The request attribute Grails sets the action name in. */
+    public static final String ACTION_NAME_ATTRIBUTE = 
'org.grails.ACTION_NAME_ATTRIBUTE'
+
+    private static final boolean SPRING_WEB_PRESENT = ClassUtils.isPresent(
+            'org.springframework.web.context.request.RequestContextHolder', 
QueryTailRequestAttribution.classLoader)
+
+    private final AtomicLong requestCounter
+    private final String requestIdPrefix
+    private final String requestIdAttribute = REQUEST_ID_ATTRIBUTE + '.' + 
UUID.randomUUID()
+
+    /**
+     * An attribution whose request ids are {@code r1}, {@code r2} and so on.
+     */
+    QueryTailRequestAttribution() {
+        this('')
+    }
+
+    /**
+     * @param requestIdPrefix what every request id starts with, which tells 
the requests of one JVM
+     * from those of another writing the same capture
+     */
+    QueryTailRequestAttribution(String requestIdPrefix) {
+        this(requestIdPrefix, new AtomicLong())
+    }
+
+    /**
+     * @param requestIdPrefix what every request id starts with
+     * @param requestCounter the counter the requests are numbered by, which 
attributions that give
+     * their ids the same prefix share, so that none of them repeats an id 
another gave
+     */
+    @PackageScope
+    QueryTailRequestAttribution(String requestIdPrefix, AtomicLong 
requestCounter) {
+        this.requestIdPrefix = requestIdPrefix ?: ''
+        this.requestCounter = requestCounter
+    }
+
+    /**
+     * Fills in the request id, page and request of a record from the current 
thread.
+     *
+     * @param record the record of a statement that just ran on this thread
+     * @return the same record
+     */
+    QueryTailRecord attribute(QueryTailRecord record) {
+        if (SPRING_WEB_PRESENT) {
+            try {
+                if (WebRequestLookup.attribute(record, requestCounter, 
requestIdPrefix, requestIdAttribute)) {
+                    return record
+                }
+            }
+            catch (IllegalStateException ignored) {
+                // the request has completed, as a task it started can outlive 
it, and is no longer to be read
+            }
+        }
+        record.requestId = null
+        record.request = null
+        record.page = QueryTailRecord.THREAD_PAGE_PREFIX + 
Thread.currentThread().name
+        record
+    }
+
+    /**
+     * Keeps every reference to Spring Web, in a signature as well as in a 
body, in a class that is only
+     * loaded once Spring Web is known to be present. The outer class can then 
be loaded, and reflected
+     * on, without Spring Web on the classpath.
+     */
+    @CompileStatic
+    private static class WebRequestLookup {
+
+        private static final String REQUEST_ID_PREFIX = 'r'
+
+        private static final String PLACEHOLDER = '?'
+
+        /** The parameters a URL mapping sets that name code rather than carry 
a value. */
+        private static final Set<String> MAPPING_PARAMETERS = ['controller', 
'action', 'namespace', 'plugin', 'format'] as Set<String>
+
+        /** A segment that is a number, a UUID or a long hexadecimal token is 
a value whatever the mapping says. */
+        private static final Pattern VALUE_SHAPED = Pattern.compile(
+                
'(?i)[0-9]+|[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|[0-9a-f]{32,}')
+
+        private static final boolean SERVLET_PRESENT = ClassUtils.isPresent(
+                'jakarta.servlet.http.HttpServletRequest', 
WebRequestLookup.classLoader)
+
+        /** The request attribute Grails keeps the URL mapping it matched a 
request with in. */
+        private static final String MATCHED_MAPPING_ATTRIBUTE = 
'org.grails.url.match.info'
+
+        /** The request attribute a Spring MVC handler mapping keeps the 
pattern it matched a request with in. */
+        private static final String MATCHED_PATTERN_ATTRIBUTE = 
'org.springframework.web.servlet.HandlerMapping.bestMatchingPattern'
+
+        /** The request attribute the line a request was described with is 
kept in, with what it was described from. */
+        private static final String REQUEST_LINE_ATTRIBUTE = 
'org.apache.grails.data.core.querytail.REQUEST_LINE'
+
+        /** The {@code getParams()} of a Grails web request, which holds the 
parameters its URL mapping took, by class. */
+        private static final ClassValue<Method> PARAMETERS = 
publicMethod('getParams')
+
+        /** The {@code getUrlData()} of the URL mapping Grails matched a 
request with, by class. */
+        private static final ClassValue<Method> URL_DATA = 
publicMethod('getUrlData')
+
+        /** The {@code getTokens()} of the data of a URL mapping, by class. */
+        private static final ClassValue<Method> TOKENS = 
publicMethod('getTokens')
+
+        private static ClassValue<Method> publicMethod(String name) {
+            new ClassValue<Method>() {
+                @Override
+                protected Method computeValue(Class<?> type) {
+                    try {
+                        return type.getMethod(name)
+                    }
+                    catch (NoSuchMethodException ignored) {
+                        return null
+                    }
+                }
+            }
+        }
+
+        /**
+         * Fills in a record from the request on the current thread.
+         *
+         * @param record the record to fill in
+         * @param requestCounter the counter the next request id is taken from
+         * @param requestIdPrefix what every request id starts with
+         * @param requestIdAttribute the request attribute this capture keeps 
its ids in
+         * @return whether there is a request on the current thread
+         */
+        static boolean attribute(QueryTailRecord record, AtomicLong 
requestCounter, String requestIdPrefix, String requestIdAttribute) {
+            RequestAttributes attributes = 
RequestContextHolder.getRequestAttributes()
+            if (attributes == null) {
+                return false
+            }
+            record.requestId = requestId(attributes, requestCounter, 
requestIdPrefix, requestIdAttribute)
+            record.page = page(attributes)
+            record.request = SERVLET_PRESENT ? 
ServletRequestLookup.describe(attributes) : null
+            true
+        }
+
+        /**
+         * Gives the request line of a request: the one described for an 
earlier statement of the request,
+         * when the request has not moved on since, else one described now.
+         *
+         * @param method the HTTP method
+         * @param path the request path, as the client sent it, with the 
context path
+         * @param contextPath the context path of the application, which the 
path starts with
+         * @param queryString the query string of the request, or null
+         * @param attributes the request
+         */
+        static String describe(String method, String path, String contextPath, 
String queryString, RequestAttributes attributes) {
+            Object controller = 
attributes.getAttribute(CONTROLLER_NAME_ATTRIBUTE, 
RequestAttributes.SCOPE_REQUEST)
+            Object action = attributes.getAttribute(ACTION_NAME_ATTRIBUTE, 
RequestAttributes.SCOPE_REQUEST)
+            Object namespace = 
attributes.getAttribute(CONTROLLER_NAMESPACE_ATTRIBUTE, 
RequestAttributes.SCOPE_REQUEST)
+            Object mapping = 
attributes.getAttribute(MATCHED_MAPPING_ATTRIBUTE, 
RequestAttributes.SCOPE_REQUEST)
+            Object pattern = 
attributes.getAttribute(MATCHED_PATTERN_ATTRIBUTE, 
RequestAttributes.SCOPE_REQUEST)
+            RequestLine described = new RequestLine(method, path, queryString, 
controller, action, namespace, mapping, pattern)
+            Object earlier = attributes.getAttribute(REQUEST_LINE_ATTRIBUTE, 
RequestAttributes.SCOPE_REQUEST)
+            if (earlier instanceof RequestLine && ((RequestLine) 
earlier).sameAs(described)) {
+                return ((RequestLine) earlier).line
+            }
+            described.line = "${method} ${describePath(path, contextPath, 
queryString, attributes, described)}".toString()
+            attributes.setAttribute(REQUEST_LINE_ATTRIBUTE, described, 
RequestAttributes.SCOPE_REQUEST)
+            described.line
+        }
+
+        /**
+         * @return the path of a request with its values replaced: read 
against the mapping that matched it,
+         * or against the pattern a Spring MVC handler mapping matched, or by 
the values its parameters hold
+         */
+        private static String describePath(String path, String contextPath, 
String queryString, RequestAttributes attributes,
+                RequestLine request) {
+            // only the framework sets the controller attribute; a client can 
send a controller parameter
+            if (request.controller != null) {
+                Set<String> names = codeNames(request)
+                String mapped = readAgainstMapping(path, contextPath, 
request.mapping, names)
+                if (mapped != null) {
+                    return mapped
+                }
+                Set<String> values = new HashSet<>()
+                addValues(values, mappedParameters(attributes))
+                addQueryValues(values, queryString)
+                return redactPath(path, values, names)
+            }
+            if (request.pattern != null) {
+                return contextPath + patternPath(request.pattern.toString())
+            }
+            withholdPath(path)
+        }
+
+        /** @return the names of the controller, action and namespace a 
request reached, which a path may spell out */
+        private static Set<String> codeNames(RequestLine request) {
+            Set<String> names = new HashSet<>()
+            for (Object name in [request.controller, request.action, 
request.namespace]) {
+                if (name != null && !name.toString().isEmpty()) {
+                    names.add(name.toString())
+                }
+            }
+            names
+        }
+
+        /**
+         * Reads a path against the tokens of the URL mapping that matched it: 
a token the mapping spells
+         * out is kept where the path spells it out too, a variable segment 
that names the controller,
+         * action or namespace is kept, and every other variable is replaced, 
as is every segment a greedy
+         * token takes. The path parameters of a segment are replaced each.
+         *
+         * @return the path read so, or null when the mapping does not say 
how, or the path does not
+         * follow its tokens
+         */
+        private static String readAgainstMapping(String path, String 
contextPath, Object mapping, Set<String> names) {
+            String[] tokens = mappingTokens(mapping)
+            if (tokens == null) {
+                return null
+            }
+            String prefix = contextPath && path.startsWith(contextPath) ? 
contextPath : ''
+            String within = path.substring(prefix.length())
+            if (!within.startsWith('/')) {
+                return null
+            }
+            List<String> segments = new 
ArrayList<>(Arrays.asList(within.substring(1).split('/', -1)))
+            boolean trailingSlash = !segments.isEmpty() && 
segments.last().isEmpty()
+            if (trailingSlash) {
+                segments.removeLast()
+            }
+            List<String> spelled = tokens.findAll { String token -> 
!token.isEmpty() }
+            int greedy = spelled.findIndexOf { String token -> 
token.contains('**') }
+            List<String> out = []
+            int at = 0
+            for (int i = 0; i < spelled.size(); i++) {
+                String token = spelled[i]
+                boolean optional = token.endsWith('?')
+                String bare = optional ? token.substring(0, token.length() - 
1) : token
+                if (i == greedy) {
+                    int taken = segments.size() - (spelled.size() - 1)
+                    if (taken < 0) {
+                        return null
+                    }
+                    for (int k = 0; k < taken; k++) {
+                        out.add(PLACEHOLDER + parametersOf(segments[at++]))
+                    }
+                    continue
+                }
+                if (at >= segments.size()) {
+                    if (optional) {
+                        continue
+                    }
+                    return null
+                }
+                String segment = segments[at]
+                int semicolon = segment.indexOf(';')
+                String bareSegment = semicolon < 0 ? segment : 
segment.substring(0, semicolon)
+                String decoded = decode(bareSegment)
+                boolean last = at == segments.size() - 1
+                if (!bare.contains('(') && !bare.contains('*')) {
+                    if (decoded != bare && !(last && withoutExtension(decoded) 
== bare)) {
+                        return null
+                    }
+                    out.add(bareSegment + parametersOf(segment))
+                }
+                else if (bare == '(*)' && (names.contains(decoded) || (last && 
names.contains(withoutExtension(decoded))))) {
+                    out.add(bareSegment + parametersOf(segment))
+                }
+                else {
+                    out.add(PLACEHOLDER + parametersOf(segment))
+                }
+                at++
+            }
+            if (at != segments.size()) {
+                return null
+            }
+            prefix + '/' + out.join('/') + (trailingSlash && !out.isEmpty() ? 
'/' : '')
+        }
+
+        private static String withoutExtension(String segment) {
+            int dot = segment.lastIndexOf('.')
+            dot > 0 ? segment.substring(0, dot) : segment
+        }
+
+        private static String parametersOf(String segment) {
+            int semicolon = segment.indexOf(';')
+            semicolon < 0 ? '' : 
withheldPathParameters(segment.substring(semicolon))
+        }
+
+        /**
+         * @return the tokens of the URL mapping Grails matched a request 
with, which spell out its
+         * literal segments and stand in for its variables, or null when there 
is none to read
+         */
+        private static String[] mappingTokens(Object mapping) {
+            if (mapping == null) {
+                return null
+            }
+            Object data = invoke(URL_DATA, mapping)
+            Object tokens = data == null ? null : invoke(TOKENS, data)
+            tokens instanceof String[] ? (String[]) tokens : null
+        }
+
+        private static Object invoke(ClassValue<Method> methods, Object 
target) {
+            Method method = methods.get(target.getClass())
+            if (method == null) {
+                return null
+            }
+            try {
+                return method.invoke(target)
+            }
+            catch (ReflectiveOperationException | RuntimeException ignored) {
+                return null
+            }
+        }
+
+        /**
+         * @return the pattern a Spring MVC handler mapping matched, with 
every segment that holds a
+         * variable or a wildcard replaced
+         */
+        private static String patternPath(String pattern) {
+            String[] segments = pattern.split('/', -1)
+            for (int i = 0; i < segments.length; i++) {
+                String segment = segments[i]
+                if (segment.contains('{') || segment.contains('*') || 
segment.contains('?')) {
+                    segments[i] = PLACEHOLDER
+                }
+            }
+            segments.join('/')
+        }
+
+        /**
+         * Adds the values of a query string, decoded as the servlet container 
decodes them. The query
+         * string is read rather than the parameters of the request, as 
reading those makes the container
+         * parse a form posted in the body and use it up before the 
application reads it.
+         */
+        private static void addQueryValues(Set<String> values, String 
queryString) {
+            if (!queryString) {
+                return
+            }
+            for (String pair : queryString.split('&')) {
+                int equals = pair.indexOf('=')
+                if (equals >= 0 && 
!MAPPING_PARAMETERS.contains(decodeQuery(pair.substring(0, equals)))) {
+                    values.add(decodeQuery(pair.substring(equals + 1)))
+                }
+            }
+        }
+
+        private static String decodeQuery(String text) {
+            try {
+                return URLDecoder.decode(text, StandardCharsets.UTF_8)
+            }
+            catch (IllegalArgumentException ignored) {
+                return text
+            }
+        }
+
+        /**
+         * Replaces every segment of a path: before the URL mapping has run, 
nothing tells a literal
+         * segment from a token, so only the shape of the path is recorded.
+         */
+        private static String withholdPath(String path) {
+            String[] segments = path.split('/', -1)
+            StringBuilder out = new StringBuilder(path.length())
+            for (int i = 0; i < segments.length; i++) {
+                if (i > 0) {
+                    out.append('/')
+                }
+                out.append(segments[i].isEmpty() ? segments[i] : PLACEHOLDER)
+            }
+            out.toString()
+        }
+
+        private static String requestId(RequestAttributes attributes, 
AtomicLong requestCounter, String requestIdPrefix, String requestIdAttribute) {
+            // the request itself, which a task the request starts sees 
through attributes of its own
+            synchronized (SERVLET_PRESENT ? 
ServletRequestLookup.mutex(attributes) : attributes) {
+                Object existing = attributes.getAttribute(requestIdAttribute, 
RequestAttributes.SCOPE_REQUEST)
+                if (existing != null) {
+                    return existing.toString()
+                }
+                String id = requestIdPrefix + REQUEST_ID_PREFIX + 
requestCounter.incrementAndGet()
+                attributes.setAttribute(requestIdAttribute, id, 
RequestAttributes.SCOPE_REQUEST)
+                return id
+            }
+        }
+
+        private static String page(RequestAttributes attributes) {

Review Comment:
   For `/book`, the action is still null when a `before` interceptor runs. 
Those queries are recorded under `book`, and the action and view queries of the 
same request under `book/index`. `--page=book/index` then misses the 
interceptor queries.
   
   Resolve the default action from the matched controller when the action 
attribute is absent. Please assert one page key for that request.



-- 
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]

Reply via email to