Skip to content

Commit a664e44

Browse files
srnagarsrnagar
authored
ClientCore: API review updates (Azure#44655)
* ClientCore: API review updates * pr comments * fix unit tests * update okhttp configuration * remove duplicate entry in version_client.txt
1 parent 555746a commit a664e44

File tree

10 files changed

+65
-80
lines changed

10 files changed

+65
-80
lines changed

‎sdk/clientcore/core/src/main/java/io/clientcore/core/traits/HttpTrait.java‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -52,23 +52,6 @@ public interface HttpTrait<T extends HttpTrait<T>> {
5252
*/
5353
T httpClient(HttpClient client);
5454

55-
/**
56-
* Sets the {@link HttpPipeline} to use for the service client.
57-
*
58-
* <p><strong>Note:</strong> It is important to understand the precedence order of the {@link HttpTrait} APIs. In
59-
* particular, if a {@link HttpPipeline} is specified, this takes precedence over all other APIs in the trait, and
60-
* they will be ignored. If no {@link HttpPipeline} is specified, an HTTP pipeline will be constructed internally
61-
* based on the settings provided to this trait. Additionally, there may be other APIs in types that implement this
62-
* trait that are also ignored if an {@link HttpPipeline} is specified, so please be sure to refer to the
63-
* documentation of types that implement this trait to understand the full set of implications.</p>
64-
*
65-
* @param pipeline {@link HttpPipeline} to use for sending service requests and receiving responses.
66-
*
67-
* @return Returns the same concrete type with the appropriate properties updated, to allow for fluent chaining of
68-
* operations.
69-
*/
70-
T httpPipeline(HttpPipeline pipeline);
71-
7255
/**
7356
* Adds a {@link HttpPipelinePolicy pipeline policy} to apply on each request sent.
7457
*

‎sdk/clientcore/core/src/main/java/io/clientcore/core/utils/configuration/Configuration.java‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
import io.clientcore.core.http.client.HttpClient;
77
import io.clientcore.core.http.client.HttpClientProvider;
88

9-
import java.util.Arrays;
9+
import java.util.ArrayList;
10+
import java.util.Collections;
1011
import java.util.List;
1112

1213
/**
@@ -92,30 +93,47 @@ public final class Configuration {
9293
* The global configuration shared by all client libraries.
9394
*/
9495
private static final Configuration GLOBAL_CONFIGURATION
95-
= new Configuration(new SystemPropertiesConfigurationSource(), new EnvironmentVariableConfigurationSource());
96+
= Configuration.from(new SystemPropertiesConfigurationSource(), new EnvironmentVariableConfigurationSource());
9697

9798
/*
9899
* The configuration that never returns any configuration values.
99100
*/
100-
private static final Configuration NONE = new Configuration();
101+
private static final Configuration NONE = new Configuration(Collections.emptyList());
101102

102103
private final List<ConfigurationSource> sources;
103104

105+
private Configuration(List<ConfigurationSource> sources) {
106+
this.sources = sources;
107+
}
108+
104109
/**
105110
* Creates an instance of {@link Configuration} with the given {@link ConfigurationSource}.
106111
* <p>
107112
* The {@code sources} will be queried in the order they are provided.
108113
* <p>
109-
* If no {@code sources} are provided all calls to get configurations will return null.
114+
* If {@code sources} are empty, all calls to get configurations will return null.
110115
*
111116
* @param sources The configuration sources to use.
117+
* @return an instance of {@link Configuration} initialized with the provided configuration sources.
118+
* @throws IllegalArgumentException If the sources are <code>null</code> or any entry in the sources array is <code>null</code>.
112119
*/
113-
public Configuration(ConfigurationSource... sources) {
114-
if (sources == null || sources.length == 0) {
115-
this.sources = null;
116-
} else {
117-
this.sources = Arrays.asList(Arrays.copyOf(sources, sources.length));
120+
public static Configuration from(ConfigurationSource... sources) {
121+
if (sources == null) {
122+
throw new IllegalArgumentException("Sources cannot be null");
123+
}
124+
125+
if (sources.length == 0) {
126+
return NONE;
127+
}
128+
129+
List<ConfigurationSource> sourceList = new ArrayList<>(sources.length);
130+
for (ConfigurationSource source : sources) {
131+
if (source == null) {
132+
throw new IllegalArgumentException("Source cannot be null");
133+
}
134+
sourceList.add(source);
118135
}
136+
return new Configuration(sourceList);
119137
}
120138

121139
/**

‎sdk/clientcore/core/src/test/java/io/clientcore/core/http/client/JdkHttpClientBuilderIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ public void buildWithHttpProxy() throws IOException {
8686

8787
@Test
8888
public void buildWithHttpProxyFromEnvConfiguration() throws IOException {
89-
Configuration configuration = new Configuration(new TestConfigurationSource()
89+
Configuration configuration = Configuration.from(new TestConfigurationSource()
9090
.put(Configuration.HTTP_PROXY,
9191
"http://" + PROXY_USER_INFO + proxyEndpoint.getHost() + ":" + proxyEndpoint.getPort())
9292
.put("java.net.useSystemProxies", "true"));

‎sdk/clientcore/core/src/test/java/io/clientcore/core/http/models/ProxyOptionsTests.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public void loadFromExplicitUnresolved(Configuration configuration, String expec
140140

141141
@Test
142142
public void mixedExplicitAndEnvironmentConfigurationIsNotSupported() {
143-
Configuration configuration = new Configuration(new TestConfigurationSource().put("https.proxyHost", "ignored")
143+
Configuration configuration = Configuration.from(new TestConfigurationSource().put("https.proxyHost", "ignored")
144144
.put("https.proxyPort", "42")
145145
.put("http.proxy.username", FAKE_PROXY_USER_PLACEHOLDER)
146146
.put("http.proxy.password", FAKE_PROXY_PASSWORD_PLACEHOLDER)
@@ -186,7 +186,7 @@ public void defaultHttpPortNull(String port) {
186186
configurationSource.put("http.proxy.port", port);
187187
}
188188

189-
ProxyOptions proxyOptions = fromConfiguration(new Configuration(configurationSource));
189+
ProxyOptions proxyOptions = fromConfiguration(Configuration.from(configurationSource));
190190

191191
assertNotNull(proxyOptions);
192192
assertEquals(443, proxyOptions.getAddress().getPort());
@@ -196,7 +196,7 @@ public void defaultHttpPortNull(String port) {
196196
@ValueSource(strings = { "", " ", "not-an-int" })
197197
public void invalidHttpPortExplicitConfigThrows(String port) {
198198
Configuration configuration
199-
= new Configuration(new TestConfigurationSource().put("http.proxy.hostname", PROXY_HOST)
199+
= Configuration.from(new TestConfigurationSource().put("http.proxy.hostname", PROXY_HOST)
200200
.put("http.proxy.username", FAKE_PROXY_USER_PLACEHOLDER)
201201
.put("http.proxy.password", FAKE_PROXY_PASSWORD_PLACEHOLDER)
202202
.put("http.proxy.port", port));
@@ -210,7 +210,7 @@ public void invalidHttpsPortEnvironmentConfigDefault(String port) {
210210
ConfigurationSource systemProps
211211
= new TestConfigurationSource().put("https.proxyHost", PROXY_HOST).put("https.proxyPort", port);
212212

213-
Configuration configuration = new Configuration(EMPTY_SOURCE, systemProps, EMPTY_SOURCE);
213+
Configuration configuration = Configuration.from(EMPTY_SOURCE, systemProps, EMPTY_SOURCE);
214214

215215
ProxyOptions proxyOptions = fromConfiguration(configuration);
216216

@@ -224,7 +224,7 @@ public void invalidHttpPortEnvironmentConfigDefault(String port) {
224224
ConfigurationSource systemProps
225225
= new TestConfigurationSource().put("http.proxyHost", PROXY_HOST).put("http.proxyPort", port);
226226

227-
Configuration configuration = new Configuration(EMPTY_SOURCE, systemProps, EMPTY_SOURCE);
227+
Configuration configuration = Configuration.from(EMPTY_SOURCE, systemProps, EMPTY_SOURCE);
228228

229229
ProxyOptions proxyOptions = fromConfiguration(configuration);
230230

@@ -378,10 +378,10 @@ private static Stream<Arguments> systemProxiesRequireUseSystemProxiesSupplier()
378378

379379
return Stream.of(
380380
// Java HTTPS configuration without 'java.net.useSystemProxies' set.
381-
Arguments.of(new Configuration(EMPTY_SOURCE, EMPTY_SOURCE, envVarHttpsSource)),
381+
Arguments.of(Configuration.from(EMPTY_SOURCE, EMPTY_SOURCE, envVarHttpsSource)),
382382

383383
// Java HTTP configuration without 'java.net.useSystemProxies' set.
384-
Arguments.of(new Configuration(EMPTY_SOURCE, EMPTY_SOURCE, envVarHttpSource)));
384+
Arguments.of(Configuration.from(EMPTY_SOURCE, EMPTY_SOURCE, envVarHttpSource)));
385385
}
386386

387387
private static Configuration createJavaEnvConfiguration(int port, String username, String password,
@@ -400,7 +400,7 @@ private static Configuration createJavaEnvConfiguration(int port, String usernam
400400
.put(JAVA_HTTP_PROXY_PASSWORD, password);
401401
}
402402

403-
return new Configuration(EMPTY_SOURCE, testSource, EMPTY_SOURCE);
403+
return Configuration.from(EMPTY_SOURCE, testSource, EMPTY_SOURCE);
404404
}
405405

406406
private static Configuration createExplicitConfiguration(int port, String username, String password,
@@ -412,7 +412,7 @@ private static Configuration createExplicitConfiguration(int port, String userna
412412
.put("http.proxy.username", username)
413413
.put("http.proxy.password", password);
414414

415-
return new Configuration(explicitSource, EMPTY_SOURCE, EMPTY_SOURCE);
415+
return Configuration.from(explicitSource, EMPTY_SOURCE, EMPTY_SOURCE);
416416
}
417417

418418
@ParameterizedTest
@@ -435,7 +435,7 @@ private static Stream<Arguments> nonProxyHostsSupplier() {
435435
TestConfigurationSource sysPropSource = new TestConfigurationSource().put("http.proxyHost", "localhost")
436436
.put("http.proxyPort", "7777")
437437
.put("http.nonProxyHosts", javaNonProxyHosts);
438-
Configuration javaProxyConfiguration = new Configuration(EMPTY_SOURCE, sysPropSource, EMPTY_SOURCE);
438+
Configuration javaProxyConfiguration = Configuration.from(EMPTY_SOURCE, sysPropSource, EMPTY_SOURCE);
439439

440440
Pattern javaProxyConfigurationPattern
441441
= compile(fromConfiguration(javaProxyConfiguration).getNonProxyHosts(), Pattern.CASE_INSENSITIVE);
@@ -507,6 +507,6 @@ public void sanitizeJavaHttpNonProxyHostsDoesNotSplitEscapedPipes() {
507507
}
508508

509509
private static Configuration setJavaSystemProxyPrerequisiteToTrue(TestConfigurationSource source) {
510-
return new Configuration(EMPTY_SOURCE, EMPTY_SOURCE, source.put(JAVA_SYSTEM_PROXY_PREREQUISITE, "true"));
510+
return Configuration.from(EMPTY_SOURCE, EMPTY_SOURCE, source.put(JAVA_SYSTEM_PROXY_PREREQUISITE, "true"));
511511
}
512512
}

‎sdk/clientcore/core/src/test/java/io/clientcore/core/implementation/utils/ImplUtilsTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,23 @@ private static Stream<Arguments> getDefaultTimeoutFromEnvironmentSupplier() {
101101

102102
return Stream.of(
103103
// Configuration has an empty string timeout property configured.
104-
Arguments.of(new Configuration(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "")),
104+
Arguments.of(Configuration.from(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "")),
105105
Duration.ofMillis(10000), logger, Duration.ofMillis(10000)),
106106

107107
// Configuration has a value that isn't a valid number.
108-
Arguments.of(new Configuration(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "ten")),
108+
Arguments.of(Configuration.from(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "ten")),
109109
Duration.ofMillis(10000), logger, Duration.ofMillis(10000)),
110110

111111
// Configuration has a negative value.
112-
Arguments.of(new Configuration(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "-10")),
112+
Arguments.of(Configuration.from(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "-10")),
113113
Duration.ofMillis(10000), logger, Duration.ZERO),
114114

115115
// Configuration has a zero value.
116-
Arguments.of(new Configuration(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "0")),
116+
Arguments.of(Configuration.from(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "0")),
117117
Duration.ofMillis(10000), logger, Duration.ZERO),
118118

119119
// Configuration has a positive value.
120-
Arguments.of(new Configuration(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "42")),
120+
Arguments.of(Configuration.from(new TestConfigurationSource().put(TIMEOUT_PROPERTY_NAME, "42")),
121121
Duration.ofMillis(10000), logger, Duration.ofMillis(42)));
122122
}
123123

‎sdk/clientcore/http-okhttp3/src/test/java/io/clientcore/http/okhttp3/OkHttpHttpClientBuilderTests.java‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -491,24 +491,24 @@ private static Stream<Arguments> buildWithEnvConfigurationProxySupplier() {
491491
/*
492492
* Simple non-authenticated HTTP proxies.
493493
*/
494-
arguments.add(Arguments.of(true, new Configuration(baseJavaProxyConfigurationSupplier.get()), defaultUri));
494+
arguments.add(Arguments.of(true, Configuration.from(baseJavaProxyConfigurationSupplier.get()), defaultUri));
495495

496496
Configuration simpleEnvProxy
497-
= new Configuration(new TestConfigurationSource().put(Configuration.HTTP_PROXY, "http://localhost:12345")
497+
= Configuration.from(new TestConfigurationSource().put(Configuration.HTTP_PROXY, "http://localhost:12345")
498498
.put(JAVA_SYSTEM_PROXY_PREREQUISITE, "true"));
499499

500500
arguments.add(Arguments.of(true, simpleEnvProxy, defaultUri));
501501

502502
/*
503503
* HTTP proxy with authentication configured.
504504
*/
505-
Configuration javaProxyWithAuthentication = new Configuration(EMPTY_SOURCE,
505+
Configuration javaProxyWithAuthentication = Configuration.from(EMPTY_SOURCE,
506506
baseJavaProxyConfigurationSupplier.get().put(JAVA_HTTP_PROXY_USER, "1").put(JAVA_HTTP_PROXY_PASSWORD, "1"),
507507
EMPTY_SOURCE);
508508

509509
arguments.add(Arguments.of(true, javaProxyWithAuthentication, defaultUri));
510510

511-
Configuration envProxyWithAuthentication = new Configuration(EMPTY_SOURCE, EMPTY_SOURCE,
511+
Configuration envProxyWithAuthentication = Configuration.from(EMPTY_SOURCE, EMPTY_SOURCE,
512512
new TestConfigurationSource().put(Configuration.HTTP_PROXY, "http://1:1@localhost:12345")
513513
.put(JAVA_SYSTEM_PROXY_PREREQUISITE, "true"));
514514

@@ -542,12 +542,12 @@ private static Stream<Arguments> buildWithEnvConfigurationProxySupplier() {
542542
for (Supplier<TestConfigurationSource> configurationSupplier : nonProxyHostsSuppliers) {
543543
for (String requestUri : requestUrisWithoutProxying) {
544544
arguments.add(Arguments.of(false,
545-
new Configuration(EMPTY_SOURCE, configurationSupplier.get(), EMPTY_SOURCE), requestUri));
545+
Configuration.from(EMPTY_SOURCE, configurationSupplier.get(), EMPTY_SOURCE), requestUri));
546546
}
547547

548548
for (String requestUri : requestUrisWithProxying) {
549549
arguments.add(Arguments.of(true,
550-
new Configuration(EMPTY_SOURCE, configurationSupplier.get(), EMPTY_SOURCE), requestUri));
550+
Configuration.from(EMPTY_SOURCE, configurationSupplier.get(), EMPTY_SOURCE), requestUri));
551551
}
552552
}
553553

@@ -566,12 +566,12 @@ private static Stream<Arguments> buildWithEnvConfigurationProxySupplier() {
566566
for (Supplier<TestConfigurationSource> configurationSupplier : authenticatedNonProxyHostsSuppliers) {
567567
for (String requestUri : requestUrisWithoutProxying) {
568568
arguments.add(Arguments.of(false,
569-
new Configuration(EMPTY_SOURCE, configurationSupplier.get(), EMPTY_SOURCE), requestUri));
569+
Configuration.from(EMPTY_SOURCE, configurationSupplier.get(), EMPTY_SOURCE), requestUri));
570570
}
571571

572572
for (String requestUri : requestUrisWithProxying) {
573573
arguments.add(Arguments.of(true,
574-
new Configuration(EMPTY_SOURCE, configurationSupplier.get(), EMPTY_SOURCE), requestUri));
574+
Configuration.from(EMPTY_SOURCE, configurationSupplier.get(), EMPTY_SOURCE), requestUri));
575575
}
576576
}
577577

@@ -588,13 +588,13 @@ private static Stream<Arguments> buildWithExplicitConfigurationProxySupplier() {
588588
/*
589589
* Simple non-authenticated HTTP proxies.
590590
*/
591-
arguments.add(Arguments.of(true, new Configuration(baseHttpProxy.get()), defaultUri));
591+
arguments.add(Arguments.of(true, Configuration.from(baseHttpProxy.get()), defaultUri));
592592

593593
/*
594594
* HTTP proxy with authentication configured.
595595
*/
596596
Configuration httpProxyWithAuthentication
597-
= new Configuration(baseHttpProxy.get().put("http.proxy.username", "1").put("http.proxy.password", "1"));
597+
= Configuration.from(baseHttpProxy.get().put("http.proxy.username", "1").put("http.proxy.password", "1"));
598598

599599
arguments.add(Arguments.of(true, httpProxyWithAuthentication, defaultUri));
600600

@@ -617,11 +617,11 @@ private static Stream<Arguments> buildWithExplicitConfigurationProxySupplier() {
617617
Supplier<TestConfigurationSource> javaNonProxyHostsSupplier
618618
= () -> baseHttpProxy.get().put("http.proxy.non-proxy-hosts", rawJavaNonProxyHosts);
619619
for (String requestUri : requestUrisWithoutProxying) {
620-
arguments.add(Arguments.of(false, new Configuration(javaNonProxyHostsSupplier.get()), requestUri));
620+
arguments.add(Arguments.of(false, Configuration.from(javaNonProxyHostsSupplier.get()), requestUri));
621621
}
622622

623623
for (String requestUri : requestUrisWithProxying) {
624-
arguments.add(Arguments.of(true, new Configuration(javaNonProxyHostsSupplier.get()), requestUri));
624+
arguments.add(Arguments.of(true, Configuration.from(javaNonProxyHostsSupplier.get()), requestUri));
625625
}
626626

627627
/*
@@ -632,12 +632,12 @@ private static Stream<Arguments> buildWithExplicitConfigurationProxySupplier() {
632632

633633
for (String requestUri : requestUrisWithoutProxying) {
634634
arguments
635-
.add(Arguments.of(false, new Configuration(authenticatedJavaNonProxyHostsSupplier.get()), requestUri));
635+
.add(Arguments.of(false, Configuration.from(authenticatedJavaNonProxyHostsSupplier.get()), requestUri));
636636
}
637637

638638
for (String requestUri : requestUrisWithProxying) {
639639
arguments
640-
.add(Arguments.of(true, new Configuration(authenticatedJavaNonProxyHostsSupplier.get()), requestUri));
640+
.add(Arguments.of(true, Configuration.from(authenticatedJavaNonProxyHostsSupplier.get()), requestUri));
641641
}
642642

643643
return arguments.stream();

‎sdk/identity/azure-identity-v2/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@
5555

5656

5757
<dependencies>
58+
<dependency>
59+
<groupId>io.clientcore</groupId>
60+
<artifactId>core</artifactId>
61+
<version>1.0.0-beta.8</version> <!-- {x-version-update;unreleased_io.clientcore:core;dependency} -->
62+
</dependency>
5863
<dependency>
5964
<groupId>com.azure.v2</groupId>
6065
<artifactId>azure-core</artifactId>

‎sdk/identity/azure-identity-v2/src/main/java/com/azure/identity/v2/EntraIdCredentialBuilderBase.java‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import com.azure.identity.v2.implementation.util.IdentityUtil;
77
import com.azure.identity.v2.implementation.util.ValidationUtil;
88
import io.clientcore.core.http.client.HttpClient;
9-
import io.clientcore.core.http.pipeline.HttpPipeline;
109
import io.clientcore.core.http.pipeline.HttpPipelinePolicy;
1110
import io.clientcore.core.http.pipeline.HttpInstrumentationOptions;
1211
import io.clientcore.core.http.pipeline.HttpRedirectOptions;
@@ -79,26 +78,6 @@ public T disableInstanceDiscovery() {
7978
return (T) this;
8079
}
8180

82-
/**
83-
* Sets the {@link HttpPipeline} to use for the service client.
84-
*
85-
* <p><strong>Note:</strong> It is important to understand the precedence order of the HttpTrait APIs. In
86-
* particular, if a {@link HttpPipeline} is specified, this takes precedence over all other APIs in the trait, and
87-
* they will be ignored. If no {@link HttpPipeline} is specified, a HTTP pipeline will be constructed internally
88-
* based on the settings provided to this trait. Additionally, there may be other APIs in types that implement this
89-
* trait that are also ignored if an {@link HttpPipeline} is specified, so please be sure to refer to the
90-
* documentation of types that implement this trait to understand the full set of implications.</p>
91-
*
92-
* @param pipeline {@link HttpPipeline} to use for sending service requests and receiving responses.
93-
* @return An updated instance of this builder with the http pipeline set as specified.
94-
*/
95-
@SuppressWarnings("unchecked")
96-
@Override
97-
public T httpPipeline(HttpPipeline pipeline) {
98-
getHttpPipelineOptions().setHttpPipeline(pipeline);
99-
return (T) this;
100-
}
101-
10281
@SuppressWarnings("unchecked")
10382
@Override
10483
public T addHttpPipelinePolicy(HttpPipelinePolicy pipelinePolicy) {

0 commit comments

Comments
 (0)