jdaugherty commented on code in PR #16631: URL: https://github.com/apache/grails-core/pull/16631#discussion_r4349802341
########## grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/jdbc/sqltail/SqlTailCapture.groovy: ########## @@ -0,0 +1,155 @@ +/* + * 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.datastore.gorm.jdbc.sqltail + +import java.nio.charset.StandardCharsets +import java.nio.file.Files +import java.nio.file.StandardOpenOption + +import groovy.transform.CompileStatic +import groovy.util.logging.Slf4j + +/** + * Writes the statements an application runs to the capture file the {@code sql-tail} command + * tails. One capture is open per file, shared by every data source of the application; the file is + * truncated when the capture opens, so it holds the statements of the current run only. Each + * statement is flushed as it is written so that the command sees it at once. + * + * <p>A capture is only opened when {@link SqlTailSettings#SETTING_ENABLED} is set; an application + * started without it never reaches this class.</p> + * + * @since 8.1 + */ +@CompileStatic +@Slf4j +class SqlTailCapture implements Closeable { + + private static final Map<String, SqlTailCapture> OPEN = new HashMap<>() + + /** The file the statements are written to. */ + final File file + + private final SqlTailRequestAttribution attribution = new SqlTailRequestAttribution() + private final String key + private Writer writer + + private SqlTailCapture(File file, String key) { + this.file = file + this.key = key + File parent = file.absoluteFile.parentFile + try { + if (parent != null && !parent.isDirectory() && !parent.mkdirs() && !parent.isDirectory()) { + throw new IOException("Cannot create the sql-tail capture directory ${parent}") + } + this.writer = Files.newBufferedWriter(file.toPath(), StandardCharsets.UTF_8, + StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE) + } + catch (IOException e) { + throw new UncheckedIOException("Cannot open the sql-tail capture file ${file}: ${e.message}", e) + } + } + + /** + * Opens the capture for a file, or returns the one already open for it. + * + * @param file the capture file + * @return the capture + * @throws UncheckedIOException when the file cannot be created + */ + static synchronized SqlTailCapture open(File file) { + String key = file.canonicalPath + SqlTailCapture capture = OPEN.get(key) + if (capture == null) { + capture = new SqlTailCapture(file, key) + OPEN.put(key, capture) + log.info('Capturing SQL statements to {} for sql-tail', file) + } + capture + } + + /** + * Opens the capture the settings name. + * + * @param settings the sql-tail settings + * @return the capture + */ + static SqlTailCapture open(SqlTailSettings settings) { + open(settings.resolveFile()) + } + + /** + * Records a statement that is about to run on the current thread. + * + * @param dataSource the connection source name the statement runs against + * @param sql the statement + */ + void record(String dataSource, String sql) { + SqlTailRecord record = new SqlTailRecord(timestamp: System.currentTimeMillis(), dataSource: dataSource, sql: sql) Review Comment: Fixed in 43640050e9. `SqlTailCapture.record()` now redacts the recorded copy before it is written: string, number and dollar-quoted literals become `?` and comments are dropped. The statement returned to Hibernate is untouched. The statement is read in the syntax of the session factory's dialect (from `Dialect.openQuote()`). Where databases disagree on where a value ends (a backslash before a quote, nested comments, double quotes as strings on MySQL, or any statement run before the dialect is known), every reading has to produce the same redacted text. If they differ, or a literal or comment never ends, the line is recorded as `(statement withheld: its values could not be redacted)` instead of the statement. Tests: - `SqlTailRedactorSpec`: the cases per syntax, the fail-closed cases, and a check that an embedded value survives under no syntax. - `SqlTailStatementInspectorSpec` (H5 and H7): with `hibernate.use_sql_comments` on, a native query with `'example-secret'` and a key in a comment, and an HQL query with the same literal (H7 inlines it into both the SQL and its generated comment). The database returns the original value, a delegate inspector sees the original statement, and the capture file holds neither the value nor the key. - `SqlTailRunnerSpec`: the same statement, captured and then shown in the pages, statements and stream views, never shows the value or the key. ########## grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/jdbc/sqltail/SqlTailCapture.groovy: ########## @@ -0,0 +1,155 @@ +/* + * 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.datastore.gorm.jdbc.sqltail + +import java.nio.charset.StandardCharsets +import java.nio.file.Files +import java.nio.file.StandardOpenOption + +import groovy.transform.CompileStatic +import groovy.util.logging.Slf4j + +/** + * Writes the statements an application runs to the capture file the {@code sql-tail} command + * tails. One capture is open per file, shared by every data source of the application; the file is + * truncated when the capture opens, so it holds the statements of the current run only. Each + * statement is flushed as it is written so that the command sees it at once. + * + * <p>A capture is only opened when {@link SqlTailSettings#SETTING_ENABLED} is set; an application + * started without it never reaches this class.</p> + * + * @since 8.1 + */ +@CompileStatic +@Slf4j +class SqlTailCapture implements Closeable { + + private static final Map<String, SqlTailCapture> OPEN = new HashMap<>() + + /** The file the statements are written to. */ + final File file + + private final SqlTailRequestAttribution attribution = new SqlTailRequestAttribution() + private final String key + private Writer writer + + private SqlTailCapture(File file, String key) { + this.file = file + this.key = key + File parent = file.absoluteFile.parentFile + try { + if (parent != null && !parent.isDirectory() && !parent.mkdirs() && !parent.isDirectory()) { + throw new IOException("Cannot create the sql-tail capture directory ${parent}") + } + this.writer = Files.newBufferedWriter(file.toPath(), StandardCharsets.UTF_8, + StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE) + } + catch (IOException e) { + throw new UncheckedIOException("Cannot open the sql-tail capture file ${file}: ${e.message}", e) + } + } + + /** + * Opens the capture for a file, or returns the one already open for it. + * + * @param file the capture file + * @return the capture + * @throws UncheckedIOException when the file cannot be created + */ + static synchronized SqlTailCapture open(File file) { + String key = file.canonicalPath + SqlTailCapture capture = OPEN.get(key) Review Comment: Fixed in 43640050e9. The session factory now owns its capture: `SqlTailStatementInspector` is also installed as the configuration's `SessionFactoryObserver`, chaining any observer already set. It opens its capture when the factory is created (or on the first statement, if that comes earlier) and closes it in `sessionFactoryClosed`, which both Hibernate versions also call when construction fails. A failure before the factory exists opens nothing. `SqlTailCapture.open()` now returns a handle per owner over a reference-counted file. Closing a handle is idempotent; when the last one closes, the file is closed and removed from the map, so the next open truncates it and starts a new run. Hibernate 5's `HibernateMappingContextConfiguration` replaced any observer set before the build; it now chains it before destroying the service registry. Tests: `SqlTailCaptureSpec` covers last-owner cleanup and reopening the same path. `SqlTailStatementInspectorSpec` (H5 and H7) closes a datastore with two data sources and checks both captures close and the next start truncates the file, keeps a second datastore on the same file writing after the first closes, and checks that an observer the application set is still called. `HibernateMappingContextConfigurationSpec` (H5) checks the chained observer and that the registry is still destroyed. -- 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]
