-
Notifications
You must be signed in to change notification settings - Fork 29
Upgrade org.eclipse.jetty to 12.1.10 #365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,12 +21,11 @@ | |||||||
| import io.cdap.plugin.salesforce.authenticator.AuthenticatorCredentials; | ||||||||
| import io.cdap.plugin.salesforce.parser.SalesforceQueryParser; | ||||||||
| import io.cdap.plugin.salesforce.plugin.OAuthInfo; | ||||||||
| import org.eclipse.jetty.client.ContentResponse; | ||||||||
| import org.eclipse.jetty.client.HttpClient; | ||||||||
| import org.eclipse.jetty.client.api.ContentResponse; | ||||||||
| import org.eclipse.jetty.client.api.Request; | ||||||||
| import org.eclipse.jetty.client.Request; | ||||||||
| import org.eclipse.jetty.http.HttpHeader; | ||||||||
| import org.eclipse.jetty.http.HttpMethod; | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||
| import org.eclipse.jetty.util.ssl.SslContextFactory; | ||||||||
|
|
||||||||
| import java.io.IOException; | ||||||||
| import java.net.HttpURLConnection; | ||||||||
|
|
@@ -152,8 +151,7 @@ public static QueryPlanResponse getQueryPlan( | |||||||
| SalesforceConstants.API_VERSION, | ||||||||
| URLEncoder.encode(query, "UTF-8")); | ||||||||
|
|
||||||||
| SslContextFactory sslContextFactory = new SslContextFactory(); | ||||||||
| HttpClient httpClient = new HttpClient(sslContextFactory); | ||||||||
| HttpClient httpClient = new HttpClient(); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Jetty 12,
Suggested change
|
||||||||
| httpClient.setConnectTimeout(credentials.getConnectTimeout()); | ||||||||
| if (!Strings.isNullOrEmpty(credentials.getProxyUrl())) { | ||||||||
| Authenticator.setProxy(credentials, httpClient); | ||||||||
|
|
@@ -163,12 +161,9 @@ public static QueryPlanResponse getQueryPlan( | |||||||
| httpClient.start(); | ||||||||
| Request request = httpClient.newRequest(explainUrl) | ||||||||
| .method(HttpMethod.GET) | ||||||||
| .header( | ||||||||
| HttpHeader.AUTHORIZATION, | ||||||||
| "Bearer " + oAuthInfo.getAccessToken()) | ||||||||
| .header( | ||||||||
| HttpHeader.CONTENT_TYPE, | ||||||||
| "application/json"); | ||||||||
| .headers(headers -> headers | ||||||||
| .put(HttpHeader.AUTHORIZATION, "Bearer " + oAuthInfo.getAccessToken()) | ||||||||
| .put(HttpHeader.CONTENT_TYPE, "application/json")); | ||||||||
| ContentResponse response = request.send(); | ||||||||
| String responseContent = response.getContentAsString(); | ||||||||
| if (response.getStatus() != HttpURLConnection.HTTP_OK) { | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,8 +25,7 @@ | |||||||
| import org.eclipse.jetty.client.HttpClient; | ||||||||
| import org.eclipse.jetty.client.HttpProxy; | ||||||||
| import org.eclipse.jetty.client.ProxyConfiguration; | ||||||||
| import org.eclipse.jetty.client.api.Request; | ||||||||
| import org.eclipse.jetty.util.ssl.SslContextFactory; | ||||||||
| import org.eclipse.jetty.client.Request; | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||
|
|
||||||||
| import java.net.URI; | ||||||||
| import java.net.URISyntaxException; | ||||||||
|
|
@@ -89,8 +88,7 @@ public static OAuthInfo getOAuthInfo(AuthenticatorCredentials credentials) throw | |||||||
| throw new IllegalArgumentException("Grant type cannot be null for OAuth flow to fetch access token."); | ||||||||
| } | ||||||||
|
|
||||||||
| SslContextFactory sslContextFactory = new SslContextFactory(); | ||||||||
| HttpClient httpClient = new HttpClient(sslContextFactory); | ||||||||
| HttpClient httpClient = new HttpClient(); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Jetty 12,
Suggested change
|
||||||||
| httpClient.setConnectTimeout(credentials.getConnectTimeout()); | ||||||||
| if (!Strings.isNullOrEmpty(credentials.getProxyUrl())) { | ||||||||
| setProxy(credentials, httpClient); | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -32,8 +32,7 @@ | |||||||
| import org.cometd.common.JSONContext; | ||||||||
| import org.cometd.common.JacksonJSONContextClient; | ||||||||
| import org.eclipse.jetty.client.HttpClient; | ||||||||
| import org.eclipse.jetty.client.api.Request; | ||||||||
| import org.eclipse.jetty.util.ssl.SslContextFactory; | ||||||||
| import org.eclipse.jetty.client.Request; | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||
| import org.slf4j.Logger; | ||||||||
| import org.slf4j.LoggerFactory; | ||||||||
|
|
||||||||
|
|
@@ -129,10 +128,8 @@ public String getMessage(long timeout, TimeUnit unit) throws InterruptedExceptio | |||||||
| private BayeuxClient getClient(AuthenticatorCredentials credentials) throws Exception { | ||||||||
| OAuthInfo oAuthInfo = Authenticator.getOAuthInfo(credentials); | ||||||||
|
|
||||||||
| SslContextFactory sslContextFactory = new SslContextFactory(); | ||||||||
|
|
||||||||
| // Set up a Jetty HTTP client to use with CometD | ||||||||
| HttpClient httpClient = new HttpClient(sslContextFactory); | ||||||||
| HttpClient httpClient = new HttpClient(); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In Jetty 12,
Suggested change
|
||||||||
| httpClient.setConnectTimeout(CONNECTION_TIMEOUT_MS); | ||||||||
| if (!Strings.isNullOrEmpty(credentials.getProxyUrl())) { | ||||||||
| Authenticator.setProxy(credentials, httpClient); | ||||||||
|
|
@@ -146,15 +143,15 @@ private BayeuxClient getClient(AuthenticatorCredentials credentials) throws Exce | |||||||
| Map<String, Object> transportOptions = new HashMap<>(); | ||||||||
| transportOptions.put(ClientTransport.JSON_CONTEXT_OPTION, jsonContext); | ||||||||
|
|
||||||||
| // Adds the OAuth header in LongPollingTransport | ||||||||
| LongPollingTransport transport = new LongPollingTransport( | ||||||||
| transportOptions, httpClient) { | ||||||||
| // Adds the OAuth header for all CometD requests | ||||||||
| httpClient.getRequestListeners().addListener(new Request.Listener() { | ||||||||
| @Override | ||||||||
| protected void customize(Request exchange) { | ||||||||
| super.customize(exchange); | ||||||||
| exchange.header("Authorization", "OAuth " + oAuthInfo.getAccessToken()); | ||||||||
| public void onBegin(Request request) { | ||||||||
| request.headers(headers -> headers.put("Authorization", "OAuth " + oAuthInfo.getAccessToken())); | ||||||||
| } | ||||||||
| }; | ||||||||
| }); | ||||||||
|
|
||||||||
| LongPollingTransport transport = new LongPollingTransport(transportOptions, httpClient); | ||||||||
|
|
||||||||
| // Now set up the Bayeux client itself | ||||||||
| return new BayeuxClient(oAuthInfo.getInstanceURL() + DEFAULT_PUSH_ENDPOINT, transport); | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,12 +20,11 @@ | |||||||||
| import io.cdap.plugin.salesforce.authenticator.AuthenticatorCredentials; | ||||||||||
| import io.cdap.plugin.salesforce.plugin.OAuthInfo; | ||||||||||
| import io.cdap.plugin.salesforce.plugin.source.batch.util.SalesforceSplitUtil; | ||||||||||
| import org.eclipse.jetty.client.ContentResponse; | ||||||||||
| import org.eclipse.jetty.client.HttpClient; | ||||||||||
| import org.eclipse.jetty.client.api.ContentResponse; | ||||||||||
| import org.eclipse.jetty.client.api.Request; | ||||||||||
| import org.eclipse.jetty.client.Request; | ||||||||||
| import org.eclipse.jetty.http.HttpHeader; | ||||||||||
| import org.eclipse.jetty.http.HttpMethod; | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||
| import org.eclipse.jetty.util.ssl.SslContextFactory; | ||||||||||
| import org.junit.Assert; | ||||||||||
| import org.junit.Test; | ||||||||||
| import org.junit.runner.RunWith; | ||||||||||
|
|
@@ -296,13 +295,12 @@ public void getQueryPlan_success_returnsQueryPlanResponse() throws Exception { | |||||||||
| PowerMockito.mockStatic(Authenticator.class); | ||||||||||
| PowerMockito.when(Authenticator.getOAuthInfo(credentials)).thenReturn(oAuthInfo); | ||||||||||
| HttpClient httpClient = PowerMockito.mock(HttpClient.class); | ||||||||||
| PowerMockito.whenNew(HttpClient.class).withArguments(Mockito.any(SslContextFactory.class)) | ||||||||||
| PowerMockito.whenNew(HttpClient.class).withNoArguments() | ||||||||||
| .thenReturn(httpClient); | ||||||||||
|
Comment on lines
+298
to
299
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since
Suggested change
|
||||||||||
| Request request = Mockito.mock(Request.class); | ||||||||||
| Mockito.when(httpClient.newRequest(Mockito.anyString())).thenReturn(request); | ||||||||||
| Mockito.when(request.method(Mockito.any(HttpMethod.class))).thenReturn(request); | ||||||||||
| Mockito.when(request.header(Mockito.any(HttpHeader.class), Mockito.anyString())) | ||||||||||
| .thenReturn(request); | ||||||||||
| Mockito.when(request.headers(Mockito.any())).thenReturn(request); | ||||||||||
| ContentResponse response = Mockito.mock(ContentResponse.class); | ||||||||||
| Mockito.when(request.send()).thenReturn(response); | ||||||||||
| Mockito.when(response.getStatus()).thenReturn(HttpURLConnection.HTTP_OK); | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excluding Jetty 9.4.x transitive dependencies from
cometd-java-clientversion4.0.0and forcing it to run with Jetty 12 will cause critical runtime failures (such asNoClassDefFoundErrororNoSuchMethodError). CometD 4.x is compiled against Jetty 9.4.x APIs (e.g., it referencesorg.eclipse.jetty.client.api.Requestwhich has been refactored/removed in Jetty 12). To support Jetty 12, you must upgradecometd-java-clientto a compatible version (such as CometD 7.x or 8.x depending on the Jakarta EE namespace used by the project).