Skip to content

Commit 5b391d8

Browse files
committed
Fix NullPointerException when catalogSessionProperties is not configured
1 parent 6d36242 commit 5b391d8

File tree

2 files changed

+64
-3
lines changed

2 files changed

+64
-3
lines changed

presto-file-session-property-manager/src/test/java/com/facebook/presto/session/file/TestFileSessionPropertyManager.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@
1919
import com.facebook.presto.session.SessionMatchSpec;
2020
import com.facebook.presto.spi.session.SessionPropertyConfigurationManager;
2121
import com.google.common.collect.ImmutableMap;
22+
import org.testng.annotations.Test;
2223

2324
import java.io.IOException;
2425
import java.nio.file.Files;
2526
import java.nio.file.Path;
2627
import java.util.Arrays;
2728
import java.util.Map;
29+
import java.util.Optional;
2830

2931
import static com.facebook.presto.session.file.FileSessionPropertyManager.CODEC;
3032
import static org.testng.Assert.assertEquals;
@@ -59,4 +61,60 @@ protected void assertProperties(Map<String, String> defaultProperties, Map<Strin
5961
assertEquals(manager.getCatalogSessionProperties(CONTEXT), catalogProperties);
6062
}
6163
}
64+
65+
@Test
66+
public void testNullSessionProperties()
67+
throws IOException
68+
{
69+
ImmutableMap<String, Map<String, String>> catalogProperties = ImmutableMap.of("CATALOG", ImmutableMap.of("PROPERTY", "VALUE"));
70+
SessionMatchSpec spec = new SessionMatchSpec(
71+
Optional.empty(),
72+
Optional.empty(),
73+
Optional.empty(),
74+
Optional.empty(),
75+
Optional.empty(),
76+
Optional.empty(),
77+
Optional.empty(),
78+
null,
79+
catalogProperties);
80+
81+
assertProperties(ImmutableMap.of(), ImmutableMap.of(), catalogProperties, spec);
82+
}
83+
84+
@Test
85+
public void testNullCatalogSessionProperties()
86+
throws IOException
87+
{
88+
Map<String, String> properties = ImmutableMap.of("PROPERTY1", "VALUE1", "PROPERTY2", "VALUE2");
89+
SessionMatchSpec spec = new SessionMatchSpec(
90+
Optional.empty(),
91+
Optional.empty(),
92+
Optional.empty(),
93+
Optional.empty(),
94+
Optional.empty(),
95+
Optional.empty(),
96+
Optional.empty(),
97+
properties,
98+
null);
99+
100+
assertProperties(properties, spec);
101+
}
102+
103+
@Test(expectedExceptions = IllegalArgumentException.class, expectedExceptionsMessageRegExp = "Either sessionProperties or catalogSessionProperties must be provided")
104+
public void testNullBothSessionProperties()
105+
throws IOException
106+
{
107+
SessionMatchSpec spec = new SessionMatchSpec(
108+
Optional.empty(),
109+
Optional.empty(),
110+
Optional.empty(),
111+
Optional.empty(),
112+
Optional.empty(),
113+
Optional.empty(),
114+
Optional.empty(),
115+
null,
116+
null);
117+
118+
assertProperties(ImmutableMap.of(), spec);
119+
}
62120
}

presto-session-property-managers-common/src/main/java/com/facebook/presto/session/SessionMatchSpec.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,12 @@ public SessionMatchSpec(
6969
this.resourceGroupRegex = requireNonNull(resourceGroupRegex, "resourceGroupRegex is null");
7070
this.clientInfoRegex = requireNonNull(clientInfoRegex, "clientInfoRegex is null");
7171
this.overrideSessionProperties = requireNonNull(overrideSessionProperties, "overrideSessionProperties is null");
72-
requireNonNull(sessionProperties, "sessionProperties is null");
73-
this.sessionProperties = ImmutableMap.copyOf(sessionProperties);
74-
this.catalogSessionProperties = ImmutableMap.copyOf(catalogSessionProperties);
72+
73+
checkArgument(sessionProperties != null || catalogSessionProperties != null,
74+
"Either sessionProperties or catalogSessionProperties must be provided");
75+
76+
this.sessionProperties = ImmutableMap.copyOf(Optional.ofNullable(sessionProperties).orElse(ImmutableMap.of()));
77+
this.catalogSessionProperties = ImmutableMap.copyOf(Optional.ofNullable(catalogSessionProperties).orElse(ImmutableMap.of()));
7578
}
7679

7780
public <T> Map<String, T> match(Map<String, T> object, SessionConfigurationContext context)

0 commit comments

Comments
 (0)