Replace nullable Session properties with Optional

prestodb · Jul 25, 2015 · c7f2a4a · c7f2a4a
1 parent a6834e6
commit c7f2a4a
Show file tree

Hide file tree

Showing 11 changed files with 62 additions and 47 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/Session.java b/presto-main/src/main/java/com/facebook/presto/Session.java
@@ -21,14 +21,13 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 
-import javax.annotation.Nullable;
-
 import java.net.URI;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.TimeZone;
 
 import static com.google.common.base.MoreObjects.toStringHelper;
@@ -39,30 +38,27 @@
 public final class Session
 {
     private final String user;
-    @Nullable
+    private final Optional<String> source;
-    private final String source;
     private final String catalog;
     private final String schema;
     private final TimeZoneKey timeZoneKey;
     private final Locale locale;
-    @Nullable
+    private final Optional<String> remoteUserAddress;
-    private final String remoteUserAddress;
+    private final Optional<String> userAgent;
-    @Nullable
-    private final String userAgent;
     private final long startTime;
     private final Map<String, String> systemProperties;
     private final Map<String, Map<String, String>> catalogProperties;
 
     @JsonCreator
     public Session(
             @JsonProperty("user") String user,
-            @JsonProperty("source") @Nullable String source,
+            @JsonProperty("source") Optional<String> source,
             @JsonProperty("catalog") String catalog,
             @JsonProperty("schema") String schema,
             @JsonProperty("timeZoneKey") TimeZoneKey timeZoneKey,
             @JsonProperty("locale") Locale locale,
-            @JsonProperty("remoteUserAddress") @Nullable String remoteUserAddress,
+            @JsonProperty("remoteUserAddress") Optional<String> remoteUserAddress,
-            @JsonProperty("userAgent") @Nullable String userAgent,
+            @JsonProperty("userAgent") Optional<String> userAgent,
             @JsonProperty("startTime") long startTime,
             @JsonProperty("systemProperties") Map<String, String> systemProperties,
             @JsonProperty("catalogProperties") Map<String, Map<String, String>> catalogProperties)
@@ -91,9 +87,8 @@ public String getUser()
         return user;
     }
 
-    @Nullable
     @JsonProperty
-    public String getSource()
+    public Optional<String> getSource()
     {
         return source;
     }
@@ -122,16 +117,14 @@ public Locale getLocale()
         return locale;
     }
 
-    @Nullable
     @JsonProperty
-    public String getRemoteUserAddress()
+    public Optional<String> getRemoteUserAddress()
     {
         return remoteUserAddress;
     }
 
-    @Nullable
     @JsonProperty
-    public String getUserAgent()
+    public Optional<String> getUserAgent()
     {
         return userAgent;
     }
@@ -231,7 +224,7 @@ public ClientSession toClientSession(URI server, boolean debug)
         return new ClientSession(
                 checkNotNull(server, "server is null"),
                 user,
-                source,
+                source.orElse(null),
                 catalog,
                 schema,
                 timeZoneKey.getId(),
@@ -358,7 +351,18 @@ public SessionBuilder setCatalogProperties(String catalog, Map<String, String> p
 
         public Session build()
         {
-            return new Session(user, source, catalog, schema, timeZoneKey, locale, remoteUserAddress, userAgent, startTime, systemProperties, catalogProperties);
+            return new Session(
+                    user,
+                    Optional.ofNullable(source),
+                    catalog,
+                    schema,
+                    timeZoneKey,
+                    locale,
+                    Optional.ofNullable(remoteUserAddress),
+                    Optional.ofNullable(userAgent),
+                    startTime,
+                    systemProperties,
+                    catalogProperties);
         }
     }
 }
diff --git a/presto-main/src/main/java/com/facebook/presto/connector/system/QuerySystemTable.java b/presto-main/src/main/java/com/facebook/presto/connector/system/QuerySystemTable.java
@@ -89,7 +89,7 @@ public RecordCursor cursor()
                     queryInfo.getQueryId().toString(),
                     queryInfo.getState().toString(),
                     queryInfo.getSession().getUser(),
