Skip to content

Commit db6ca28

Browse files
jschneidereitjschneidereit
committed
remove unnecessary interceptor constructs, and some other small refactors
1 parent 74e6bcd commit db6ca28

File tree

12 files changed

+62
-122
lines changed

12 files changed

+62
-122
lines changed

‎jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/auth/AuthenticationSettings.java‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.salesforce.datacloud.jdbc.util.PropertiesExtensions;
2626
import java.net.URI;
2727
import java.net.URISyntaxException;
28-
import java.sql.SQLException;
2928
import java.util.Properties;
3029
import java.util.Set;
3130
import java.util.stream.Collectors;
@@ -37,7 +36,7 @@
3736

3837
@Getter
3938
public abstract class AuthenticationSettings {
40-
public static AuthenticationSettings of(@NonNull Properties properties) throws SQLException {
39+
public static AuthenticationSettings of(@NonNull Properties properties) throws DataCloudJDBCException {
4140
checkNotEmpty(properties);
4241
checkHasAllRequired(properties);
4342

@@ -72,14 +71,14 @@ private static boolean hasRefreshToken(Properties properties) {
7271
return hasAll(properties, Keys.REFRESH_TOKEN_KEYS);
7372
}
7473

75-
private static void checkNotEmpty(@NonNull Properties properties) throws SQLException {
74+
private static void checkNotEmpty(@NonNull Properties properties) throws DataCloudJDBCException {
7675
if (properties.isEmpty()) {
7776
throw new DataCloudJDBCException(
7877
Messages.PROPERTIES_EMPTY, "28000", new IllegalArgumentException(Messages.PROPERTIES_EMPTY));
7978
}
8079
}
8180

82-
private static void checkHasAllRequired(Properties properties) throws SQLException {
81+
private static void checkHasAllRequired(Properties properties) throws DataCloudJDBCException {
8382
if (hasAll(properties, Keys.REQUIRED_KEYS)) {
8483
return;
8584
}
@@ -91,15 +90,15 @@ private static void checkHasAllRequired(Properties properties) throws SQLExcepti
9190
throw new DataCloudJDBCException(missing, "28000", new IllegalArgumentException(missing));
9291
}
9392

94-
final URI getLoginUri() throws SQLException {
93+
final URI getLoginUri() throws DataCloudJDBCException {
9594
try {
9695
return new URI(loginUrl);
9796
} catch (URISyntaxException ex) {
9897
throw new DataCloudJDBCException(ex.getMessage(), "28000", ex);
9998
}
10099
}
101100

102-
protected AuthenticationSettings(@NonNull Properties properties) throws SQLException {
101+
protected AuthenticationSettings(@NonNull Properties properties) throws DataCloudJDBCException {
103102
checkNotEmpty(properties);
104103

105104
this.relevantProperties = copy(properties, Keys.ALL);
@@ -172,7 +171,7 @@ protected static class Messages {
172171

173172
@Getter
174173
class PasswordAuthenticationSettings extends AuthenticationSettings {
175-
protected PasswordAuthenticationSettings(@NonNull Properties properties) throws SQLException {
174+
protected PasswordAuthenticationSettings(@NonNull Properties properties) throws DataCloudJDBCException {
176175
super(properties);
177176

178177
this.password = required(this.getRelevantProperties(), Keys.PASSWORD);
@@ -185,7 +184,7 @@ protected PasswordAuthenticationSettings(@NonNull Properties properties) throws
185184

186185
@Getter
187186
class PrivateKeyAuthenticationSettings extends AuthenticationSettings {
188-
protected PrivateKeyAuthenticationSettings(@NonNull Properties properties) throws SQLException {
187+
protected PrivateKeyAuthenticationSettings(@NonNull Properties properties) throws DataCloudJDBCException {
189188
super(properties);
190189

191190
this.privateKey = required(this.getRelevantProperties(), Keys.PRIVATE_KEY);
@@ -198,7 +197,7 @@ protected PrivateKeyAuthenticationSettings(@NonNull Properties properties) throw
198197

199198
@Getter
200199
class RefreshTokenAuthenticationSettings extends AuthenticationSettings {
201-
protected RefreshTokenAuthenticationSettings(@NonNull Properties properties) throws SQLException {
200+
protected RefreshTokenAuthenticationSettings(@NonNull Properties properties) throws DataCloudJDBCException {
202201
super(properties);
203202

204203
this.refreshToken = required(this.getRelevantProperties(), Keys.REFRESH_TOKEN);

‎jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/auth/AuthenticationStrategy.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,12 @@
3030
import org.apache.commons.lang3.StringUtils;
3131

3232
interface AuthenticationStrategy {
33-
static AuthenticationStrategy of(@NonNull Properties properties) throws SQLException {
33+
static AuthenticationStrategy of(@NonNull Properties properties) throws DataCloudJDBCException {
3434
val settings = AuthenticationSettings.of(properties);
3535
return of(settings);
3636
}
3737

38-
static AuthenticationStrategy of(@NonNull AuthenticationSettings settings) throws SQLException {
38+
static AuthenticationStrategy of(@NonNull AuthenticationSettings settings) throws DataCloudJDBCException {
3939
if (settings instanceof PasswordAuthenticationSettings) {
4040
return new PasswordAuthenticationStrategy((PasswordAuthenticationSettings) settings);
4141
} else if (settings instanceof PrivateKeyAuthenticationSettings) {
@@ -67,7 +67,7 @@ class Keys {
6767
}
6868

6969
abstract class SharedAuthenticationStrategy implements AuthenticationStrategy {
70-
protected final FormCommand.Builder builder(HttpCommandPath path) throws SQLException {
70+
protected final FormCommand.Builder builder(HttpCommandPath path) throws DataCloudJDBCException {
7171
val settings = getSettings();
7272
val builder = FormCommand.builder();
7373

@@ -95,7 +95,7 @@ class PasswordAuthenticationStrategy extends SharedAuthenticationStrategy {
9595
* password flow docs</a>
9696
*/
9797
@Override
98-
public FormCommand buildAuthenticate() throws SQLException {
98+
public FormCommand buildAuthenticate() throws DataCloudJDBCException {
9999
val builder = super.builder(HttpCommandPath.AUTHENTICATE);
100100

101101
builder.bodyEntry(Keys.GRANT_TYPE, GRANT_TYPE);
@@ -122,7 +122,7 @@ class RefreshTokenAuthenticationStrategy extends SharedAuthenticationStrategy {
122122
* token flow docs</a>
123123
*/
124124
@Override
125-
public FormCommand buildAuthenticate() throws SQLException {
125+
public FormCommand buildAuthenticate() throws DataCloudJDBCException {
126126
val builder = super.builder(HttpCommandPath.AUTHENTICATE);
127127

128128
builder.bodyEntry(Keys.GRANT_TYPE, GRANT_TYPE);
@@ -147,7 +147,7 @@ class PrivateKeyAuthenticationStrategy extends SharedAuthenticationStrategy {
147147
* docs</a>
148148
*/
149149
@Override
150-
public FormCommand buildAuthenticate() throws SQLException {
150+
public FormCommand buildAuthenticate() throws DataCloudJDBCException {
151151
val builder = super.builder(HttpCommandPath.AUTHENTICATE);
152152

153153
builder.bodyEntry(Keys.GRANT_TYPE, GRANT_TYPE);

‎jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/auth/DataCloudTokenProcessor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ private static AuthenticationResponseWithError throwExceptionOnError(
160160
return response;
161161
}
162162

163-
public static DataCloudTokenProcessor of(Properties properties) throws SQLException {
163+
public static DataCloudTokenProcessor of(Properties properties) throws DataCloudJDBCException {
164164
val settings = AuthenticationSettings.of(properties);
165165
val strategy = AuthenticationStrategy.of(settings);
166166
val client = ClientBuilder.buildOkHttpClient(properties);

‎jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/auth/PrivateKeyHelpers.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public static Audience of(String url) throws SQLException {
5959

6060
@UtilityClass
6161
class JwtParts {
62-
public static String buildJwt(PrivateKeyAuthenticationSettings settings) throws SQLException {
62+
public static String buildJwt(PrivateKeyAuthenticationSettings settings) throws DataCloudJDBCException {
6363
try {
6464
Instant now = Instant.now();
6565
Audience audience = Audience.of(settings.getLoginUrl());

‎jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/DataCloudConnection.java‎

Lines changed: 13 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import com.salesforce.datacloud.jdbc.interceptor.DataspaceHeaderInterceptor;
3131
import com.salesforce.datacloud.jdbc.interceptor.HyperExternalClientContextHeaderInterceptor;
3232
import com.salesforce.datacloud.jdbc.interceptor.HyperWorkloadHeaderInterceptor;
33-
import com.salesforce.datacloud.jdbc.interceptor.TracingHeadersInterceptor;
3433
import com.salesforce.datacloud.jdbc.util.Unstable;
3534
import com.salesforce.datacloud.query.v3.DataCloudQueryStatus;
3635
import io.grpc.ClientInterceptor;
@@ -51,7 +50,6 @@
5150
import java.sql.Statement;
5251
import java.sql.Struct;
5352
import java.time.Duration;
54-
import java.util.ArrayList;
5553
import java.util.List;
5654
import java.util.Map;
5755
import java.util.Objects;
@@ -65,7 +63,6 @@
6563
import lombok.Builder;
6664
import lombok.Getter;
6765
import lombok.NonNull;
68-
import lombok.Setter;
6966
import lombok.extern.slf4j.Slf4j;
7067
import lombok.val;
7168

@@ -84,11 +81,7 @@ public class DataCloudConnection implements Connection, AutoCloseable {
8481
@NonNull @Builder.Default
8582
private final Properties properties = new Properties();
8683

87-
@Getter(AccessLevel.PACKAGE)
88-
@Setter
89-
@Builder.Default
90-
private List<ClientInterceptor> interceptors = new ArrayList<>();
91-
84+
@Unstable
9285
@Getter(AccessLevel.PACKAGE)
9386
@NonNull private final HyperGrpcClientExecutor executor;
9487

@@ -108,33 +101,12 @@ public static DataCloudConnection fromChannel(@NonNull ManagedChannelBuilder<?>
108101
.build();
109102
}
110103

111-
/** This flow is not supported by the JDBC Driver Manager, only use it if you know what you're doing. */
112-
public static DataCloudConnection fromTokenSupplier(
113-
AuthorizationHeaderInterceptor authInterceptor, @NonNull String host, int port, Properties properties)
114-
throws SQLException {
115-
val channel = ManagedChannelBuilder.forAddress(host, port);
116-
return fromTokenSupplier(authInterceptor, channel, properties);
117-
}
118-
119-
/** This flow is not supported by the JDBC Driver Manager, only use it if you know what you're doing. */
120-
public static DataCloudConnection fromTokenSupplier(
121-
AuthorizationHeaderInterceptor authInterceptor, ManagedChannelBuilder<?> builder, Properties properties)
122-
throws SQLException {
123-
val interceptors = getClientInterceptors(authInterceptor, properties);
124-
val executor = HyperGrpcClientExecutor.of(builder.intercept(interceptors), properties);
125-
126-
return DataCloudConnection.builder()
127-
.executor(executor)
128-
.properties(properties)
129-
.build();
130-
}
131-
132104
/**
133105
* Initializes a list of interceptors that handle channel level concerns that can be defined through properties
134106
* @param properties - The connection properties
135107
* @return a list of client interceptors
136108
*/
137-
static List<ClientInterceptor> getPropertyDerivedClientInterceptors(Properties properties) {
109+
private static List<ClientInterceptor> getPropertyDerivedClientInterceptors(Properties properties) {
138110
return Stream.of(
139111
HyperExternalClientContextHeaderInterceptor.of(properties),
140112
HyperWorkloadHeaderInterceptor.of(properties),
@@ -143,22 +115,13 @@ static List<ClientInterceptor> getPropertyDerivedClientInterceptors(Properties p
143115
.collect(Collectors.toList());
144116
}
145117

146-
/**
147-
* Initializes the full set of client interceptors from property handling to tracing and auth
148-
* @param authInterceptor an optional auth interceptor, is allowed to be null
149-
* @param properties the connection properties
150-
* @return a list of client interceptors
151-
*/
152-
static List<ClientInterceptor> getClientInterceptors(
153-
AuthorizationHeaderInterceptor authInterceptor, Properties properties) {
154-
val list = getPropertyDerivedClientInterceptors(properties);
155-
list.add(0, TracingHeadersInterceptor.of());
156-
if (authInterceptor != null) {
157-
list.add(0, authInterceptor);
118+
private static DataCloudTokenProcessor getDataCloudTokenProcessor(Properties properties)
119+
throws DataCloudJDBCException {
120+
if (!AuthenticationSettings.hasAny(properties)) {
121+
throw new DataCloudJDBCException("No authentication settings provided");
158122
}
159-
;
160-
log.info("Registering interceptor. interceptor={}", list);
161-
return list;
123+
124+
return DataCloudTokenProcessor.of(properties);
162125
}
163126

164127
public static DataCloudConnection of(String url, Properties properties) throws SQLException {
@@ -167,17 +130,15 @@ public static DataCloudConnection of(String url, Properties properties) throws S
167130
connectionString.withParameters(properties);
168131
properties.setProperty(LOGIN_URL, connectionString.getLoginUrl());
169132

170-
if (!AuthenticationSettings.hasAny(properties)) {
171-
throw new DataCloudJDBCException("No authentication settings provided");
172-
}
173-
174-
val tokenProcessor = DataCloudTokenProcessor.of(properties);
133+
val tokenProcessor = getDataCloudTokenProcessor(properties);
134+
val authInterceptor = AuthorizationHeaderInterceptor.of(tokenProcessor);
175135

176136
val host = tokenProcessor.getDataCloudToken().getTenantUrl();
177137
val builder = ManagedChannelBuilder.forAddress(host, DEFAULT_PORT);
178-
val authInterceptor = AuthorizationHeaderInterceptor.of(tokenProcessor);
179138

180-
val interceptors = getClientInterceptors(authInterceptor, properties);
139+
val interceptors = getPropertyDerivedClientInterceptors(properties);
140+
interceptors.add(0, authInterceptor);
141+
181142
val executor = HyperGrpcClientExecutor.of(builder.intercept(interceptors), properties);
182143

183144
return DataCloudConnection.builder()

‎jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/DataCloudPreparedStatement.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
import java.util.Calendar;
5454
import java.util.Map;
5555
import java.util.TimeZone;
56-
import lombok.SneakyThrows;
5756
import lombok.experimental.UtilityClass;
5857
import lombok.extern.slf4j.Slf4j;
5958
import lombok.val;
@@ -97,9 +96,7 @@ public boolean execute(String sql) throws SQLException {
9796
"Per the JDBC specification this method cannot be called on a PreparedStatement, use DataCloudPreparedStatement::execute() instead.");
9897
}
9998

100-
@Override
101-
@SneakyThrows
102-
protected HyperGrpcClientExecutor getQueryExecutor() {
99+
private HyperGrpcClientExecutor getQueryExecutor() throws DataCloudJDBCException {
103100
final byte[] encodedRow;
104101
try {
105102
encodedRow = ArrowUtils.toArrowByteArray(parameterManager.getParameters(), calendar);
@@ -114,7 +111,10 @@ protected HyperGrpcClientExecutor getQueryExecutor() {
114111
.build())
115112
.build();
116113

117-
return getQueryExecutor(preparedQueryParams);
114+
return dataCloudConnection.getExecutor().toBuilder()
115+
.additionalQueryParams(preparedQueryParams)
116+
.queryTimeout(getQueryTimeout())
117+
.build();
118118
}
119119

120120
@Override

‎jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/core/DataCloudStatement.java‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import lombok.NonNull;
3636
import lombok.extern.slf4j.Slf4j;
3737
import lombok.val;
38-
import salesforce.cdp.hyperdb.v1.QueryParam;
3938

4039
@Slf4j
4140
public class DataCloudStatement implements Statement, AutoCloseable {
@@ -59,20 +58,10 @@ public DataCloudStatement(@NonNull DataCloudConnection connection) {
5958

6059
protected QueryStatusListener listener;
6160

62-
protected HyperGrpcClientExecutor getQueryExecutor() {
63-
return getQueryExecutor(null);
64-
}
65-
66-
protected HyperGrpcClientExecutor getQueryExecutor(QueryParam additionalQueryParams) {
67-
val clientBuilder = dataCloudConnection.getExecutor().toBuilder();
68-
69-
clientBuilder.interceptors(dataCloudConnection.getInterceptors());
70-
71-
if (additionalQueryParams != null) {
72-
clientBuilder.additionalQueryParams(additionalQueryParams);
73-
}
74-
75-
return clientBuilder.queryTimeout(getQueryTimeout()).build();
61+
private HyperGrpcClientExecutor getQueryExecutor() {
62+
return dataCloudConnection.getExecutor().toBuilder()
63+
.queryTimeout(getQueryTimeout())
64+
.build();
7665
}
7766

7867
private void assertQueryExecuted() throws SQLException {

‎jdbc-core/src/main/java/com/salesforce/datacloud/jdbc/interceptor/HeaderMutatingClientInterceptor.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import io.grpc.MethodDescriptor;
2626
import lombok.SneakyThrows;
2727

28+
@FunctionalInterface
2829
public interface HeaderMutatingClientInterceptor extends ClientInterceptor {
2930
void mutate(final Metadata headers);
3031

‎jdbc-core/src/test/java/com/salesforce/datacloud/jdbc/core/DataCloudStatementTest.java‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.salesforce.datacloud.jdbc.exception.DataCloudJDBCException;
2626
import com.salesforce.datacloud.jdbc.util.Constants;
2727
import com.salesforce.datacloud.jdbc.util.GrpcUtils;
28-
import com.salesforce.datacloud.jdbc.util.RequestRecordingInterceptor;
2928
import com.salesforce.datacloud.jdbc.util.SqlErrorCodes;
3029
import io.grpc.StatusRuntimeException;
3130
import java.sql.ResultSet;
@@ -134,23 +133,6 @@ public void testExecute() {
134133
}
135134
}
136135

137-
@Test
138-
@SneakyThrows
139-
public void testExecuteQueryIncludesInterceptorsProvidedByCaller() {
140-
setupHyperGrpcClientWithMockedResultSet("abc", ImmutableList.of());
141-
val interceptor = new RequestRecordingInterceptor();
142-
Mockito.when(connection.getInterceptors()).thenReturn(ImmutableList.of(interceptor));
143-
144-
assertThat(interceptor.getQueries().size()).isEqualTo(0);
145-
statement.executeQuery("SELECT * FROM table");
146-
assertThat(interceptor.getQueries().size()).isEqualTo(1);
147-
statement.executeQuery("SELECT * FROM table");
148-
assertThat(interceptor.getQueries().size()).isEqualTo(2);
149-
statement.executeQuery("SELECT * FROM table");
150-
assertThat(interceptor.getQueries().size()).isEqualTo(3);
151-
assertDoesNotThrow(() -> statement.close());
152-
}
153-
154136
@Test
155137
public void testExecuteQueryWithSqlException() {
156138
StatusRuntimeException fakeException = GrpcUtils.getFakeStatusRuntimeExceptionAsInvalidArgument();

‎jdbc-core/src/test/java/com/salesforce/datacloud/jdbc/hyper/HyperServerProcess.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,10 @@ public DataCloudConnection getConnection(Map<String, String> connectionSettings)
149149
properties.putAll(connectionSettings);
150150
val auth = AuthorizationHeaderInterceptor.of(new HyperTestBase.NoopTokenSupplier());
151151
log.info("Creating connection to port {}", getPort());
152-
ManagedChannelBuilder<?> channel =
153-
ManagedChannelBuilder.forAddress("127.0.0.1", getPort()).usePlaintext();
152+
ManagedChannelBuilder<?> channel = ManagedChannelBuilder.forAddress("127.0.0.1", getPort())
153+
.intercept(auth)
154+
.usePlaintext();
154155

155-
return DataCloudConnection.fromTokenSupplier(auth, channel, properties);
156+
return DataCloudConnection.fromChannel(channel, properties);
156157
}
157158
}

0 commit comments

Comments
 (0)