Applied arch board feedback for Key Vault Administration (#17284)

* Removed exposure of implementation package and any usage of KeyVaultErrorException from public APIs. * Renamed KeyVaultRoleAssignmentScope to KeyVaultRoleScope. Changed the name type from UUID to String in role assignment APIs. * Renamed APIs for re-hydrating LROs. * Added ServiceVersion support in the clients and their builders. Internally this will not be used until some changes in the code generation tool are applied. * Annotated read-only classes with @immutable. Added the "allowed" prefix to some KeyVaultPermission properties. Change the type of `startTime` and `endTime` in KeyVaultLongRunningOperation from Long to OffsetDateTime. * Changed the KeyVaultRoleScope enum from using URI to URL and added an overload that takes a the string representation of a URL. * Added overloads that allow passing a custom polling interval to LROs. * Removed the use of KeyVaultRoleAssignmentProperties in clients' public APIs in favor of using the `roleDefinitionId` and `servicePrincipalId` values directly. * Fixed Javadoc and test issues. * Fixed checkstyle issues. * Applied arch board meeting and PR feedback: * Renamed parameters containing the 'Uri' suffix to 'Url'. * Changed the type of `startTime` and `endTime` in the constructor of KeyVaultLongRunningOperation and its subtypes from `Long` to `OffsetDateTime`. * Removed unnecessary versions from KeyVaultAdministrationServiceVersion. Additional changes: * Renamed `scope` in KeyVaultRoleAssignment to `roleScope` to align with the access client APIs. * Polished Javadoc * Removed APIs to refresh LROs based on PR feedback. * Removed unused import in KeyVaultBackupAsyncClientTest.
Azure · Nov 11, 2020 · 0dc762a · 0dc762a
1 parent f3676ac
commit 0dc762a
Show file tree

Hide file tree

Showing 31 changed files with 590 additions and 1,208 deletions.
diff --git a/...ain/java/com/azure/security/keyvault/administration/KeyVaultAccessControlAsyncClient.java b/...ain/java/com/azure/security/keyvault/administration/KeyVaultAccessControlAsyncClient.java
diff --git a/...src/main/java/com/azure/security/keyvault/administration/KeyVaultAccessControlClient.java b/...src/main/java/com/azure/security/keyvault/administration/KeyVaultAccessControlClient.java
diff --git a/...n/java/com/azure/security/keyvault/administration/KeyVaultAccessControlClientBuilder.java b/...n/java/com/azure/security/keyvault/administration/KeyVaultAccessControlClientBuilder.java
@@ -74,6 +74,7 @@ public final class KeyVaultAccessControlClientBuilder {
     private RetryPolicy retryPolicy;
     private Configuration configuration;
     private ClientOptions clientOptions;
+    private KeyVaultAdministrationServiceVersion serviceVersion;
 
     /**
      * Creates a {@link KeyVaultAccessControlClientBuilder} instance that is able to configure and construct
@@ -125,8 +126,10 @@ public KeyVaultAccessControlAsyncClient buildAsyncClient() {
                     KeyVaultErrorCodeStrings.getErrorString(KeyVaultErrorCodeStrings.VAULT_END_POINT_REQUIRED)));
         }
 
+        serviceVersion = serviceVersion != null ? serviceVersion : KeyVaultAdministrationServiceVersion.getLatest();
+
         if (pipeline != null) {
-            return new KeyVaultAccessControlAsyncClient(vaultUrl, pipeline);
+            return new KeyVaultAccessControlAsyncClient(vaultUrl, pipeline, serviceVersion);
         }
 
         // Closest to API goes first, closest to wire goes last.
@@ -158,7 +161,7 @@ public KeyVaultAccessControlAsyncClient buildAsyncClient() {
             .httpClient(httpClient)
             .build();
 
-        return new KeyVaultAccessControlAsyncClient(vaultUrl, buildPipeline);
+        return new KeyVaultAccessControlAsyncClient(vaultUrl, buildPipeline, serviceVersion);
     }
 
     /**
@@ -295,13 +298,28 @@ public KeyVaultAccessControlClientBuilder retryPolicy(RetryPolicy retryPolicy) {
      * <p>More About <a href="https://azure.github.io/azure-sdk/general_azurecore.html#telemetry-policy">Azure Core: Telemetry policy</a>
      *
      * @param clientOptions the {@link ClientOptions} to be set on the client.
-     * @return The updated KeyVaultAccessControlClientBuilder object.
+     * @return The updated {@link KeyVaultAccessControlClientBuilder} object.
      */
     public KeyVaultAccessControlClientBuilder clientOptions(ClientOptions clientOptions) {
         this.clientOptions = clientOptions;
         return this;
     }
 
+    /**
+     * Sets the {@link KeyVaultAdministrationServiceVersion} that is used when making API requests.
+     * <p>
+     * If a service version is not provided, the service version that will be used will be the latest known service
+     * version based on the version of the client library being used. If no service version is specified, updating to a
+     * newer version the client library will have the result of potentially moving to a newer service version.
+     *
+     * @param serviceVersion {@link KeyVaultAdministrationServiceVersion} of the service API used when making requests.
+     * @return The updated {@link KeyVaultAccessControlClientBuilder} object.
+     */
+    public KeyVaultAccessControlClientBuilder serviceVersion(KeyVaultAdministrationServiceVersion serviceVersion) {
+        this.serviceVersion = serviceVersion;
+        return this;
+    }
+
     private URL getBuildEndpoint(Configuration configuration) {
         if (vaultUrl != null) {
             return vaultUrl;

diff --git a/...java/com/azure/security/keyvault/administration/KeyVaultAdministrationServiceVersion.java b/...java/com/azure/security/keyvault/administration/KeyVaultAdministrationServiceVersion.java
@@ -0,0 +1,33 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.azure.security.keyvault.administration;
+
+import com.azure.core.util.ServiceVersion;
+
+/**
+ * The versions of Azure Key Vault Administration service supported by this client library.
+ */
+public enum KeyVaultAdministrationServiceVersion implements ServiceVersion {
+    V7_2_PREVIEW("7.2-preview");
+
+    private final String version;
+
+    KeyVaultAdministrationServiceVersion(String version) {
+        this.version = version;
+    }
+
+    @Override
+    public String getVersion() {
+        return this.version;
+    }
+
+    /**
+     * Gets the latest service version supported by this client library.
+     *
+     * @return The latest {@link KeyVaultAdministrationServiceVersion}.
+     */
+    public static KeyVaultAdministrationServiceVersion getLatest() {
+        return V7_2_PREVIEW;
+    }
+}
diff --git a/...n/src/main/java/com/azure/security/keyvault/administration/KeyVaultBackupAsyncClient.java b/...n/src/main/java/com/azure/security/keyvault/administration/KeyVaultBackupAsyncClient.java
diff --git a/...ration/src/main/java/com/azure/security/keyvault/administration/KeyVaultBackupClient.java b/...ration/src/main/java/com/azure/security/keyvault/administration/KeyVaultBackupClient.java
@@ -6,7 +6,6 @@
 import com.azure.core.annotation.ReturnType;
 import com.azure.core.annotation.ServiceClient;
 import com.azure.core.annotation.ServiceMethod;
-import com.azure.core.util.polling.PollerFlux;
 import com.azure.core.util.polling.SyncPoller;
 import com.azure.security.keyvault.administration.models.KeyVaultBackupOperation;
 import com.azure.security.keyvault.administration.models.KeyVaultRestoreOperation;
@@ -66,26 +65,14 @@ public SyncPoller<KeyVaultBackupOperation, String> beginBackup(String blobStorag
         return asyncClient.beginBackup(blobStorageUrl, sasToken, pollingInterval).getSyncPoller();
     }
 
-    /**
-     * Gets a pending {@link KeyVaultBackupOperation backup operation} from the Key Vault.
-     *
-     * @param jobId The operation identifier.
-     * @throws NullPointerException if the {@code jobId} is null.
-     * @return A {@link SyncPoller} to poll on the backup operation status.
-     */
-    @ServiceMethod(returns = ReturnType.SINGLE)
-    public SyncPoller<KeyVaultBackupOperation, String> getBackupOperation(String jobId) {
-        return asyncClient.getBackupOperation(jobId).getSyncPoller();
-    }
-
     /**
      * Initiates a full restore of the Key Vault.
      *
      * @param backupFolderUrl The URL for the Blob Storage resource where the backup is located, including the path to
      * the blob container where the backup resides. This would be the exact value that is returned as the result of a
      * backup operation. An example of such a URL may look like the following: https://contoso.blob.core.windows.net/backup/mhsm-contoso-2020090117323313.
      * @param sasToken A Shared Access Signature (SAS) token to authorize access to the blob.
-     * @return A {@link SyncPoller} polling on the {@link KeyVaultRestoreOperation backup operation} status.
+     * @return A {@link SyncPoller} to poll on the {@link KeyVaultRestoreOperation restore operation} status.
      * @throws NullPointerException if the {@code backupFolderUrl} or {@code sasToken} are {@code null}.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
@@ -101,26 +88,14 @@ public SyncPoller<KeyVaultRestoreOperation, Void> beginRestore(String backupFold
      * backup operation. An example of such a URL may look like the following: https://contoso.blob.core.windows.net/backup/mhsm-contoso-2020090117323313.
      * @param sasToken A Shared Access Signature (SAS) token to authorize access to the blob.
      * @param pollingInterval The interval at which the operation status will be polled for.
-     * @return A {@link SyncPoller} polling on the {@link KeyVaultRestoreOperation backup operation} status.
+     * @return A {@link SyncPoller} to poll on the {@link KeyVaultRestoreOperation restore operation} status.
      * @throws NullPointerException if the {@code backupFolderUrl} or {@code sasToken} are {@code null}.
      */
     @ServiceMethod(returns = ReturnType.SINGLE)
     public SyncPoller<KeyVaultRestoreOperation, Void> beginRestore(String backupFolderUrl, String sasToken, Duration pollingInterval) {
         return asyncClient.beginRestore(backupFolderUrl, sasToken, pollingInterval).getSyncPoller();
     }
 
-    /**
-     * Gets a pending {@link KeyVaultRestoreOperation full or selective restore operation} from the Key Vault.
-     *
-     * @param jobId The operation identifier.
-     * @throws NullPointerException if the {@code jobId} is null.
-     * @return A {@link SyncPoller} to poll on the restore operation status.
-     */
-    @ServiceMethod(returns = ReturnType.SINGLE)
-    public SyncPoller<KeyVaultRestoreOperation, Void> getRestoreOperation(String jobId) {
-        return asyncClient.getRestoreOperation(jobId).getSyncPoller();
-    }
-
     /**
      * Restores all versions of a given key using the supplied SAS token pointing to a previously stored Azure Blob
      * storage backup folder.
@@ -130,7 +105,7 @@ public SyncPoller<KeyVaultRestoreOperation, Void> getRestoreOperation(String job
      * the blob container where the backup resides. This would be the exact value that is returned as the result of a
      * backup operation. An example of such a URL may look like the following: https://contoso.blob.core.windows.net/backup/mhsm-contoso-2020090117323313.
      * @param sasToken A Shared Access Signature (SAS) token to authorize access to the blob.
-     * @return A {@link PollerFlux} polling on the {@link KeyVaultRestoreOperation backup operation} status.
+     * @return A {@link SyncPoller} to poll on the {@link KeyVaultRestoreOperation restore operation} status.
      * @throws NullPointerException if the {@code keyName}, {@code backupFolderUrl} or {@code sasToken} are {@code
      * null}.
      */
@@ -149,7 +124,7 @@ public SyncPoller<KeyVaultRestoreOperation, Void> beginSelectiveRestore(String k
      * backup operation. An example of such a URL may look like the following: https://contoso.blob.core.windows.net/backup/mhsm-contoso-2020090117323313.
      * @param sasToken A Shared Access Signature (SAS) token to authorize access to the blob.
      * @param pollingInterval The interval at which the operation status will be polled for.
-     * @return A {@link PollerFlux} polling on the {@link KeyVaultRestoreOperation backup operation} status.
+     * @return A {@link SyncPoller} to poll on the {@link KeyVaultRestoreOperation restore operation} status.
      * @throws NullPointerException if the {@code keyName}, {@code backupFolderUrl} or {@code sasToken} are {@code
      * null}.
      */

diff --git a/...src/main/java/com/azure/security/keyvault/administration/KeyVaultBackupClientBuilder.java b/...src/main/java/com/azure/security/keyvault/administration/KeyVaultBackupClientBuilder.java
@@ -73,6 +73,7 @@ public final class KeyVaultBackupClientBuilder {
     private RetryPolicy retryPolicy;
     private Configuration configuration;
     private ClientOptions clientOptions;
+    private KeyVaultAdministrationServiceVersion serviceVersion;
 
     /**
      * Creates a {@link KeyVaultBackupClientBuilder} instance that is able to configure and construct instances of
@@ -124,8 +125,10 @@ public KeyVaultBackupAsyncClient buildAsyncClient() {
                     KeyVaultErrorCodeStrings.getErrorString(KeyVaultErrorCodeStrings.VAULT_END_POINT_REQUIRED)));
         }
 
+        serviceVersion = serviceVersion != null ? serviceVersion : KeyVaultAdministrationServiceVersion.getLatest();
+
         if (pipeline != null) {
-            return new KeyVaultBackupAsyncClient(vaultUrl, pipeline);
+            return new KeyVaultBackupAsyncClient(vaultUrl, pipeline, serviceVersion);
         }
 
         // Closest to API goes first, closest to wire goes last.
@@ -157,7 +160,7 @@ public KeyVaultBackupAsyncClient buildAsyncClient() {
             .httpClient(httpClient)
             .build();
 
-        return new KeyVaultBackupAsyncClient(vaultUrl, buildPipeline);
+        return new KeyVaultBackupAsyncClient(vaultUrl, buildPipeline, serviceVersion);
     }
 
     /**
@@ -295,13 +298,28 @@ public KeyVaultBackupClientBuilder retryPolicy(RetryPolicy retryPolicy) {
      * <p>More About <a href="https://azure.github.io/azure-sdk/general_azurecore.html#telemetry-policy">Azure Core: Telemetry policy</a>
      *
      * @param clientOptions the {@link ClientOptions} to be set on the client.
-     * @return The updated KeyVaultBackupClientBuilder object.
+     * @return The updated {@link KeyVaultBackupClientBuilder} object.
      */
     public KeyVaultBackupClientBuilder clientOptions(ClientOptions clientOptions) {
         this.clientOptions = clientOptions;
         return this;
     }
 
+    /**
+     * Sets the {@link KeyVaultAdministrationServiceVersion} that is used when making API requests.
+     * <p>
+     * If a service version is not provided, the service version that will be used will be the latest known service
+     * version based on the version of the client library being used. If no service version is specified, updating to a
+     * newer version the client library will have the result of potentially moving to a newer service version.
+     *
+     * @param serviceVersion {@link KeyVaultAdministrationServiceVersion} of the service API used when making requests.
+     * @return The updated {@link KeyVaultBackupClientBuilder} object.
+     */
+    public KeyVaultBackupClientBuilder serviceVersion(KeyVaultAdministrationServiceVersion serviceVersion) {
+        this.serviceVersion = serviceVersion;
+        return this;
+    }
+
     private URL getBuildEndpoint(Configuration configuration) {
         if (vaultUrl != null) {
             return vaultUrl;

diff --git a/.../main/java/com/azure/security/keyvault/administration/models/KeyVaultBackupOperation.java b/.../main/java/com/azure/security/keyvault/administration/models/KeyVaultBackupOperation.java
@@ -3,34 +3,39 @@
 
 package com.azure.security.keyvault.administration.models;
 
+import com.azure.core.annotation.Immutable;
+
+import java.time.OffsetDateTime;
+
 /**
  * A class that contains the details of a backup operation.
  */
+@Immutable
 public final class KeyVaultBackupOperation extends KeyVaultLongRunningOperation {
-    private final String azureStorageBlobContainerUri;
+    private final String azureStorageBlobContainerUrl;
 
     /**
      * Creates an object containing the details of a {@link KeyVaultBackupOperation}.
      *
      * @param status Status of the {@link KeyVaultBackupOperation}.
      * @param statusDetails The status details of the {@link KeyVaultBackupOperation}.
      * @param error Error encountered, if any, during the {@link KeyVaultBackupOperation}.
-     * @param startTime The start time of the {@link KeyVaultBackupOperation} in UTC.
-     * @param endTime The end time of the {@link KeyVaultBackupOperation} in UTC.
+     * @param startTime The start time of the {@link KeyVaultBackupOperation}.
+     * @param endTime The end time of the {@link KeyVaultBackupOperation}.
      * @param jobId Identifier for the full {@link KeyVaultBackupOperation}.
-     * @param azureStorageBlobContainerUri The Azure blob storage container URI which contains the backup.
+     * @param azureStorageBlobContainerUrl The Azure blob storage container URI which contains the backup.
      */
-    public KeyVaultBackupOperation(String status, String statusDetails, KeyVaultError error, String jobId, Long startTime, Long endTime, String azureStorageBlobContainerUri) {
+    public KeyVaultBackupOperation(String status, String statusDetails, KeyVaultError error, String jobId, OffsetDateTime startTime, OffsetDateTime endTime, String azureStorageBlobContainerUrl) {
         super(status, statusDetails, error, jobId, startTime, endTime);
-        this.azureStorageBlobContainerUri = azureStorageBlobContainerUri;
+        this.azureStorageBlobContainerUrl = azureStorageBlobContainerUrl;
     }
 
     /**
      * Get the Azure Blob Storage container URI where the backup resides.
      *
      * @return The backup URI in {@link String} form.
      */
-    public String getAzureStorageBlobContainerUri() {
-        return azureStorageBlobContainerUri;
+    public String getAzureStorageBlobContainerUrl() {
+        return azureStorageBlobContainerUrl;
     }
 }
diff --git a/...ration/src/main/java/com/azure/security/keyvault/administration/models/KeyVaultError.java b/...ration/src/main/java/com/azure/security/keyvault/administration/models/KeyVaultError.java
@@ -3,9 +3,12 @@
 
 package com.azure.security.keyvault.administration.models;
 
+import com.azure.core.annotation.Immutable;
+
 /**
  * A class that represents an error occurred in a Key Vault operation.
  */
+@Immutable
 public final class KeyVaultError {
     private final String code;
     private final String message;

diff --git a/.../java/com/azure/security/keyvault/administration/models/KeyVaultLongRunningOperation.java b/.../java/com/azure/security/keyvault/administration/models/KeyVaultLongRunningOperation.java
@@ -3,28 +3,33 @@
 
 package com.azure.security.keyvault.administration.models;
 
+import com.azure.core.annotation.Immutable;
+
+import java.time.OffsetDateTime;
+
 /**
  * A class that contains the details of a long running operation.
  */
+@Immutable
 public class KeyVaultLongRunningOperation {
     private final String status;
     private final String statusDetails;
     private final KeyVaultError error;
     private final String jobId;
-    private final Long startTime;
-    private final Long endTime;
+    private final OffsetDateTime startTime;
+    private final OffsetDateTime endTime;
 
     /**
      * Creates an object containing the details of a {@link KeyVaultLongRunningOperation}.
      *
      * @param status Status of the {@link KeyVaultLongRunningOperation}.
      * @param statusDetails The status details of the {@link KeyVaultLongRunningOperation}.
      * @param error Error encountered, if any, during the {@link KeyVaultLongRunningOperation}.
-     * @param startTime The start time of the {@link KeyVaultLongRunningOperation} in UTC.
-     * @param endTime The end time of the {@link KeyVaultLongRunningOperation} in UTC.
+     * @param startTime The start time of the {@link KeyVaultLongRunningOperation}.
+     * @param endTime The end time of the {@link KeyVaultLongRunningOperation}.
      * @param jobId Identifier for the full {@link KeyVaultLongRunningOperation}.
      */
-    public KeyVaultLongRunningOperation(String status, String statusDetails, KeyVaultError error, String jobId, Long startTime, Long endTime) {
+    public KeyVaultLongRunningOperation(String status, String statusDetails, KeyVaultError error, String jobId, OffsetDateTime startTime, OffsetDateTime endTime) {
         this.status = status;
         this.statusDetails = statusDetails;
         this.error = error;
@@ -61,20 +66,20 @@ public KeyVaultError getError() {
     }
 
     /**
-     * Get the start time of the {@link KeyVaultLongRunningOperation} in UTC.
+     * Get the start time of the {@link KeyVaultLongRunningOperation}.
      *
-     * @return The start time in UTC.
+     * @return The start time.
      */
-    public Long getStartTime() {
+    public OffsetDateTime getStartTime() {
         return startTime;
     }
 
     /**
-     * Get the end time of the {@link KeyVaultLongRunningOperation} in UTC.
+     * Get the end time of the {@link KeyVaultLongRunningOperation}.
      *
-     * @return The end time in UTC.
+     * @return The end time.
      */
-    public Long getEndTime() {
+    public OffsetDateTime getEndTime() {
         return endTime;
     }