Skip to content

Commit

Permalink
Merge branch 'master' into tokenCred
Browse files Browse the repository at this point in the history
  • Loading branch information
sima-zhu authored Jul 30, 2019
2 parents 9d849c8 + 2db7c54 commit ea937c9
Show file tree
Hide file tree
Showing 234 changed files with 1,835 additions and 1,061 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
* No external dependency exposed in public API
*/
public class ExternalDependencyExposedCheck extends AbstractCheck {
private static final String EXTERNAL_DEPENDENCY_ERROR =
"Class ''%s'', is a class from external dependency. You should not use it as a return or method argument type.";

private static final String EXTERNAL_DEPENDENCY_ERROR = "Class ''%s'', is a class from external dependency. You should not use it as a %s type.";
private static final Set<String> VALID_DEPENDENCY_SET = Collections.unmodifiableSet(new HashSet<>(Arrays.asList(
"java", "com.azure", "reactor", "io.netty.buffer.ByteBuf"
)));
Expand Down Expand Up @@ -51,6 +49,7 @@ public int[] getAcceptableTokens() {
public int[] getRequiredTokens() {
return new int[] {
TokenTypes.IMPORT,
TokenTypes.CLASS_DEF,
TokenTypes.METHOD_DEF
};
}
Expand All @@ -68,7 +67,7 @@ public void visitToken(DetailAST token) {
// CLASS_DEF always has MODIFIERS
final AccessModifier accessModifier = CheckUtil.getAccessModifierFromModifiersToken(
token.findFirstToken(TokenTypes.MODIFIERS));
isPublicClass = accessModifier.equals(AccessModifier.PUBLIC);
isPublicClass = accessModifier.equals(AccessModifier.PUBLIC) || accessModifier.equals(AccessModifier.PROTECTED);
break;
case TokenTypes.METHOD_DEF:
if (!isPublicClass) {
Expand Down Expand Up @@ -101,14 +100,14 @@ private void checkNoExternalDependencyExposed(DetailAST methodDefToken) {
final DetailAST typeToken = methodDefToken.findFirstToken(TokenTypes.TYPE);
if (typeToken != null) {
getInvalidReturnTypes(typeToken).forEach(
(token, returnTypeName) -> log(token, String.format(EXTERNAL_DEPENDENCY_ERROR, returnTypeName)));
(token, returnTypeName) -> log(token, String.format(EXTERNAL_DEPENDENCY_ERROR, returnTypeName, "return")));
}

// Checks for the parameters of the method
final DetailAST parametersToken = methodDefToken.findFirstToken(TokenTypes.PARAMETERS);
if (parametersToken != null) {
getInvalidParameterTypes(parametersToken).forEach(
(token, returnTypeName) -> log(token, String.format(EXTERNAL_DEPENDENCY_ERROR, returnTypeName)));
(token, parameterTypeName) -> log(token, String.format(EXTERNAL_DEPENDENCY_ERROR, parameterTypeName, "method argument")));
}
}

Expand All @@ -134,7 +133,7 @@ private Map<DetailAST, String> getInvalidReturnTypes(DetailAST typeToken) {
// TYPE_ARGUMENTS, add all invalid external types to the map
final DetailAST typeArgumentsToken = typeToken.findFirstToken(TokenTypes.TYPE_ARGUMENTS);
if (typeArgumentsToken != null) {
invalidReturnTypeMap.putAll(getInvalidTypeFromTypeArguments(typeArgumentsToken));
getInvalidParameterType(typeArgumentsToken, invalidReturnTypeMap);
}

return invalidReturnTypeMap;
Expand All @@ -150,61 +149,42 @@ private Map<DetailAST, String> getInvalidParameterTypes(DetailAST parametersType
final Map<DetailAST, String> invalidParameterTypesMap = new HashMap<>();
for (DetailAST ast = parametersTypeToken.getFirstChild(); ast != null; ast = ast.getNextSibling()) {
if (ast.getType() == TokenTypes.PARAMETER_DEF) {
invalidParameterTypesMap.putAll(getInvalidTypeFromTypeArguments(ast.findFirstToken(TokenTypes.TYPE)));
getInvalidParameterType(ast.findFirstToken(TokenTypes.TYPE), invalidParameterTypesMap);
}
}
return invalidParameterTypesMap;
}

/**
* A helper function that checks TYPE AST node. Since both return type and input parameter argument type has
* TYPE AST node under. This function applied to both.
* Get all invalid AST nodes from a given token. DFS tree traversal used to find all invalid nodes.
*
* @param typeArgumentsToken TYPE_ARGUMENTS AST node
* @return a map that maps all the invalid TYPE_ARGUMENT node and the type name
* @param token TYPE_ARGUMENT, TYPE_ARGUMENTS or TYPE AST node
* @return a map that maps all the invalid node and the type name
*/
private Map<DetailAST, String> getInvalidTypeFromTypeArguments(DetailAST typeArgumentsToken) {
final Map<DetailAST, String> invalidTypesMap = new HashMap<>();
if (typeArgumentsToken == null) {
private Map<DetailAST, String> getInvalidParameterType(DetailAST token, Map<DetailAST, String> invalidTypesMap) {
if (token == null) {
return invalidTypesMap;
}
// Checks multiple type arguments
for (DetailAST ast = typeArgumentsToken.getFirstChild(); ast != null; ast = ast.getNextSibling()) {
if (ast.getType() != TokenTypes.TYPE_ARGUMENT) {
continue;
}

final String invalidTypeName = getInvalidTypeNameFromTypeArgument(ast);
if (invalidTypeName != null) {
invalidTypesMap.put(ast, invalidTypeName);
for (DetailAST ast = token.getFirstChild(); ast != null; ast = ast.getNextSibling()) {
final int tokenType = ast.getType();
if (tokenType == TokenTypes.IDENT) {
final String identName = ast.getText();
if (!isValidClassDependency(identName)) {
invalidTypesMap.put(ast, identName);
}
} else if (tokenType == TokenTypes.TYPE_ARGUMENT || tokenType == TokenTypes.TYPE_ARGUMENTS) {
getInvalidParameterType(ast, invalidTypesMap);
}
}
return invalidTypesMap;
}

/**
* Get invalid type name from TYPE_ARGUMENT
*
* @param typeArgumentToken TYPE_ARGUMENT AST node
* @return an invalid type name if it is an invalid library. Otherwise, returns null.
*/
private String getInvalidTypeNameFromTypeArgument(DetailAST typeArgumentToken) {
final DetailAST identToken = typeArgumentToken.findFirstToken(TokenTypes.IDENT);
// if there is no IDENT token, implies the token is default java types.
if (identToken == null) {
return null;
}

final String typeName = identToken.getText();
// if not exist in the classPathMap, that implies the type is java default types, such as int.
return isValidClassDependency(typeName) ? null : typeName;
}

/**
* A helper function that checks for whether a class is from a valid internal dependency or is a suppression class
*
* @param typeName the type name of class
* @return true if the class is a suppression class, otherwise, return false.
* @return true if the class is a suppression class, otherwise, return false.
*/
private boolean isValidClassDependency(String typeName) {
// If the qualified class name does not exist in the map,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,20 @@
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientInstantiationCheck" files=".*[/\\]com[/\\]microsoft[/\\].*"/>
<suppress checks="com.azure.tools.checkstyle.checks.ThrownClientLoggerCheck" files=".*[/\\]com[/\\]microsoft[/\\].*"/>

<!-- Custom checkstyle rules that don't apply to files under test or implementation package -->
<suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files="(.*[/\\]src[/\\]test[/\\]java[/\\].*)|(.*[/\\]implementation[/\\].*)\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientInstantiationCheck" files="(.*[/\\]src[/\\]test[/\\]java[/\\].*)|(.*[/\\]implementation[/\\].*)\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" files="(.*[/\\]src[/\\]test[/\\]java[/\\].*)|(.*[/\\]implementation[/\\].*)\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceInterfaceCheck" files="(.*[/\\]src[/\\]test[/\\]java[/\\].*)|(.*[/\\]implementation[/\\].*)\.java"/>
<!-- Custom checkstyle rules that don't apply to files under test package -->
<suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files=".*[/\\]src[/\\]test[/\\]java[/\\].*\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientInstantiationCheck" files=".*[/\\]src[/\\]test[/\\]java[/\\].*\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" files=".*[/\\]src[/\\]test[/\\]java[/\\].*\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceInterfaceCheck" files=".*[/\\]src[/\\]test[/\\]java[/\\].*\.java"/>

<!-- Custom checkstyle rules that don't apply to files under implementation package -->
<suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files=".*[/\\]implementation[/\\].*\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientInstantiationCheck" files=".*[/\\]implementation[/\\].*\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceClientBuilderCheck" files=".*[/\\]implementation[/\\].*\.java"/>
<suppress checks="com.azure.tools.checkstyle.checks.ServiceInterfaceCheck" files=".*[/\\]implementation[/\\].*\.java"/>

<!-- Custom checkstyle rules that don't apply to files under samples package -->
<suppress checks="com.azure.tools.checkstyle.checks.ExternalDependencyExposedCheck" files=".*[/\\]samples[/\\].*\.java"/>

<!-- OpenCensus should only be depended on from within the tracing-opencensus module -->
<suppress checks="IllegalImport" files=".*[/\\]com[/\\]azure[/\\]tracing[/\\]opentelemetry[/\\]*"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@
<Bug pattern="NP_NULL_PARAM_DEREF"/>
</Match>

<!-- The switch cases are encoded version of all SAS query parameters, which will further be appended to a request URL it seems like. A default case wouldn't make sense -->
<!-- The switch cases are encoded version of all SAS query parameters, which will further be appended to a request URL it seems like. A default case wouldn't make sense -->
<Match>
<Class name="com.microsoft.azure.storage.blob.SASQueryParameters"/>
<Bug pattern="SF_SWITCH_NO_DEFAULT"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
* <p>Create a pipeline without configuration</p>
*
* <pre>
* HttpPipeline.builder()
* new HttpPipelineBuilder()
* .build();
* </pre>
*
* <p>Create a pipeline using the default HTTP client and a retry policy</p>
*
* <pre>
* HttpPipeline.builder()
* new HttpPipelineBuilder()
* .httpClient(HttpClient.createDefault())
* .policies(new RetryPolicy())
* .build();
Expand Down
4 changes: 2 additions & 2 deletions storage/client/blob/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-core</artifactId>
<version>1.0.0-preview.2</version>
<version>1.0.0-preview.3</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
Expand All @@ -69,7 +69,7 @@
<dependency>
<groupId>com.azure</groupId>
<artifactId>azure-core-test</artifactId>
<version>1.0.0-preview.2</version>
<version>1.0.0-preview.3</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public class BlobAsyncClient {
public BlockBlobAsyncClient asBlockBlobAsyncClient() {
return new BlockBlobAsyncClient(new AzureBlobStorageBuilder()
.url(getBlobUrl().toString())
.pipeline(azureBlobStorage.httpPipeline()), snapshot);
.pipeline(azureBlobStorage.getHttpPipeline()), snapshot);
}

/**
Expand All @@ -110,7 +110,7 @@ public BlockBlobAsyncClient asBlockBlobAsyncClient() {
public AppendBlobAsyncClient asAppendBlobAsyncClient() {
return new AppendBlobAsyncClient(new AzureBlobStorageBuilder()
.url(getBlobUrl().toString())
.pipeline(azureBlobStorage.httpPipeline()), snapshot);
.pipeline(azureBlobStorage.getHttpPipeline()), snapshot);
}

/**
Expand All @@ -122,7 +122,7 @@ public AppendBlobAsyncClient asAppendBlobAsyncClient() {
public PageBlobAsyncClient asPageBlobAsyncClient() {
return new PageBlobAsyncClient(new AzureBlobStorageBuilder()
.url(getBlobUrl().toString())
.pipeline(azureBlobStorage.httpPipeline()), snapshot);
.pipeline(azureBlobStorage.getHttpPipeline()), snapshot);
}

/**
Expand All @@ -136,7 +136,7 @@ public ContainerAsyncClient getContainerAsyncClient() {
BlobURLParts parts = URLParser.parse(getBlobUrl());
return new ContainerAsyncClient(new AzureBlobStorageBuilder()
.url(String.format("%s://%s/%s", parts.scheme(), parts.host(), parts.containerName()))
.pipeline(azureBlobStorage.httpPipeline()));
.pipeline(azureBlobStorage.getHttpPipeline()));
}

/**
Expand All @@ -147,13 +147,13 @@ public ContainerAsyncClient getContainerAsyncClient() {
*/
public URL getBlobUrl() {
try {
UrlBuilder urlBuilder = UrlBuilder.parse(azureBlobStorage.url());
UrlBuilder urlBuilder = UrlBuilder.parse(azureBlobStorage.getUrl());
if (snapshot != null) {
urlBuilder.query("snapshot=" + snapshot);
}
return urlBuilder.toURL();
} catch (MalformedURLException e) {
throw new RuntimeException(String.format("Invalid URL on %s: %s" + getClass().getSimpleName(), azureBlobStorage.url()), e);
throw new RuntimeException(String.format("Invalid URL on %s: %s" + getClass().getSimpleName(), azureBlobStorage.getUrl()), e);
}
}

Expand Down Expand Up @@ -925,7 +925,7 @@ public String generateSAS(String identifier, BlobSASPermission permissions, Offs
cacheControl, contentDisposition, contentEncoding, contentLanguage, contentType);

SharedKeyCredential sharedKeyCredential =
Utility.getSharedKeyCredential(this.azureBlobStorage.httpPipeline());
Utility.getSharedKeyCredential(this.azureBlobStorage.getHttpPipeline());

Utility.assertNotNull("sharedKeyCredential", sharedKeyCredential);

Expand All @@ -944,7 +944,7 @@ ServiceSASSignatureValues configureServiceSASSignatureValues(ServiceSASSignature
String accountName) {

// Set canonical name
serviceSASSignatureValues.canonicalName(this.azureBlobStorage.url(), accountName);
serviceSASSignatureValues.canonicalName(this.azureBlobStorage.getUrl(), accountName);

// Set snapshotId
serviceSASSignatureValues.snapshotId(getSnapshotId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.azure.core.credentials.TokenCredential;
import com.azure.core.http.HttpClient;
import com.azure.core.http.HttpPipeline;
import com.azure.core.http.HttpPipelineBuilder;
import com.azure.core.http.policy.AddDatePolicy;
import com.azure.core.http.policy.BearerTokenAuthenticationPolicy;
import com.azure.core.http.policy.HttpLogDetailLevel;
Expand Down Expand Up @@ -120,7 +121,7 @@ private AzureBlobStorageBuilder buildImpl() {
policies.addAll(this.policies);
policies.add(new HttpLoggingPolicy(logLevel));

HttpPipeline pipeline = HttpPipeline.builder()
HttpPipeline pipeline = new HttpPipelineBuilder()
.policies(policies.toArray(new HttpPipelinePolicy[0]))
.httpClient(httpClient)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public BlockBlobAsyncClient getBlockBlobAsyncClient(String blobName) {
public BlockBlobAsyncClient getBlockBlobAsyncClient(String blobName, String snapshot) {
return new BlockBlobAsyncClient(new AzureBlobStorageBuilder()
.url(Utility.appendToURLPath(getContainerUrl(), blobName).toString())
.pipeline(azureBlobStorage.httpPipeline()), snapshot);
.pipeline(azureBlobStorage.getHttpPipeline()), snapshot);
}

/**
Expand Down Expand Up @@ -140,7 +140,7 @@ public PageBlobAsyncClient getPageBlobAsyncClient(String blobName) {
public PageBlobAsyncClient getPageBlobAsyncClient(String blobName, String snapshot) {
return new PageBlobAsyncClient(new AzureBlobStorageBuilder()
.url(Utility.appendToURLPath(getContainerUrl(), blobName).toString())
.pipeline(azureBlobStorage.httpPipeline()), snapshot);
.pipeline(azureBlobStorage.getHttpPipeline()), snapshot);
}

/**
Expand Down Expand Up @@ -173,7 +173,7 @@ public AppendBlobAsyncClient getAppendBlobAsyncClient(String blobName) {
public AppendBlobAsyncClient getAppendBlobAsyncClient(String blobName, String snapshot) {
return new AppendBlobAsyncClient(new AzureBlobStorageBuilder()
.url(Utility.appendToURLPath(getContainerUrl(), blobName).toString())
.pipeline(azureBlobStorage.httpPipeline()), snapshot);
.pipeline(azureBlobStorage.getHttpPipeline()), snapshot);
}

/**
Expand Down Expand Up @@ -202,7 +202,7 @@ public BlobAsyncClient getBlobAsyncClient(String blobName) {
public BlobAsyncClient getBlobAsyncClient(String blobName, String snapshot) {
return new BlobAsyncClient(new AzureBlobStorageBuilder()
.url(Utility.appendToURLPath(getContainerUrl(), blobName).toString())
.pipeline(azureBlobStorage.httpPipeline()), snapshot);
.pipeline(azureBlobStorage.getHttpPipeline()), snapshot);
}

/**
Expand All @@ -213,7 +213,7 @@ public BlobAsyncClient getBlobAsyncClient(String blobName, String snapshot) {
public StorageAsyncClient getStorageAsyncClient() {
return new StorageAsyncClient(new AzureBlobStorageBuilder()
.url(Utility.stripLastPathSegment(getContainerUrl()).toString())
.pipeline(azureBlobStorage.httpPipeline()));
.pipeline(azureBlobStorage.getHttpPipeline()));
}

/**
Expand All @@ -224,9 +224,9 @@ public StorageAsyncClient getStorageAsyncClient() {
*/
public URL getContainerUrl() {
try {
return new URL(azureBlobStorage.url());
return new URL(azureBlobStorage.getUrl());
} catch (MalformedURLException e) {
throw new RuntimeException(String.format("Invalid URL on %s: %s" + getClass().getSimpleName(), azureBlobStorage.url()), e);
throw new RuntimeException(String.format("Invalid URL on %s: %s" + getClass().getSimpleName(), azureBlobStorage.getUrl()), e);
}
}

Expand Down Expand Up @@ -681,7 +681,7 @@ private Flux<BlobItem> listBlobsHierarchyHelper(String delimiter, ListBlobsOptio
} else {
prefixes = Flux.empty();
}
Flux<BlobItem> result = blobs.concatWith(prefixes.map(prefix -> new BlobItem().name(prefix.name()).isPrefix(true)));
Flux<BlobItem> result = blobs.map(item -> item.isPrefix(false)).concatWith(prefixes.map(prefix -> new BlobItem().name(prefix.name()).isPrefix(true)));

if (response.value().nextMarker() != null) {
// Recursively add the continuation items to the observable.
Expand Down Expand Up @@ -1102,7 +1102,7 @@ public String generateSAS(String identifier, ContainerSASPermission permissions,
cacheControl, contentDisposition, contentEncoding, contentLanguage, contentType);

SharedKeyCredential sharedKeyCredential =
Utility.getSharedKeyCredential(this.azureBlobStorage.httpPipeline());
Utility.getSharedKeyCredential(this.azureBlobStorage.getHttpPipeline());

Utility.assertNotNull("sharedKeyCredential", sharedKeyCredential);

Expand All @@ -1119,7 +1119,7 @@ public String generateSAS(String identifier, ContainerSASPermission permissions,
*/
private ServiceSASSignatureValues configureServiceSASSignatureValues(ServiceSASSignatureValues serviceSASSignatureValues, String accountName) {
// Set canonical name
serviceSASSignatureValues.canonicalName(this.azureBlobStorage.url(), accountName);
serviceSASSignatureValues.canonicalName(this.azureBlobStorage.getUrl(), accountName);

// Set snapshotId to null
serviceSASSignatureValues.snapshotId(null);
Expand Down
Loading

0 comments on commit ea937c9

Please sign in to comment.