andrewgaul requested changes on this pull request.

Please narrow this pull request to just the skeleton and queue code.  Also 
address all error-prone and Checkstyle issues.

> +        <version>2.1.0-SNAPSHOT</version>
+    </parent>
+
+    <artifactId>azure-queue-storage</artifactId>
+    <name>jclouds Azure Storage queue provider</name>
+    <description>jclouds components to access Azure queue storage 
Service</description>
+    <packaging>bundle</packaging>
+
+    <properties>
+    
<!--<test.azureblob.endpoint>https://${jclouds.identity}.blob.core.windows.net</test.azureblob.endpoint>-->
+        <test.azureblob.api-version>2016-05-31</test.azureblob.api-version>
+    <!--<test.azureblob.build-version />-->
+    
<!--<test.azureblob.identity>${test.azure.identity}</test.azureblob.identity>-->
+    
<!--<test.azureblob.credential>${test.azure.credential}</test.azureblob.credential>-->
+
+    
<!--<jclouds.osgi.export>org.jclouds.azureblob*;version="${project.version}",org.jclouds.azure.storage*;version="${project.version}"</jclouds.osgi.export>-->

Why are these commented out?

> +                                <id>integration</id>
+                                <phase>integration-test</phase>
+                                <goals>
+                                    <goal>test</goal>
+                                </goals>
+                                <!--<configuration>-->
+                                <!--<systemPropertyVariables>-->
+                                
<!--<test.azureblob.endpoint>${test.azureblob.endpoint}</test.azureblob.endpoint>-->
+                                
<!--<test.azureblob.api-version>${test.azureblob.api-version}</test.azureblob.api-version>-->
+                                
<!--<test.azureblob.build-version>${test.azureblob.build-version}</test.azureblob.build-version>-->
+                                
<!--<test.azureblob.identity>${test.azureblob.identity}</test.azureblob.identity>-->
+                                
<!--<test.azureblob.credential>${test.azureblob.credential}</test.azureblob.credential>-->
+                                
<!--<jclouds.blobstore.httpstream.url>${jclouds.blobstore.httpstream.url}</jclouds.blobstore.httpstream.url>-->
+                                
<!--<jclouds.blobstore.httpstream.md5>${jclouds.blobstore.httpstream.md5}</jclouds.blobstore.httpstream.md5>-->
+                                <!--</systemPropertyVariables>-->
+                                <!--</configuration>-->

More commented out code.

