You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jeremy Daggett <no...@github.com> on 2014/04/04 21:21:19 UTC
[jclouds] Allow Swift to set content type (#338)
This PR fixes the case where jclouds is assuming the blob content type to be `application/unknown`. The original discussion can be found [here](http://markmail.org/thread/qdxqrkhnjygjny4k).
If the user does not specify a content type, then the `X-Detect-Conent-Type` header should be added to the request headers to indicate that the server should detect the content type.
This feature is supported in the Swift 1.9.1 through 1.13.1rc1 [releases](https://bugs.launchpad.net/openstack-api-site/+bug/1274988).
A test case is in the works now and I wanted to get some other eyes on it. DO NOT MERGE.
You can merge this Pull Request by running:
git pull https://github.com/rackspace/jclouds fix-swift-content-type
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/338
-- Commit Summary --
* Allow Swift to set content type
-- File Changes --
M apis/swift/src/main/java/org/jclouds/openstack/swift/blobstore/functions/BlobToObject.java (11)
M apis/swift/src/main/java/org/jclouds/openstack/swift/reference/SwiftHeaders.java (3)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/338.patch
https://github.com/jclouds/jclouds/pull/338.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338
Re: [jclouds] Allow Swift to set content type (#338)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1289](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1289/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-43784792
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Zack Shoylev <no...@github.com>.
I need to double check this does not affect the metadata updates. Some of the new tests will probably catch it if it does, though. Risk?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-43691164
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-pull-requests #734 UNSTABLE
Unrelated [test failure](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/org.apache.jclouds$jclouds-compute/734/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-39638458
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Andrew Phillips <no...@github.com>.
> we should have a unit test for this
Good catch, @shrinandj - yes, indeed, we're missing a test here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-40775344
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Everett Toews <no...@github.com>.
Closed #338.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#event-184125408
Re: [jclouds] Allow Swift to set content type (#338)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1128](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1128/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-43770835
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Zack Shoylev <no...@github.com>.
Closed #338.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#event-123399739
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #1128 FAILURE
Broken BuildHive again :-( I'll reopen the support ticket we had with CloudBees for this one.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-43784218
Re: [jclouds] Allow Swift to set content type (#338)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #998](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/998/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-39606744
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Zack Shoylev <no...@github.com>.
Reopened #338.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#event-123399744
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -45,7 +47,14 @@ public SwiftObject apply(Blob from) {
> return null;
> SwiftObject object = objectProvider.create(blob2ObjectMd.apply(from.getMetadata()));
> object.setPayload(checkNotNull(from.getPayload(), "payload: " + from));
> - object.setAllHeaders(from.getAllHeaders());
> +
> + Multimap<String, String> headers = from.getAllHeaders();
> + // let Swift determine the content type...
> + if (from.getMetadata().getContentMetadata().getContentType().equals("application/unknown")) {
Agreed. Good catch on the case sensitivity and the potential null!
I believe that we should move *all* String representations of MIME types to the Guava [MediaType](http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/net/MediaType.html) class. ;)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338/files#r11759878
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Jeremy Daggett <no...@github.com>.
@shrinandj @demobox Yes, there is no test there now, but there will be.I will get back to this PR next week.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-40775757
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Zack Shoylev <no...@github.com>.
Well the other tests look good. +1
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-43796261
Re: [jclouds] Allow Swift to set content type (#338)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1204](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1204/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-39607849
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Zack Shoylev <no...@github.com>.
Retriggering tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-43770700
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Shri Javadekar <no...@github.com>.
Apart from what @andrewgaul said, we should have a unit test for this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-40763386
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Andrew Phillips <no...@github.com>.
> @@ -45,7 +47,14 @@ public SwiftObject apply(Blob from) {
> return null;
> SwiftObject object = objectProvider.create(blob2ObjectMd.apply(from.getMetadata()));
> object.setPayload(checkNotNull(from.getPayload(), "payload: " + from));
> - object.setAllHeaders(from.getAllHeaders());
> +
> + Multimap<String, String> headers = from.getAllHeaders();
> + // let Swift determine the content type...
> + if (from.getMetadata().getContentMetadata().getContentType().equals("application/unknown")) {
Make this a constant (is it perhaps already a constant somewhere)? And do we need a case-insensitive comparison here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338/files#r11321333
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Andrew Phillips <no...@github.com>.
Still have an open comment about the potential NPE. @jdaggett thoughts on that?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-44225595
Re: [jclouds] Allow Swift to set content type (#338)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #734](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/734/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-39607179
Re: [jclouds] Allow Swift to set content type (#338)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #819](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/819/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-43783779
Re: [jclouds] Allow Swift to set content type (#338)
Posted by Everett Toews <no...@github.com>.
It's release time in jclouds and that means we like to do a little house cleaning by sweeping out the pull request queue. This PR hasn't been touched in over 6 months. Please give us a status update.
If we don't hear anything by the end of the week, we'll close this pull request.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/338#issuecomment-59872311