-                    queryInfo.getSession().getSource(),
+                    queryInfo.getSession().getSource().orElse(null),
                     queryInfo.getQuery(),
 
                     toMillis(queryStats.getQueuedTime()),

diff --git a/presto-main/src/main/java/com/facebook/presto/event/query/QueryMonitor.java b/presto-main/src/main/java/com/facebook/presto/event/query/QueryMonitor.java
@@ -66,13 +66,13 @@ public void createdEvent(QueryInfo queryInfo)
                 new QueryCreatedEvent(
                         queryInfo.getQueryId(),
                         queryInfo.getSession().getUser(),
-                        queryInfo.getSession().getSource(),
+                        queryInfo.getSession().getSource().orElse(null),
                         serverVersion,
                         environment,
                         queryInfo.getSession().getCatalog(),
                         queryInfo.getSession().getSchema(),
-                        queryInfo.getSession().getRemoteUserAddress(),
+                        queryInfo.getSession().getRemoteUserAddress().orElse(null),
-                        queryInfo.getSession().getUserAgent(),
+                        queryInfo.getSession().getUserAgent().orElse(null),
                         queryInfo.getSelf(),
                         queryInfo.getQuery(),
                         queryInfo.getQueryStats().getCreateTime()
@@ -111,13 +111,13 @@ public void completionEvent(QueryInfo queryInfo)
                     new QueryCompletionEvent(
                             queryInfo.getQueryId(),
                             queryInfo.getSession().getUser(),
-                            queryInfo.getSession().getSource(),
+                            queryInfo.getSession().getSource().orElse(null),
                             serverVersion,
                             environment,
                             queryInfo.getSession().getCatalog(),
                             queryInfo.getSession().getSchema(),
-                            queryInfo.getSession().getRemoteUserAddress(),
+                            queryInfo.getSession().getRemoteUserAddress().orElse(null),
-                            queryInfo.getSession().getUserAgent(),
+                            queryInfo.getSession().getUserAgent().orElse(null),
                             queryInfo.getState(),
                             queryInfo.getSelf(),
                             queryInfo.getFieldNames(),

diff --git a/presto-main/src/main/java/com/facebook/presto/execution/QueryQueueDefinition.java b/presto-main/src/main/java/com/facebook/presto/execution/QueryQueueDefinition.java
@@ -20,7 +20,6 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Strings.nullToEmpty;
 
 public class QueryQueueDefinition
 {
@@ -47,7 +46,7 @@ public QueryQueueDefinition(String template, int maxConcurrent, int maxQueued)
     public String getExpandedTemplate(Session session)
     {
         String expanded = USER_PATTERN.matcher(template).replaceAll(session.getUser());
-        return SOURCE_PATTERN.matcher(expanded).replaceAll(nullToEmpty(session.getSource()));
+        return SOURCE_PATTERN.matcher(expanded).replaceAll(session.getSource().orElse(""));
     }
 
     String getTemplate()

diff --git a/presto-main/src/main/java/com/facebook/presto/execution/QueryQueueRule.java b/presto-main/src/main/java/com/facebook/presto/execution/QueryQueueRule.java
@@ -25,7 +25,6 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Strings.nullToEmpty;
 import static java.lang.String.format;
 
 public class QueryQueueRule
@@ -70,8 +69,8 @@ public List<QueryQueueDefinition> match(Session session)
             return null;
         }
         if (sourceRegex != null) {
-            String source = session.getSource();
+            String source = session.getSource().orElse("");
-            if (!sourceRegex.matcher(nullToEmpty(source)).matches()) {
+            if (!sourceRegex.matcher(source).matches()) {
                 return null;
             }
         }

diff --git a/presto-main/src/main/java/com/facebook/presto/sql/analyzer/TupleAnalyzer.java b/presto-main/src/main/java/com/facebook/presto/sql/analyzer/TupleAnalyzer.java
@@ -1077,13 +1077,13 @@ private TupleDescriptor analyzeView(Query query, QualifiedTableName name, String
         try {
             Session viewSession = Session.builder()
                     .setUser(session.getUser())
-                    .setSource(session.getSource())
+                    .setSource(session.getSource().orElse(null))
                     .setCatalog(catalog)
                     .setSchema(schema)
                     .setTimeZoneKey(session.getTimeZoneKey())
                     .setLocale(session.getLocale())
-                    .setRemoteUserAddress(session.getRemoteUserAddress())
+                    .setRemoteUserAddress(session.getRemoteUserAddress().orElse(null))
-                    .setUserAgent(session.getUserAgent())
+                    .setUserAgent(session.getUserAgent().orElse(null))
                     .setStartTime(session.getStartTime())
                     .build();
 

diff --git a/presto-main/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java b/presto-main/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java
@@ -175,7 +175,7 @@ public static LocalQueryRunner createHashEnabledQueryRunner(LocalQueryRunner loc
         Session session = localQueryRunner.getDefaultSession();
         Session.SessionBuilder builder = Session.builder()
                 .setUser(session.getUser())
-                .setSource(session.getSource())
+                .setSource(session.getSource().orElse(null))
                 .setCatalog(session.getCatalog())
                 .setTimeZoneKey(session.getTimeZoneKey())
                 .setLocale(session.getLocale())

diff --git a/presto-main/src/test/java/com/facebook/presto/execution/TestQueryQueueDefinition.java b/presto-main/src/test/java/com/facebook/presto/execution/TestQueryQueueDefinition.java
@@ -14,20 +14,21 @@
 package com.facebook.presto.execution;
 
 import com.facebook.presto.Session;
-import com.facebook.presto.spi.type.TimeZoneKey;
 import com.google.common.collect.ImmutableMap;
 import org.testng.annotations.Test;
 
-import java.util.Locale;
+import java.util.Optional;
 
+import static com.facebook.presto.spi.type.TimeZoneKey.UTC_KEY;
+import static java.util.Locale.ENGLISH;
 import static org.testng.Assert.assertEquals;
 
 public class TestQueryQueueDefinition
 {
     @Test
     public void testNameExpansion()
     {
-        Session session = new Session("bob", "the-internet", "", "", TimeZoneKey.UTC_KEY, Locale.ENGLISH, null, null, 0, ImmutableMap.of(), ImmutableMap.of());
+        Session session = new Session("bob", Optional.of("the-internet"), "", "", UTC_KEY, ENGLISH, Optional.empty(), Optional.empty(), 0, ImmutableMap.of(), ImmutableMap.of());
         QueryQueueDefinition definition = new QueryQueueDefinition("user.${USER}", 1, 1);
         assertEquals(definition.getExpandedTemplate(session), "user.bob");
         definition = new QueryQueueDefinition("source.${SOURCE}", 1, 1);

diff --git a/presto-main/src/test/java/com/facebook/presto/execution/TestQueryQueueRule.java b/presto-main/src/test/java/com/facebook/presto/execution/TestQueryQueueRule.java
@@ -14,22 +14,23 @@
 package com.facebook.presto.execution;
 
 import com.facebook.presto.Session;
-import com.facebook.presto.spi.type.TimeZoneKey;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import org.testng.annotations.Test;
 
-import java.util.Locale;
+import java.util.Optional;
 import java.util.regex.Pattern;
 
+import static com.facebook.presto.spi.type.TimeZoneKey.UTC_KEY;
+import static java.util.Locale.ENGLISH;
 import static org.testng.Assert.assertEquals;
 
 public class TestQueryQueueRule
 {
     @Test
     public void testBasic()
     {
-        Session session = new Session("bob", "the-internet", "", "", TimeZoneKey.UTC_KEY, Locale.ENGLISH, null, null, 0, ImmutableMap.of(), ImmutableMap.of());
+        Session session = new Session("bob", Optional.of("the-internet"), "", "", UTC_KEY, ENGLISH, Optional.empty(), Optional.empty(), 0, ImmutableMap.of(), ImmutableMap.of());
         QueryQueueDefinition definition = new QueryQueueDefinition("user.${USER}", 1, 1);
         QueryQueueRule rule = new QueryQueueRule(Pattern.compile(".+"), null, ImmutableMap.of(), ImmutableList.of(definition));
         assertEquals(rule.match(session), ImmutableList.of(definition));
@@ -38,7 +39,18 @@ public void testBasic()
     @Test
     public void testBigQuery()
     {
-        Session session = new Session("bob", "the-internet", "", "", TimeZoneKey.UTC_KEY, Locale.ENGLISH, null, null, 0, ImmutableMap.of("big_query", "true"), ImmutableMap.of());
+        Session session = new Session(
+                "bob",
+                Optional.of("the-internet"),
+                "",
+                "",
+                UTC_KEY,
+                ENGLISH,
+                Optional.empty(),
+                Optional.empty(),
+                0,
+                ImmutableMap.of("big_query", "true"),
+                ImmutableMap.of());
         QueryQueueDefinition definition = new QueryQueueDefinition("big", 1, 1);
         QueryQueueRule rule = new QueryQueueRule(null, null, ImmutableMap.of("big_query", Pattern.compile("true", Pattern.CASE_INSENSITIVE)), ImmutableList.of(definition));
         assertEquals(rule.match(session), ImmutableList.of(definition));

diff --git a/presto-main/src/test/java/com/facebook/presto/metadata/TestJsonTableHandle.java b/presto-main/src/test/java/com/facebook/presto/metadata/TestJsonTableHandle.java
@@ -52,13 +52,13 @@ public class TestJsonTableHandle
             "type", "information_schema",
             "session", ImmutableMap.<String, Object>builder()
                     .put("user", TEST_SESSION.getUser())
-                    .put("source", TEST_SESSION.getSource())
+                    .put("source", TEST_SESSION.getSource().get())
                     .put("catalog", TEST_SESSION.getCatalog())
                     .put("schema", TEST_SESSION.getSchema())
                     .put("timeZoneKey", (int) TEST_SESSION.getTimeZoneKey().getKey())
                     .put("locale", TEST_SESSION.getLocale().toString())
-                    .put("remoteUserAddress", TEST_SESSION.getRemoteUserAddress())
+                    .put("remoteUserAddress", TEST_SESSION.getRemoteUserAddress().get())
-                    .put("userAgent", TEST_SESSION.getUserAgent())
+                    .put("userAgent", TEST_SESSION.getUserAgent().get())
                     .put("startTime", TEST_SESSION.getStartTime())
                     .put("systemProperties", ImmutableMap.of())
                     .put("catalogProperties", ImmutableMap.of())

diff --git a/presto-main/src/test/java/com/facebook/presto/server/TestResourceUtil.java b/presto-main/src/test/java/com/facebook/presto/server/TestResourceUtil.java
@@ -55,12 +55,12 @@ public void testCreateSession()
         Session session = createSessionForRequest(request);
 
         assertEquals(session.getUser(), "testUser");
-        assertEquals(session.getSource(), "testSource");
+        assertEquals(session.getSource().get(), "testSource");
         assertEquals(session.getCatalog(), "testCatalog");
         assertEquals(session.getSchema(), "testSchema");
         assertEquals(session.getLocale(), Locale.TAIWAN);
         assertEquals(session.getTimeZoneKey(), getTimeZoneKey("Asia/Taipei"));
-        assertEquals(session.getRemoteUserAddress(), "testRemote");
+        assertEquals(session.getRemoteUserAddress().get(), "testRemote");
         assertEquals(session.getSystemProperties(), ImmutableMap.<String, String>builder()
                 .put("first", "abc")
                 .put("second", "mno")