> +
+package org.jclouds.azure.storage;
+
+import org.jclouds.azure.storage.features.MessageApi;
+import org.jclouds.azure.storage.features.QueueApi;
+import org.jclouds.rest.annotations.Delegate;
+
+import java.io.Closeable;
+
+public interface AzureStorageQueueApi extends Closeable {
+
+   @Delegate
+   QueueApi getQueueApi();
+
+   @Delegate
+   MessageApi getMessageApi();

Please remove all message functionality which you can submit in a subsequent 
pull request.

> + */
+package org.jclouds.azure.storage;
+
+import org.jclouds.azure.storage.domain.AzureStorageError;
+import org.jclouds.http.HttpCommand;
+import org.jclouds.http.HttpResponse;
+import org.jclouds.http.HttpResponseException;
+
+/**
+ * Encapsulates an Error from Azure Storage Services.
+ *
+ * @see <a 
href="http://docs.amazonwebservices.com/AmazonS3/2006-03-01/UsingRESTError.html";
 />
+ * @see AzureStorageError
+ * @see org.jclouds.aws.handlers.ParseAzureStorageErrorFromXmlContent
+ */
+public class AzureStorageResponseException extends HttpResponseException {

Where do you use this code?

> +import org.jclouds.date.TimeStamp;
+import org.jclouds.json.config.GsonModule;
+import org.jclouds.rest.ConfiguresHttpApi;
+import org.jclouds.rest.config.HttpApiModule;
+
+import javax.inject.Named;
+import java.util.concurrent.TimeUnit;
+
+import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL;
+
+@ConfiguresHttpApi
+public class AzureStorageQueueModule extends 
HttpApiModule<AzureStorageQueueApi> {
+   @Override
+   protected void configure() {
+      
bind(GsonModule.DateAdapter.class).to(GsonModule.Iso8601DateAdapter.class);
+      super.configure();

Call `super` first.

> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azure.storage.domain;
+
+import com.google.common.collect.Maps;
+
+import java.util.Map;
+
+/**
+ * When an Azure Storage request is in error, the client receives an error 
response.
+ *
+ * @see <a href="http://msdn.microsoft.com/en-us/library/dd573365.aspx"; />
+ */
+public class AzureStorageError {

Rename to `AzureQueueStorageError`.

> + * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azure.storage.domain;
+
+import java.net.URI;
+import java.util.Set;
+
+public interface BoundedSet<T> extends Set<T> {

Remove unused code.

> +
+import javax.xml.bind.annotation.XmlAccessType;
+import javax.xml.bind.annotation.XmlAccessorType;
+import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlRootElement;
+import javax.xml.bind.annotation.adapters.CollapsedStringAdapter;
+import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
+
+
+
+@XmlAccessorType(XmlAccessType.FIELD)
+public class Queue {
+   @XmlElement(name = "Name")
+   private String name;
+
+   // TODO: remove?

Remove `url` field which is no longer supported:

https://docs.microsoft.com/en-us/rest/api/storageservices/list-queues1

> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azure.storage.features;
+
+import 
org.jclouds.azure.storage.domain.internals.QueueResponse.CreateQueueResponse;
+import 
org.jclouds.azure.storage.domain.internals.QueueResponse.DeleteQueueResponse;
+import 
org.jclouds.azure.storage.domain.internals.QueueResponse.ListQueueResponse;
+import org.jclouds.azure.storage.filters.SharedKeyLiteAuthentication;
+import org.jclouds.azure.storage.parser.ParseCreateQueueResponse;
+import org.jclouds.azure.storage.parser.ParseDeleteQueueResponse;
+import org.jclouds.rest.annotations.*;
+
+import javax.inject.Named;
+import javax.ws.rs.*;

error-prone does not allow wildcard imports.  See the CloudBees log.

> +import java.util.Collection;
+import java.util.Set;
+
+import static com.google.common.io.BaseEncoding.base64;
+import static com.google.common.io.ByteStreams.readBytes;
+import static org.jclouds.crypto.Macs.asByteProcessor;
+import static org.jclouds.util.Patterns.NEWLINE_PATTERN;
+import static org.jclouds.util.Strings2.toInputStream;
+
+/**
+ * Signs the Azure Storage request.
+ *
+ * @see <a href= "http://msdn.microsoft.com/en-us/library/dd179428.aspx"; />
+ */
+@Singleton
+public class SharedKeyLiteAuthentication implements HttpRequestFilter {

azureblob and gogrid have identical classes.  We should factor these out into 
some common location.  Please open a JIRA issue and add a TODO here.

> +import org.jclouds.azure.storage.parser.ParseDeleteQueueResponse;
+import org.jclouds.rest.annotations.*;
+
+import javax.inject.Named;
+import javax.ws.rs.*;
+import java.io.Closeable;
+
+@Headers(keys = "x-ms-version", values = "{jclouds.api-version}")
+@RequestFilters(SharedKeyLiteAuthentication.class)
+public interface QueueApi extends Closeable {
+
+   @Named("azure_storage_queue_create")
+   @PUT
+   @Path("/{queueName}")
+   @ResponseParser(ParseCreateQueueResponse.class)
+   CreateQueueResponse create(@PathParam("queueName") String queueName);

This is a wacky interface -- instead can you throw an exception when create 
fails?

> +   @PUT
+   @Path("/{queueName}")
+   @ResponseParser(ParseCreateQueueResponse.class)
+   CreateQueueResponse create(@PathParam("queueName") String queueName);
+
+   @Named("azure_storage_queue_list")
+   @GET
+   @QueryParams(keys = "comp", values = "list")
+   @JAXBResponseParser
+   ListQueueResponse list();
+
+   @Named("azure_storage_queue_delete")
+   @DELETE
+   @Path("/{queueName}")
+   @ResponseParser(ParseDeleteQueueResponse.class)
+   DeleteQueueResponse delete(@PathParam("queueName") String queueName);

Same criticism.

> + */
+public final class AzureStorageHeaders {
+
+   public static final String CACHE_CONTROL = "x-ms-blob-cache-control";
+   public static final String CONTENT_DISPOSITION = 
"x-ms-blob-content-disposition";
+   public static final String CONTENT_ENCODING = "x-ms-blob-content-encoding";
+   public static final String CONTENT_LANGUAGE = "x-ms-blob-content-language";
+   public static final String CONTENT_TYPE = "x-ms-blob-content-type";
+
+   public static final String USER_METADATA_PREFIX = "x-ms-meta-";
+
+   public static final String COPY_SOURCE = "x-ms-copy-source";
+   public static final String COPY_SOURCE_IF_MODIFIED_SINCE = 
"x-ms-source-if-modified-since";
+   public static final String COPY_SOURCE_IF_UNMODIFIED_SINCE = 
"x-ms-source-if-unmodified-since";
+   public static final String COPY_SOURCE_IF_MATCH = "x-ms-source-if-match";
+   public static final String COPY_SOURCE_IF_NONE_MATCH = 
"x-ms-source-if-none-match";

Remove all unused fields.

> + *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azure.storage.reference;
+
+/**
+ * Additional headers specified by Azure Storage REST API.
+ *
+ * @see <a href="http://msdn.microsoft.com/en-us/library/dd179357.aspx"; />
+ */
+public final class AzureStorageHeaders {

Rename to `AzureQueueStorageHeaders`.

> +   SharedKeyLiteAuthentication signer;
+
+   @Inject
+   ParseSax.Factory factory;
+
+   @Inject
+   Provider<ErrorHandler> errorHandlerProvider;
+
+   public AzureStorageError parseAzureStorageErrorFromContent(HttpCommand 
command,
+                                                              HttpResponse 
response, InputStream content) throws HttpException {
+      AzureStorageError error = 
factory.create(errorHandlerProvider.get()).parse(content);
+      
error.setRequestId(response.getFirstHeaderOrNull(AzureStorageHeaders.REQUEST_ID));
+      if ("AuthenticationFailed".equals(error.getCode())) {
+         // this signature is incorrect for URLs from AzureBlobRequestSigner
+         
error.setStringSigned(signer.createStringToSign(command.getCurrentRequest()));
+         error.setSignature(signer.signString(error.getStringSigned()));

What does this code do?

> + *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.azure.storage.xml;
+
+import org.jclouds.azure.storage.domain.AzureStorageError;
+import org.jclouds.http.functions.ParseSax;
+
+/**
+ * Parses the error from the Amazon S3 REST API.

Bad comment.

> @@ -0,0 +1,40 @@
+<?xml version="1.0" encoding="utf-8"?>
+<StorageServiceProperties>
+    <Logging>
+        <Version>version-number</Version>
+        <Delete>true|false</Delete>

Not sure what this is but remove it since it is invalid input.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/405#pullrequestreview-52235800

Reply via email to