Merge pull request #49 from xingyutangyuan/master

Fix GoogleSqlTest
google · Nov 29, 2023 · da92ae4 · da92ae4
2 parents f5f8c61 + cf68931
commit da92ae4
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 12 deletions.
diff --git a/mug-guava/src/main/java/com/google/mu/safesql/GoogleSql.java b/mug-guava/src/main/java/com/google/mu/safesql/GoogleSql.java
@@ -23,9 +23,11 @@ public final class GoogleSql {
 
 
   /**
-   * Similar to {@link SafeQuery#template}, except {@link Instant} are translated to `TIMESTAMP()` GoogleSql function,
-   * {@link ZonedDateTime} are translated to `DTETIME()` GoogleSql function, and {@link LocalDate} are translated
-   * to `DATE()` GoogleSql function.
+   * Much like {@link SafeQuery#template}, but with additional GoogleSQL translation rules.
+   * 
+   * <p>Specifically, {@link Instant} are translated to `TIMESTAMP()` GoogleSql function,
+   * {@link ZonedDateTime} are translated to `DATETIME()` GoogleSql function,
+   * and {@link LocalDate} are translated to `DATE()` GoogleSql function.
    */
   public static StringFormat.To<SafeQuery> template(@CompileTimeConstant String formatString) {
     return SafeQuery.template(formatString, value -> {

diff --git a/mug-guava/src/main/java/com/google/mu/safesql/SafeQuery.java b/mug-guava/src/main/java/com/google/mu/safesql/SafeQuery.java
@@ -132,7 +132,8 @@ static StringFormat.To<SafeQuery> template(
               placeholders
                   .collect(
                       new StringBuilder(),
-                      (b, p, v) -> b.append(it.next()).append(fillInPlaceholder(p, v, defaultConverter)))
+                      (b, p, v) ->
+                          b.append(it.next()).append(fillInPlaceholder(p, v, defaultConverter)))
                   .append(it.next())
                   .toString());
         });
@@ -158,7 +159,7 @@ public int hashCode() {
   }
 
   private static String fillInPlaceholder(
-      Substring.Match placeholder, Object value, Function<Object, String> byDefault) {
+      Substring.Match placeholder, Object value, Function<Object, String> defaultConverter) {
     validatePlaceholder(placeholder);
     if (value instanceof Iterable) {
       Iterable<?> iterable = (Iterable<?>) value;
@@ -173,7 +174,8 @@ private static String fillInPlaceholder(
         return String.join(
             "\", \"", Iterables.transform(iterable, v -> quotedBy('"', placeholder, v)));
       }
-      return String.join(", ", Iterables.transform(iterable, v -> unquoted(placeholder, v)));
+      return String.join(
+          ", ", Iterables.transform(iterable, v -> unquoted(placeholder, v, defaultConverter)));
     }
     if (placeholder.isImmediatelyBetween("'", "'")) {
       return quotedBy('\'', placeholder, value);
@@ -184,11 +186,11 @@ private static String fillInPlaceholder(
     if (placeholder.isImmediatelyBetween("`", "`")) {
       return backquoted(placeholder, value);
     }
-    String result = unquoted(placeholder, value);
-    return result == null ? byDefault.apply(value) : result;
+    return unquoted(placeholder, value, defaultConverter);
   }
 
-  private static String unquoted(Substring.Match placeholder, Object value) {
+  private static String unquoted(
+      Substring.Match placeholder, Object value, Function<Object, String> byDefault) {
     if (value == null) {
       return "NULL";
     }
@@ -210,7 +212,7 @@ private static String unquoted(Substring.Match placeholder, Object value) {
             + "subqueries must be wrapped in another SafeQuery object;\n"
             + "and string literals must be quoted like '%s'",
             TRUSTED_SQL_TYPE_NAME, placeholder);
-    return null;
+    return byDefault.apply(value);
   }
 
   private static String quotedBy(char quoteChar, Substring.Match placeholder, Object value) {

diff --git a/mug-guava/src/test/java/com/google/mu/safesql/GoogleSqlTest.java b/mug-guava/src/test/java/com/google/mu/safesql/GoogleSqlTest.java
@@ -8,16 +8,23 @@
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
 @RunWith(JUnit4.class)
 public class GoogleSqlTest {
+  @BeforeClass  // Consistently set the system property across the test suite
+  public static void setUpTrustedType() {
+    System.setProperty(
+        "com.google.mu.safesql.SafeQuery.trusted_sql_type",
+        SafeQueryTest.TrustedSql.class.getName());
+  }
 
   @Test
   public void farPastTimestampPlaceholder() {
-    ZonedDateTime time = ZonedDateTime.of(1900, 1, 1, 0, 0, 0, 0, googleZoneId());
+    ZonedDateTime time = ZonedDateTime.of(1900, 1, 1, 0, 0, 0, 0, ZoneId.of("America/Los_Angeles"));
     assertThat(
             template("SELECT * FROM tbl WHERE creation_time = {creation_time}")
                 .with(/* creation_time */ time.toInstant()))

diff --git a/mug-guava/src/test/java/com/google/mu/safesql/SafeQueryTest.java b/mug-guava/src/test/java/com/google/mu/safesql/SafeQueryTest.java
@@ -494,7 +494,7 @@ public void trustedSqlStringShouldNotBeDoubleQuoted() {
         .contains("TrustedSql should not be quoted: \"{value}\"");
   }
 
-  private static final class TrustedSql {
+  static final class TrustedSql {
     private final String sql;
 
     TrustedSql(String sql) {