You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2016/07/13 00:35:54 UTC

[jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

* upgrade to Guava 19
* replace TypeToken.isAssignableFrom with TypeToken.isSupertypeOf,
  available starting in Guava 19
* add needed JSR305 and error-prone annotation dependencies
* consume or ignore values from methods with CheckReturnValue
* replace usage of removed Iterators.emptyIterator
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/982

-- Commit Summary --

  * JCLOUDS-1074: Guava 20 compatibility

-- File Changes --

    M apis/rackspace-cloudfiles/src/test/java/org/jclouds/rackspace/cloudfiles/v1/options/UpdateCDNContainerOptionsTest.java (2)
    M common/googlecloud/src/main/java/org/jclouds/googlecloud/internal/ListPages.java (3)
    M core/pom.xml (8)
    M core/src/main/java/org/jclouds/ContextBuilder.java (2)
    M core/src/main/java/org/jclouds/apis/ApiPredicates.java (4)
    M core/src/main/java/org/jclouds/apis/Apis.java (2)
    M core/src/main/java/org/jclouds/internal/BaseView.java (2)
    M core/src/main/java/org/jclouds/providers/ProviderPredicates.java (2)
    M core/src/main/java/org/jclouds/rest/InputParamValidator.java (2)
    M project/pom.xml (14)
    M providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/features/PlacementGroupApiLiveTest.java (1)
    M providers/softlayer/src/test/java/org/jclouds/softlayer/features/VirtualGuestApiLiveTest.java (3)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/982.patch
https://github.com/jclouds/jclouds/pull/982.diff

---
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/982

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
This pull request may prove contentious since it upgrades Guava to the latest release to work around a removed method.  Do we want to go through the pain of reflection to make this more compatible?

@blop opened in response to [JCLOUDS-1074](https://issues.apache.org/jira/browse/JCLOUDS-1074)

---
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/982#issuecomment-232223013

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> +            // Guava 18 and earlier method
> +            isSuperTypeOfType = TypeToken.class.getDeclaredMethod("isAssignableFrom", Type.class);
> +            isSuperTypeOfTypeToken = TypeToken.class.getDeclaredMethod("isAssignableFrom", TypeToken.class);
> +         } catch (NoSuchMethodException nsme2) {
> +            throw Throwables.propagate(nsme2);
> +         }
> +      }
> +      IS_SUPERTYPE_OF_TYPE = isSuperTypeOfType;
> +      IS_SUPERTYPE_OF_TYPETOKEN = isSuperTypeOfTypeToken;
> +   }
> +
> +   private TypeTokenUtils() {
> +      throw new AssertionError("intentionally not implemented");
> +   }
> +
> +   public static <C> boolean isSupertypeOf(TypeToken<C> token, Type type) {

Do we know if this is much more efficient that simply doing:
```
  return isSupertypeOf(token, TypeToken.of(type));
```
? See https://google.github.io/guava/releases/19.0/api/docs/com/google/common/reflect/TypeToken.html#of(java.lang.reflect.Type).

That would allow to get down the class to use only _one_ method?

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77363270

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Ignasi Barrera <no...@github.com>.
We also need to make sure we don't break jclouds-karaf and the CLI.

---
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/982#issuecomment-232561257

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
> @@ -133,7 +134,7 @@ public void testSetTagsOnVirtualGuest() throws Exception {
>        Set<TagReference> tagReferences = found.getTagReferences();
>        assertNotNull(tagReferences);
>        for (String tag : tags) {
> -         Iterables.contains(tagReferences, tag);
> +         assertThat(Iterables.contains(tagReferences, tag)).isTrue();

This is due to Guava 20 introducing annotating new methods which error-prone catches.

---
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/982/files/de6171ce5d7c55b8733bbbfc3e7606a0739f2dad#r70562697

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Ignasi Barrera <no...@github.com>.
Thanks @andrewgaul! Just one comment in a test.

-- 
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/982#issuecomment-244299902

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Ignasi Barrera <no...@github.com>.
This is something we want to bring to the dev mailing list.

The only problem with this is dropping compatibility with Guava < 19, which will probably have a big impact in users. However, if we don't do such a change in 2.0... then when?

Like @demobox, I'm a bit hesitant until we know a bit more about the Guava versions people are using now, although not that worried by the version gap between 1.9.x and 2.0. We already have the [guava/guice compatibility build](https://jclouds.ci.cloudbees.com/job/jclouds-guava-guice-compat/) and I'd say it should work for 1.9.x too. Despite Guava 16 being the default version we use, Guava 19 should be already supported, so users should be able to upgrade Guava without upgrading jclouds, when planning a future upgrade to 2.0.

We should run a Guava 19 build against 1.9.x and, if it succeeds, I'd be happy to have this merged as soon as we document a smooth upgrade plan for users willing to upgrade to 2.0.

---
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/982#issuecomment-232561019

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
I am working on this and should complete it over the weekend.

-- 
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/982#issuecomment-244176023

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Ignasi Barrera <no...@github.com>.
Ping?

-- 
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/982#issuecomment-242943783

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> @@ -45,7 +45,7 @@ protected BaseView(@Provider Context backend, @Provider TypeToken<? extends Cont
>     @SuppressWarnings("unchecked")
>     @Override
>     public <C extends Context> C unwrap(TypeToken<C> type) {
> -      checkArgument(checkNotNull(type, "type").isAssignableFrom(backendType), "backend type: %s not assignable to %s", backendType, type);
> +      checkArgument(checkNotNull(type, "type").isSupertypeOf(backendType), "backend type: %s not assignable to %s", backendType, type);

[minor] Perhaps change the message to `type: %s is not a supertype of %s...` to match the code? Probably more comprehensible for users, too ;-)

---
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/982/files/de6171ce5d7c55b8733bbbfc3e7606a0739f2dad#r70555934

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> This is something we want to bring to the dev mailing list...we should run a Guava 19 build against 1.9.x 

Sounds like a good plan to me.

> Despite Guava 16 being the default version we use, Guava 19 should be already supported

It's not so much that we already support 19 that I'm concerned about, more that we currently allow users of our stable version to use any Guava version from 16 upwards. If we move to 19, that will force all stable users to jump up to 3 major versions (two years of development) of a commonly-used library, and we don't know how many other libraries they might be using which are **not** compatible with Guava 19? There's a decent amount of changes from 16 to 19:

* http://google.github.io/guava/releases/17.0/api/diffs/
* http://google.github.io/guava/releases/18.0/api/diffs/
* http://google.github.io/guava/releases/19.0/api/diffs/

Having said that, I agree that 2.0 would be the most obvious time to consider this. Still, I'd want to avoid a situation where we end up losing more 2.0 users because of a forced Guava jump than we gain from being Guava 20 compatible.

To be continued on the dev list, I suspect... ;-)

---
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/982#issuecomment-232640304

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
> +         isSuperTypeOfTypeToken = TypeToken.class.getDeclaredMethod("isSupertypeOf", TypeToken.class);
> +      } catch (NoSuchMethodException nsme) {
> +         try {
> +            // Guava 18 and earlier method
> +            isSuperTypeOfType = TypeToken.class.getDeclaredMethod("isAssignableFrom", Type.class);
> +            isSuperTypeOfTypeToken = TypeToken.class.getDeclaredMethod("isAssignableFrom", TypeToken.class);
> +         } catch (NoSuchMethodException nsme2) {
> +            throw Throwables.propagate(nsme2);
> +         }
> +      }
> +      IS_SUPERTYPE_OF_TYPE = isSuperTypeOfType;
> +      IS_SUPERTYPE_OF_TYPETOKEN = isSuperTypeOfTypeToken;
> +   }
> +
> +   private TypeTokenUtils() {
> +      throw new AssertionError("intentionally not implemented");

We use the `AssertionError` pattern throughout.

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77431175

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
Based on your feedback I will go through the small pain of reflection to address Guava 20 compatibility.

---
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/982#issuecomment-233530069

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
> @@ -45,7 +46,7 @@ protected BaseView(@Provider Context backend, @Provider TypeToken<? extends Cont
>     @SuppressWarnings("unchecked")
>     @Override
>     public <C extends Context> C unwrap(TypeToken<C> type) {
> -      checkArgument(checkNotNull(type, "type").isAssignableFrom(backendType), "backend type: %s not assignable to %s", backendType, type);
> +      checkArgument(TypeTokenUtils.isSupertypeOf(checkNotNull(type, "type"), backendType), "backend type: %s is not a supertype of %s", backendType, type);

Done.

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77431189

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> This pull request may prove contentious since it upgrades Guava to the latest release to work around a removed method. Do we want to go through the pain of reflection to make this more compatible?

Thanks for the warning, @andrewgaul! Hm...Guava 20 isn't even released yet, and we are still compatible with 19.0, which is the latest official version. On the other hand, given that this is `master` and we are getting close to 2.0, it seems like waiting until _after_ 2.0 for this is not going to make things any easier.

* do we have any idea what the tentative release date is for Guava 20?
* is there any way we could easily find out which Guava versions people are using? Specifically, how many are looking at using 20?

Given that we are on 16 on the next branch down, forcing 19 on to users from `master` seems like a pretty big gap. Unless we have good data that lots of users are planning to use 20 (only 1 vote on the issue so far), I'm hesitant about this one as I think it will force more users into upgrading than it will help users looking to use 20.

Can we put a call out to ask users to upvote JCLOUDS-1074 if the are affected by this?

---
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/982#issuecomment-232544143

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> Based on your feedback I will go through the small pain of reflection to address Guava 20 compatibility.

Thanks, Andrew G!

---
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/982#issuecomment-234272985

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
>        }
> +      Set<String> actualTags = actualTagsBuilder.build();
> +      assertThat(actualTags).containsAll(tags);

Just curious: related to this PR, or just cleanup?

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77363775

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
> @@ -133,7 +134,7 @@ public void testSetTagsOnVirtualGuest() throws Exception {
>        Set<TagReference> tagReferences = found.getTagReferences();
>        assertNotNull(tagReferences);
>        for (String tag : tags) {
> -         Iterables.contains(tagReferences, tag);
> +         assertThat(Iterables.contains(tagReferences, tag)).isTrue();

Inspecting this further, this test always failed due to `tagReferences` having a different type than `tag`.  I have corrected it.

---
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/982/files/de6171ce5d7c55b8733bbbfc3e7606a0739f2dad#r70564224

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
Updated without Guava 19 dependency.

-- 
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/982#issuecomment-244283238

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> @@ -50,6 +50,14 @@
>        <version>1.1.1</version>
>      </dependency>
>      <dependency>
> +      <groupId>com.google.code.findbugs</groupId>
> +      <artifactId>jsr305</artifactId>
> +    </dependency>
> +    <dependency>
> +      <groupId>com.google.errorprone</groupId>
> +      <artifactId>error_prone_annotations</artifactId>
> +    </dependency>
> +    <dependency>

Have these been broken out of the Guava dep?

---
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/982/files/de6171ce5d7c55b8733bbbfc3e7606a0739f2dad#r70555860

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> @@ -104,7 +104,7 @@ public void testPeanutButterDidntTurnIntoWine() {
>           wine.unwrap(typeToken(PeanutButter.class));
>           fail();
>        } catch (IllegalArgumentException e) {
> -         assertEquals(e.getMessage(), "backend type: org.jclouds.internal.BaseViewTest$Water not assignable to org.jclouds.internal.BaseViewTest$PeanutButter");
> +         assertEquals(e.getMessage(), "backend type: org.jclouds.internal.BaseViewTest$Water is not a supertype of org.jclouds.internal.BaseViewTest$PeanutButter");

I think this is **our** exception, thrown in `Context.unwrap` and modified in this PR above?

@andrewgaul Does that sound right?

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77363688

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
> @@ -45,7 +45,7 @@ protected BaseView(@Provider Context backend, @Provider TypeToken<? extends Cont
>     @SuppressWarnings("unchecked")
>     @Override
>     public <C extends Context> C unwrap(TypeToken<C> type) {
> -      checkArgument(checkNotNull(type, "type").isAssignableFrom(backendType), "backend type: %s not assignable to %s", backendType, type);
> +      checkArgument(checkNotNull(type, "type").isSupertypeOf(backendType), "backend type: %s not assignable to %s", backendType, type);

Done.

---
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/982/files/de6171ce5d7c55b8733bbbfc3e7606a0739f2dad#r70562678

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
> +            // Guava 18 and earlier method
> +            isSuperTypeOfType = TypeToken.class.getDeclaredMethod("isAssignableFrom", Type.class);
> +            isSuperTypeOfTypeToken = TypeToken.class.getDeclaredMethod("isAssignableFrom", TypeToken.class);
> +         } catch (NoSuchMethodException nsme2) {
> +            throw Throwables.propagate(nsme2);
> +         }
> +      }
> +      IS_SUPERTYPE_OF_TYPE = isSuperTypeOfType;
> +      IS_SUPERTYPE_OF_TYPETOKEN = isSuperTypeOfTypeToken;
> +   }
> +
> +   private TypeTokenUtils() {
> +      throw new AssertionError("intentionally not implemented");
> +   }
> +
> +   public static <C> boolean isSupertypeOf(TypeToken<C> token, Type type) {

Your suggestion is shorter and the result is equivalent but I prefer to call through to the reflected method to preserve the intent of the workaround.

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77431214

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> +         isSuperTypeOfTypeToken = TypeToken.class.getDeclaredMethod("isSupertypeOf", TypeToken.class);
> +      } catch (NoSuchMethodException nsme) {
> +         try {
> +            // Guava 18 and earlier method
> +            isSuperTypeOfType = TypeToken.class.getDeclaredMethod("isAssignableFrom", Type.class);
> +            isSuperTypeOfTypeToken = TypeToken.class.getDeclaredMethod("isAssignableFrom", TypeToken.class);
> +         } catch (NoSuchMethodException nsme2) {
> +            throw Throwables.propagate(nsme2);
> +         }
> +      }
> +      IS_SUPERTYPE_OF_TYPE = isSuperTypeOfType;
> +      IS_SUPERTYPE_OF_TYPETOKEN = isSuperTypeOfTypeToken;
> +   }
> +
> +   private TypeTokenUtils() {
> +      throw new AssertionError("intentionally not implemented");

[minor] Or [`UnsupportedOperationException`](http://docs.oracle.com/javase/6/docs/api/java/lang/UnsupportedOperationException.html)? But if `AssertionError` is what we use in similar situations elsewhere, fine with that

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77362885

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
>        }
> +      Set<String> actualTags = actualTagsBuilder.build();
> +      assertThat(actualTags).containsAll(tags);

Guava 20 adds a `CheckReturnValue` annotation to several methods which we must consume when compiling with the newer library.

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77431185

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Ignasi Barrera <no...@github.com>.
Can we setup a Guavva 20 Jenkins build to validate this branch?

---
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/982#issuecomment-232363809

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
> @@ -50,6 +50,14 @@
>        <version>1.1.1</version>
>      </dependency>
>      <dependency>
> +      <groupId>com.google.code.findbugs</groupId>
> +      <artifactId>jsr305</artifactId>
> +    </dependency>
> +    <dependency>
> +      <groupId>com.google.errorprone</groupId>
> +      <artifactId>error_prone_annotations</artifactId>
> +    </dependency>
> +    <dependency>

Sorry the jsr305 was a spurious change.  Without the error-prone annotations dependency, I see errors of the form:

```
error: cannot access CanIgnoreReturnValue
  class file for com.google.errorprone.annotations.CanIgnoreReturnValue not found
```

---
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/982/files/de6171ce5d7c55b8733bbbfc3e7606a0739f2dad#r70564003

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Gaul <no...@github.com>.
> @@ -104,7 +104,7 @@ public void testPeanutButterDidntTurnIntoWine() {
>           wine.unwrap(typeToken(PeanutButter.class));
>           fail();
>        } catch (IllegalArgumentException e) {
> -         assertEquals(e.getMessage(), "backend type: org.jclouds.internal.BaseViewTest$Water not assignable to org.jclouds.internal.BaseViewTest$PeanutButter");
> +         assertEquals(e.getMessage(), "backend type: org.jclouds.internal.BaseViewTest$Water is not a supertype of org.jclouds.internal.BaseViewTest$PeanutButter");

Yes `BaseView` generates this, albeit with the typo as remarked on below.

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77431173

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> @@ -45,7 +46,7 @@ protected BaseView(@Provider Context backend, @Provider TypeToken<? extends Cont
>     @SuppressWarnings("unchecked")
>     @Override
>     public <C extends Context> C unwrap(TypeToken<C> type) {
> -      checkArgument(checkNotNull(type, "type").isAssignableFrom(backendType), "backend type: %s not assignable to %s", backendType, type);
> +      checkArgument(TypeTokenUtils.isSupertypeOf(checkNotNull(type, "type"), backendType), "backend type: %s is not a supertype of %s", backendType, type);

Should the error message be the other way around? I.e. `"type %s is not a supertype of backend type %s", type, backendType'?

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77362529

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Ignasi Barrera <no...@github.com>.
> @@ -104,7 +104,7 @@ public void testPeanutButterDidntTurnIntoWine() {
>           wine.unwrap(typeToken(PeanutButter.class));
>           fail();
>        } catch (IllegalArgumentException e) {
> -         assertEquals(e.getMessage(), "backend type: org.jclouds.internal.BaseViewTest$Water not assignable to org.jclouds.internal.BaseViewTest$PeanutButter");
> +         assertEquals(e.getMessage(), "backend type: org.jclouds.internal.BaseViewTest$Water is not a supertype of org.jclouds.internal.BaseViewTest$PeanutButter");

Does this message depend on the Guava version being used? In that case we need to change the assertion, otherwise our builds that check different versions of Guava will fail.

-- 
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/982/files/5be952718a45dd48d05cd93bb2f3a0b2a51b23ff#r77302215

Re: [jclouds/jclouds] JCLOUDS-1074: Guava 20 compatibility (#982)

Posted by Andrew Phillips <no...@github.com>.
> @@ -133,7 +134,7 @@ public void testSetTagsOnVirtualGuest() throws Exception {
>        Set<TagReference> tagReferences = found.getTagReferences();
>        assertNotNull(tagReferences);
>        for (String tag : tags) {
> -         Iterables.contains(tagReferences, tag);
> +         assertThat(Iterables.contains(tagReferences, tag)).isTrue();

Nice catch - thanks!

---
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/982/files/de6171ce5d7c55b8733bbbfc3e7606a0739f2dad#r70556030