Skip to content

Commit

Permalink
ACR cleanups and fixes (#33160)
Browse files Browse the repository at this point in the history 
* Fixing ACR test to use valid hex digest and other cleanups and tests
  • Loading branch information
@lmolkova
lmolkova authored Jan 26, 2023
1 parent 7b7b901 commit b62cf36
Show file tree
Hide file tree
Showing 37 changed files with 2,081 additions and 1,637 deletions.
3 changes: 0 additions & 3 deletions sdk/containerregistry/azure-containers-containerregistry/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@
</scm>

<properties>
<!-- Temporarily skipping code coverage until tests are enabled/added -->
<jacoco.skip>true</jacoco.skip>
<!-- Configures the Java 9+ run to perform the required module exports, opens, and reads that are necessary for testing but shouldn't be part of the module-info. -->
<javaModulesSurefireArgLine>
--add-exports com.azure.core/com.azure.core.implementation.http=ALL-UNNAMED
Expand All @@ -40,7 +38,6 @@
--add-opens com.azure.containers.containerregistry/com.azure.containers.containerregistry.implementation.models=ALL-UNNAMED
--add-opens com.azure.core/com.azure.core.implementation.util=ALL-UNNAMED
--add-reads com.azure.core=ALL-UNNAMED
--add-reads com.azure.containers.containerregistry=com.azure.http.netty
</javaModulesSurefireArgLine>
</properties>

Expand Down
25 changes: 12 additions & 13 deletions ...ry/src/main/java/com/azure/containers/containerregistry/ContainerRegistryAsyncClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,13 @@
*/
@ServiceClient(builder = ContainerRegistryClientBuilder.class, isAsync = true)
public final class ContainerRegistryAsyncClient {
private static final ClientLogger LOGGER = new ClientLogger(ContainerRegistryAsyncClient.class);
private final AzureContainerRegistryImpl registryImplClient;
private final ContainerRegistriesImpl registriesImplClient;
private final HttpPipeline httpPipeline;
private final String endpoint;
private final String apiVersion;

private final ClientLogger logger = new ClientLogger(ContainerRegistryAsyncClient.class);

ContainerRegistryAsyncClient(HttpPipeline httpPipeline, String endpoint, String version) {
this.httpPipeline = httpPipeline;
this.endpoint = endpoint;
Expand Down Expand Up @@ -119,7 +118,7 @@ public PagedFlux<String> listRepositoryNames() {
private Mono<PagedResponse<String>> listRepositoryNamesSinglePageAsync(Integer pageSize, Context context) {
try {
if (pageSize != null && pageSize < 0) {
return monoError(logger, new IllegalArgumentException("'pageSize' cannot be negative."));
return monoError(LOGGER, new IllegalArgumentException("'pageSize' cannot be negative."));
}

Mono<PagedResponse<String>> pagedResponseMono = this.registriesImplClient.getRepositoriesSinglePageAsync(null, pageSize, context.addData(AZ_TRACING_NAMESPACE_KEY, CONTAINER_REGISTRY_TRACING_NAMESPACE_VALUE))
Expand All @@ -128,7 +127,7 @@ private Mono<PagedResponse<String>> listRepositoryNamesSinglePageAsync(Integer p
return pagedResponseMono;

} catch (RuntimeException e) {
return monoError(logger, e);
return monoError(LOGGER, e);
}
}

Expand All @@ -137,7 +136,7 @@ private Mono<PagedResponse<String>> listRepositoryNamesNextSinglePageAsync(Strin
Mono<PagedResponse<String>> pagedResponseMono = this.registriesImplClient.getRepositoriesNextSinglePageAsync(nextLink, context.addData(AZ_TRACING_NAMESPACE_KEY, CONTAINER_REGISTRY_TRACING_NAMESPACE_VALUE));
return pagedResponseMono.map(res -> UtilsImpl.getPagedResponseWithContinuationToken(res));
} catch (RuntimeException e) {
return monoError(logger, e);
return monoError(LOGGER, e);
}
}

Expand Down Expand Up @@ -168,20 +167,20 @@ public Mono<Response<Void>> deleteRepositoryWithResponse(String repositoryName)
}

private Mono<Response<Void>> deleteRepositoryWithResponse(String repositoryName, Context context) {
try {
if (repositoryName == null) {
return monoError(logger, new NullPointerException("'repositoryName' cannot be null."));
}
if (repositoryName == null) {
return monoError(LOGGER, new NullPointerException("'repositoryName' cannot be null."));
}

if (repositoryName.isEmpty()) {
return monoError(logger, new IllegalArgumentException("'repositoryName' cannot be empty."));
}
if (repositoryName.isEmpty()) {
return monoError(LOGGER, new IllegalArgumentException("'repositoryName' cannot be empty."));
}

try {
return this.registriesImplClient.deleteRepositoryWithResponseAsync(repositoryName, context.addData(AZ_TRACING_NAMESPACE_KEY, CONTAINER_REGISTRY_TRACING_NAMESPACE_VALUE))
.flatMap(response -> Mono.just(UtilsImpl.deleteResponseToSuccess(response)))
.onErrorMap(UtilsImpl::mapException);
} catch (RuntimeException e) {
return monoError(logger, e);
return monoError(LOGGER, e);
}
}

Expand Down
18 changes: 9 additions & 9 deletions ...egistry/src/main/java/com/azure/containers/containerregistry/ContainerRegistryClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import com.azure.core.util.Context;
import com.azure.core.util.logging.ClientLogger;

import java.util.Objects;

import static com.azure.containers.containerregistry.implementation.UtilsImpl.enableSync;
import static com.azure.containers.containerregistry.implementation.UtilsImpl.getTracingContext;
import static com.azure.containers.containerregistry.implementation.UtilsImpl.mapAcrErrorsException;
Expand Down Expand Up @@ -62,7 +64,7 @@
*/
@ServiceClient(builder = ContainerRegistryClientBuilder.class)
public final class ContainerRegistryClient {
private final ClientLogger logger = new ClientLogger(ContainerRegistryClient.class);
private static final ClientLogger LOGGER = new ClientLogger(ContainerRegistryClient.class);
private final ContainerRegistriesImpl registriesImplClient;
private final HttpPipeline httpPipeline;
private final String endpoint;
Expand Down Expand Up @@ -134,14 +136,14 @@ public PagedIterable<String> listRepositoryNames(Context context) {

private PagedResponse<String> listRepositoryNamesSinglePageSync(Integer pageSize, Context context) {
if (pageSize != null && pageSize < 0) {
throw logger.logExceptionAsError(new IllegalArgumentException("'pageSize' cannot be negative."));
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'pageSize' cannot be negative."));
}

try {
return this.registriesImplClient.getRepositoriesSinglePage(null,
pageSize, enableSync(getTracingContext(context)));
} catch (AcrErrorsException exception) {
throw logger.logExceptionAsError(mapAcrErrorsException(exception));
throw LOGGER.logExceptionAsError(mapAcrErrorsException(exception));
}
}

Expand All @@ -150,7 +152,7 @@ private PagedResponse<String> listRepositoryNamesNextSinglePageSync(String nextL
return this.registriesImplClient.getRepositoriesNextSinglePage(nextLink,
enableSync(getTracingContext(context)));
} catch (AcrErrorsException exception) {
throw logger.logExceptionAsError(mapAcrErrorsException(exception));
throw LOGGER.logExceptionAsError(mapAcrErrorsException(exception));
}
}

Expand Down Expand Up @@ -194,20 +196,18 @@ public void deleteRepository(String repositoryName) {
*/
@ServiceMethod(returns = ReturnType.SINGLE)
public Response<Void> deleteRepositoryWithResponse(String repositoryName, Context context) {
if (repositoryName == null) {
throw logger.logExceptionAsError(new NullPointerException("'repositoryName' cannot be null."));
}
Objects.requireNonNull(repositoryName, "'repositoryName' cannot be null");

if (repositoryName.isEmpty()) {
throw logger.logExceptionAsError(new IllegalArgumentException("'repositoryName' cannot be empty."));
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'repositoryName' cannot be empty."));
}

try {
return UtilsImpl.deleteResponseToSuccess(
this.registriesImplClient.deleteRepositoryWithResponse(repositoryName,
enableSync(getTracingContext(context))));
} catch (AcrErrorsException exception) {
throw logger.logExceptionAsError(mapAcrErrorsException(exception));
throw LOGGER.logExceptionAsError(mapAcrErrorsException(exception));
}
}

Expand Down
12 changes: 6 additions & 6 deletions .../src/main/java/com/azure/containers/containerregistry/ContainerRegistryClientBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ public final class ContainerRegistryClientBuilder implements
EndpointTrait<ContainerRegistryClientBuilder>,
HttpTrait<ContainerRegistryClientBuilder>,
TokenCredentialTrait<ContainerRegistryClientBuilder> {
private final ClientLogger logger = new ClientLogger(ContainerRegistryClientBuilder.class);
private static final ClientLogger LOGGER = new ClientLogger(ContainerRegistryClientBuilder.class);

private final List<HttpPipelinePolicy> perCallPolicies = new ArrayList<>();
private final List<HttpPipelinePolicy> perRetryPolicies = new ArrayList<>();
private ClientOptions clientOptions;
Expand All @@ -145,7 +146,7 @@ public ContainerRegistryClientBuilder endpoint(String endpoint) {
try {
new URL(endpoint);
} catch (MalformedURLException ex) {
throw logger.logExceptionAsWarning(new IllegalArgumentException("'endpoint' must be a valid URL", ex));
throw LOGGER.logExceptionAsWarning(new IllegalArgumentException("'endpoint' must be a valid URL", ex));
}

this.endpoint = endpoint;
Expand Down Expand Up @@ -202,7 +203,7 @@ public ContainerRegistryClientBuilder credential(TokenCredential credential) {
@Override
public ContainerRegistryClientBuilder pipeline(HttpPipeline httpPipeline) {
if (this.httpPipeline != null && httpPipeline == null) {
logger.info("HttpPipeline is being set to 'null' when it was previously configured.");
LOGGER.info("HttpPipeline is being set to 'null' when it was previously configured.");
}
this.httpPipeline = httpPipeline;
return this;
Expand Down Expand Up @@ -238,7 +239,7 @@ public ContainerRegistryClientBuilder serviceVersion(ContainerRegistryServiceVer
@Override
public ContainerRegistryClientBuilder httpClient(HttpClient httpClient) {
if (this.httpClient != null && httpClient == null) {
logger.info("HttpClient is being set to 'null' when it was previously configured.");
LOGGER.info("HttpClient is being set to 'null' when it was previously configured.");
}
this.httpClient = httpClient;
return this;
Expand Down Expand Up @@ -441,7 +442,6 @@ private HttpPipeline getHttpPipeline() {
this.perRetryPolicies,
this.httpClient,
this.endpoint,
this.version,
this.logger);
this.version);
}
}
28 changes: 12 additions & 16 deletions ...nerregistry/src/main/java/com/azure/containers/containerregistry/ContainerRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.net.MalformedURLException;
import java.net.URL;
import java.util.Objects;

import static com.azure.containers.containerregistry.implementation.UtilsImpl.enableSync;
import static com.azure.containers.containerregistry.implementation.UtilsImpl.getTracingContext;
Expand Down Expand Up @@ -57,7 +58,7 @@
*/
@ServiceClient(builder = ContainerRegistryClientBuilder.class)
public final class ContainerRepository {
private final ClientLogger logger = new ClientLogger(ContainerRepository.class);
private static final ClientLogger LOGGER = new ClientLogger(ContainerRepository.class);
private final ContainerRegistriesImpl serviceClient;
private final String repositoryName;
private final String endpoint;
Expand All @@ -76,12 +77,9 @@ public final class ContainerRepository {
* @param version {@link ContainerRegistryServiceVersion} of the service to be used when making requests.
*/
ContainerRepository(String repositoryName, HttpPipeline httpPipeline, String endpoint, String version) {
if (repositoryName == null) {
throw logger.logExceptionAsError(new NullPointerException("'repositoryName' can't be null."));
}

Objects.requireNonNull(repositoryName, "'repositoryName' cannot be null");
if (repositoryName.isEmpty()) {
throw logger.logExceptionAsError(new IllegalArgumentException("'repositoryName' can't be empty."));
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'repositoryName' can't be empty."));
}

AzureContainerRegistryImpl registryImpl = new AzureContainerRegistryImplBuilder()
Expand All @@ -101,7 +99,7 @@ public final class ContainerRepository {
this.registryLoginServer = endpointUrl.getHost();
} catch (MalformedURLException ex) {
// This will not happen.
throw logger.logExceptionAsWarning(new IllegalArgumentException("'endpoint' must be a valid URL", ex));
throw LOGGER.logExceptionAsWarning(new IllegalArgumentException("'endpoint' must be a valid URL", ex));
}
}

Expand Down Expand Up @@ -151,7 +149,7 @@ public Response<Void> deleteWithResponse(Context context) {
this.serviceClient.deleteRepositoryWithResponse(repositoryName, enableSync(getTracingContext(context)));
return UtilsImpl.deleteResponseToSuccess(response);
} catch (AcrErrorsException exception) {
throw logger.logExceptionAsError(mapAcrErrorsException(exception));
throw LOGGER.logExceptionAsError(mapAcrErrorsException(exception));
}
}

Expand Down Expand Up @@ -205,7 +203,7 @@ public Response<ContainerRepositoryProperties> getPropertiesWithResponse(Context
try {
return this.serviceClient.getPropertiesWithResponse(repositoryName, enableSync(getTracingContext(context)));
} catch (AcrErrorsException exception) {
throw logger.logExceptionAsError(mapAcrErrorsException(exception));
throw LOGGER.logExceptionAsError(mapAcrErrorsException(exception));
}
}

Expand Down Expand Up @@ -355,7 +353,7 @@ private PagedIterable<ArtifactManifestProperties> listManifestPropertiesSync(Art

private PagedResponse<ArtifactManifestProperties> listManifestPropertiesSinglePageSync(Integer pageSize, ArtifactManifestOrder order, Context context) {
if (pageSize != null && pageSize < 0) {
throw logger.logExceptionAsError(new IllegalArgumentException("'pageSize' cannot be negative."));
throw LOGGER.logExceptionAsError(new IllegalArgumentException("'pageSize' cannot be negative."));
}

final String orderString = order == ArtifactManifestOrder.NONE ? null : order.toString();
Expand All @@ -367,7 +365,7 @@ private PagedResponse<ArtifactManifestProperties> listManifestPropertiesSinglePa
return UtilsImpl.getPagedResponseWithContinuationToken(res,
baseArtifacts -> UtilsImpl.mapManifestsProperties(baseArtifacts, repositoryName, registryLoginServer));
} catch (AcrErrorsException exception) {
throw logger.logExceptionAsError(mapAcrErrorsException(exception));
throw LOGGER.logExceptionAsError(mapAcrErrorsException(exception));
}
}

Expand All @@ -378,7 +376,7 @@ private PagedResponse<ArtifactManifestProperties> listManifestPropertiesNextSing
return UtilsImpl.getPagedResponseWithContinuationToken(res,
baseArtifacts -> UtilsImpl.mapManifestsProperties(baseArtifacts, repositoryName, registryLoginServer));
} catch (AcrErrorsException exception) {
throw logger.logExceptionAsError(mapAcrErrorsException(exception));
throw LOGGER.logExceptionAsError(mapAcrErrorsException(exception));
}
}

Expand Down Expand Up @@ -440,9 +438,7 @@ public ContainerRepositoryProperties updateProperties(ContainerRepositoryPropert
}

private Response<ContainerRepositoryProperties> updatePropertiesWithResponseSync(ContainerRepositoryProperties repositoryProperties, Context context) {
if (repositoryProperties == null) {
throw logger.logExceptionAsError(new NullPointerException("'value' cannot be null."));
}
Objects.requireNonNull(repositoryProperties, "'repositoryProperties' cannot be null");

RepositoryWriteableProperties writableProperties = new RepositoryWriteableProperties()
.setDeleteEnabled(repositoryProperties.isDeleteEnabled())
Expand All @@ -455,7 +451,7 @@ private Response<ContainerRepositoryProperties> updatePropertiesWithResponseSync
return this.serviceClient.updatePropertiesWithResponse(repositoryName, writableProperties,
enableSync(getTracingContext(context)));
} catch (AcrErrorsException exception) {
throw logger.logExceptionAsError(mapAcrErrorsException(exception));
throw LOGGER.logExceptionAsError(mapAcrErrorsException(exception));
}
}

Expand Down
Loading

0 comments on commit b62cf36

Please sign in to comment.