gaul requested changes on this pull request.
I still don't understand if this represent a work in progress or something that
actually functions. How would I manually test this? Where are the unit and
live tests which would eliminate the need for manual testing?
> @@ -110,7 +172,27 @@ HttpRequest replaceDateHeader(HttpRequest request) {
request =
request.toBuilder().replaceHeaders(Multimaps.forMap(builder.build())).build();
return request;
}
-
+
+ /**
+ * this is the method to parse container name and blob name from the
HttpRequest.
+ * applicable for the cases with SAS Authentication
+ *
+ */
+ public String[] cutUri (URI uri) throws NullPointerException,
IllegalArgumentException {
+ String path = uri.getPath();
+ String[] result = path.split("/");
+ if (result.length < 2) {
+ throw new IllegalArgumentException("there is neither ContainerName
nor BlobName in the URI path");
+ }
+ for (int i = 1; i < result.length; i++){
+ if (result[i] == null) {
Can this ever happen? Did you mean to check for the empty string?
> @@ -141,7 +223,7 @@ public String calculateSignature(String toSign) throws
> HttpException {
return signature;
}
- public String signString(String toSign) {
+ public String signString(String toSign) {
Spurious change.
> this.storageUrl = URI.create("https://"; + creds.get().identity +
> ".blob.core.windows.net/");
this.blob2HttpGetOptions = checkNotNull(blob2HttpGetOptions,
"blob2HttpGetOptions");
this.timeStampProvider = checkNotNull(timeStampProvider,
"timeStampProvider");
this.dateService = checkNotNull(dateService, "dateService");
this.auth = auth;
+ this.isSAS = sasAuthentication;
Tabs instead of spaces (repeated throughout).
> }
-
+
+ /**
+ * this is an updated filter method, which decides whether the SAS or
SharedKeyLite
Javadoc comments should look like this (space before *):
```
/**
*
*/
```
> + /**
+ * this filter method is applied only for the cases with SAS Authentication.
+ *
+ */
+ public HttpRequest filterSAS(HttpRequest request, String credential) throws
HttpException {
+ String containerName = null;
+ String blobName = null;
+ URI requestUri = request.getEndpoint();
+ try {
+ String[] parametersArray = cutUri(requestUri);
+ containerName = parametersArray[1];
+ UriBuilder endpoint =
Uris.uriBuilder(storageUrl).appendPath(containerName);
+ if (parametersArray.length == 3) {
+ endpoint.appendPath(parametersArray[2]).query(credential);
+ } else {
+ endpoint.query("restype=container&" + credential);
Does this properly format query parameters? Would `UriBuilder.addQuery(String,
String)` address this more clearly?
> @@ -74,6 +74,23 @@ void testIdempotent(HttpRequest request) {
System.out.printf("%s: %d iterations before the timestamp updated %n",
Thread.currentThread().getName(),
iterations);
}
+
+ @Test
These unit tests cover a tiny part of the functionality. We need tests like
`apis/s3/src/test/java/org/jclouds/s3/filters/RequestAuthorizeSignatureV2Test.java`
and `apis/s3/src/test/java/org/jclouds/s3/S3ClientLiveTest.java` to ensure
this functionality works.
--
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/pull/1270#pullrequestreview-202293076
