Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Applied APIView suggestion for KV Admin. #22381

Merged
merged 4 commits into from
Jun 18, 2021
Merged

Applied APIView suggestion for KV Admin. #22381

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
246e75a
Applied APIView suggestions.
vcolin7 Jun 18, 2021
9c0ebcb
Applied APIView suggestions.
vcolin7 Jun 18, 2021
9986db8
Fixed SpotBugs issue.
vcolin7 Jun 18, 2021
657639f
Fixed another SpotBugs issue.
vcolin7 Jun 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sdk/keyvault/azure-security-keyvault-administration/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Release History

## 4.0.0 (2021-06-17)
- Initial release of `KeyVaultAccessControlClient` and `KeyVaultAccessControlAsyncClient` to managed role assignments and definitions for Managed HSM.
- Initial release of `KeyVaultAccessControlClient` and `KeyVaultAccessControlAsyncClient` to manage role assignments and definitions for Managed HSM.
- Initial release of `KeyVaultBackupClient` and `KeyVaultBackupAsyncClient` to backup and restore Managed HSM.

### Features Added
Expand Down
9 changes: 6 additions & 3 deletions ...on/src/main/java/com/azure/security/keyvault/administration/models/KeyVaultRoleScope.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ public static KeyVaultRoleScope fromString(String name) {
*
* @param url A string representing a URL containing the name of the scope to look for.
* @return The corresponding {@link KeyVaultRoleScope}.
* @throws MalformedURLException If the given {@link String URL String} is malformed.
*/
public static KeyVaultRoleScope fromUrl(String url) throws MalformedURLException {
return fromString(new URL(url).getPath(), KeyVaultRoleScope.class);
public static KeyVaultRoleScope fromUrl(String url) {
try {
return fromString(new URL(url).getPath(), KeyVaultRoleScope.class);
} catch (MalformedURLException e) {
throw new RuntimeException(e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should still document that an exception is thrown by the method even if it isn't checked. Also, should this be an IllegalArgumentException to be more targeted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think IllegalArgumentException is fine but since it a static method I'm not sure if( we should use a logger or not. I don't think we want to have static loggers. Any thoughts @srngar?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not using a logger for now, but did change to the exception type you suggested

Copy link
Member

@srnagar srnagar Jun 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IllegalArgumentException should be logged and thrown here. You can use static final loggers in static methods.

}
}

/**
Expand Down
7 changes: 5 additions & 2 deletions ...n/java/com/azure/security/keyvault/certificates/models/KeyVaultCertificateIdentifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package com.azure.security.keyvault.certificates.models;

import com.azure.core.annotation.Immutable;
import com.azure.core.util.logging.ClientLogger;
import com.azure.security.keyvault.certificates.CertificateAsyncClient;
import com.azure.security.keyvault.certificates.CertificateClient;

Expand All @@ -16,6 +17,7 @@
*/
@Immutable
public final class KeyVaultCertificateIdentifier {
private final ClientLogger logger = new ClientLogger(KeyVaultCertificateIdentifier.class);
private final String sourceId, vaultUrl, name, version;

/**
Expand All @@ -37,7 +39,7 @@ public final class KeyVaultCertificateIdentifier {
*/
public KeyVaultCertificateIdentifier(String sourceId) {
if (sourceId == null) {
throw new NullPointerException("'sourceId' cannot be null");
throw logger.logExceptionAsError(new NullPointerException("'sourceId' cannot be null"));
}

try {
Expand All @@ -55,7 +57,8 @@ public KeyVaultCertificateIdentifier(String sourceId) {
this.name = pathSegments[2];
this.version = pathSegments.length == 4 ? pathSegments[3] : null;
} catch (MalformedURLException e) {
throw new IllegalArgumentException("'sourceId' is not a valid Key Vault identifier.", e);
throw logger.logExceptionAsError(
new IllegalArgumentException("'sourceId' is not a valid Key Vault identifier.", e));
}
}

Expand Down
7 changes: 5 additions & 2 deletions ...ult-keys/src/main/java/com/azure/security/keyvault/keys/models/KeyVaultKeyIdentifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package com.azure.security.keyvault.keys.models;

import com.azure.core.annotation.Immutable;
import com.azure.core.util.logging.ClientLogger;
import com.azure.security.keyvault.keys.KeyAsyncClient;
import com.azure.security.keyvault.keys.KeyClient;

Expand All @@ -16,6 +17,7 @@
*/
@Immutable
public final class KeyVaultKeyIdentifier {
private final ClientLogger clientLogger = new ClientLogger(KeyVaultKeyIdentifier.class);
private final String sourceId, vaultUrl, name, version;

/**
Expand All @@ -37,7 +39,7 @@ public final class KeyVaultKeyIdentifier {
*/
public KeyVaultKeyIdentifier(String sourceId) {
if (sourceId == null) {
throw new NullPointerException("'sourceId' cannot be null.");
throw clientLogger.logExceptionAsError(new NullPointerException("'sourceId' cannot be null."));
}

try {
Expand All @@ -55,7 +57,8 @@ public KeyVaultKeyIdentifier(String sourceId) {
this.name = pathSegments[2];
this.version = pathSegments.length == 4 ? pathSegments[3] : null;
} catch (MalformedURLException e) {
throw new IllegalArgumentException("'sourceId' is not a valid Key Vault identifier.", e);
throw clientLogger.logExceptionAsError(
new IllegalArgumentException("'sourceId' is not a valid Key Vault identifier.", e));
}
}

Expand Down
7 changes: 5 additions & 2 deletions ...ts/src/main/java/com/azure/security/keyvault/secrets/models/KeyVaultSecretIdentifier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package com.azure.security.keyvault.secrets.models;

import com.azure.core.annotation.Immutable;
import com.azure.core.util.logging.ClientLogger;
import com.azure.security.keyvault.secrets.SecretAsyncClient;
import com.azure.security.keyvault.secrets.SecretClient;

Expand All @@ -16,6 +17,7 @@
*/
@Immutable
public final class KeyVaultSecretIdentifier {
private final ClientLogger clientLogger = new ClientLogger(KeyVaultSecretIdentifier.class);
private final String sourceId, vaultUrl, name, version;

/**
Expand All @@ -37,7 +39,7 @@ public final class KeyVaultSecretIdentifier {
*/
public KeyVaultSecretIdentifier(String sourceId) {
if (sourceId == null) {
throw new NullPointerException("'sourceId' cannot be null.");
throw clientLogger.logThrowableAsError(new NullPointerException("'sourceId' cannot be null."));
}

try {
Expand All @@ -55,7 +57,8 @@ public KeyVaultSecretIdentifier(String sourceId) {
this.name = pathSegments[2];
this.version = pathSegments.length == 4 ? pathSegments[3] : null;
} catch (MalformedURLException e) {
throw new IllegalArgumentException("'sourceId' is not a valid Key Vault identifier.", e);
throw clientLogger.logThrowableAsError(
new IllegalArgumentException("'sourceId' is not a valid Key Vault identifier.", e));
}
}

Expand Down