Fix O(n^2) response time for "list-security-groups" on openstack-nova.

For https://issues.apache.org/jira/browse/JCLOUDS-1235.

This change takes the approach of storing the information about the
overall list of groups within the `SecurityGroupInRegion` when it is
created, so that any subsequent conversion operation has access to all
the groups in the same region as the one to be converted.

It also collapses the functionality of `NovaSecurityGroupToSecurityGroup`,
`SecurityGroupRuleToIpPermission` and `FindSecurityGroupWithNameAndReturnTrue`
all into `NovaSecurityGroupInRegionToSecurityGroup`, and deletes the
now unused-classes SecurityGroupRuleToIpPermission,
NovaSecurityGroupToSecurityGroup and associated tests.

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/config/NovaComputeServiceContextModule.java

@@ -46,7 +46,6 @@ import org.jclouds.compute.strategy.impl.CreateNodesWithGroupEncodedIntoNameThen
 import org.jclouds.domain.Location;
 import org.jclouds.domain.LoginCredentials;
 import org.jclouds.functions.IdentityFunction;
-import org.jclouds.net.domain.IpPermission;
 import org.jclouds.openstack.nova.v2_0.compute.NovaComputeService;
 import org.jclouds.openstack.nova.v2_0.compute.NovaComputeServiceAdapter;
 import org.jclouds.openstack.nova.v2_0.compute.extensions.NovaImageExtension;
@@ -57,9 +56,7 @@ import org.jclouds.openstack.nova.v2_0.compute.functions.FlavorInRegionToHardwar
 import org.jclouds.openstack.nova.v2_0.compute.functions.ImageInRegionToImage;
 import org.jclouds.openstack.nova.v2_0.compute.functions.ImageToOperatingSystem;
 import org.jclouds.openstack.nova.v2_0.compute.functions.NovaSecurityGroupInRegionToSecurityGroup;
-import org.jclouds.openstack.nova.v2_0.compute.functions.NovaSecurityGroupToSecurityGroup;
 import org.jclouds.openstack.nova.v2_0.compute.functions.OrphanedGroupsByRegionId;
-import org.jclouds.openstack.nova.v2_0.compute.functions.SecurityGroupRuleToIpPermission;
 import org.jclouds.openstack.nova.v2_0.compute.functions.ServerInRegionToNodeMetadata;
 import org.jclouds.openstack.nova.v2_0.compute.loaders.CreateUniqueKeyPair;
 import org.jclouds.openstack.nova.v2_0.compute.loaders.FindSecurityGroupOrCreate;
@@ -68,7 +65,6 @@ import org.jclouds.openstack.nova.v2_0.compute.options.NovaTemplateOptions;
 import org.jclouds.openstack.nova.v2_0.compute.strategy.ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet;
 import org.jclouds.openstack.nova.v2_0.domain.FloatingIP;
 import org.jclouds.openstack.nova.v2_0.domain.KeyPair;
-import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule;
 import org.jclouds.openstack.nova.v2_0.domain.Server;
 import org.jclouds.openstack.nova.v2_0.domain.regionscoped.FlavorInRegion;
 import org.jclouds.openstack.nova.v2_0.domain.regionscoped.ImageInRegion;
@@ -114,12 +110,6 @@ public class NovaComputeServiceContextModule extends
       bind(new TypeLiteral<Function<ServerInRegion, NodeMetadata>>() {
       }).to(ServerInRegionToNodeMetadata.class);
 
-      bind(new TypeLiteral<Function<SecurityGroupRule, IpPermission>>() {
-      }).to(SecurityGroupRuleToIpPermission.class);
-
-      bind(new TypeLiteral<Function<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup, SecurityGroup>>() {
-      }).to(NovaSecurityGroupToSecurityGroup.class);
-
       bind(new TypeLiteral<Function<SecurityGroupInRegion, SecurityGroup>>() {
       }).to(NovaSecurityGroupInRegionToSecurityGroup.class);
 

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java

@@ -58,6 +58,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.common.base.Supplier;
 import com.google.common.cache.LoadingCache;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Multimap;
 import com.google.common.util.concurrent.ListeningExecutorService;
@@ -135,8 +136,9 @@ public class NovaSecurityGroupExtension implements SecurityGroupExtension {
       }
 
       Set<String> groupNames = instance.getSecurityGroupNames();
+       final FluentIterable<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup> allGroups = sgApi.get().list();
        Set<? extends SecurityGroupInRegion> rawGroups =
-              sgApi.get().list().filter(nameIn(groupNames)).transform(groupToGroupInRegion(region)).toSet();
+              allGroups.filter(nameIn(groupNames)).transform(groupToGroupInRegion(allGroups, region)).toSet();
 
       return ImmutableSet.copyOf(transform(filter(rawGroups, notNull()), groupConverter));
    }
@@ -153,7 +155,8 @@ public class NovaSecurityGroupExtension implements SecurityGroupExtension {
          return null;
       }
 
-      SecurityGroupInRegion rawGroup = new SecurityGroupInRegion(sgApi.get().get(groupId), region);
+      final FluentIterable<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup> allGroups = sgApi.get().list();
+      SecurityGroupInRegion rawGroup = new SecurityGroupInRegion(sgApi.get().get(groupId), region, allGroups);
 
       return groupConverter.apply(rawGroup);
    }
@@ -359,17 +362,20 @@ public class NovaSecurityGroupExtension implements SecurityGroupExtension {
             }
 
 
-            return sgApi.get().list().transform(groupToGroupInRegion(from)).toSet();
+             final FluentIterable<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup> allGroups = sgApi.get().list();
+             return allGroups.transform(groupToGroupInRegion(allGroups, from)).toSet();
          }
 
       };
    }
 
-   protected Function<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup, SecurityGroupInRegion> groupToGroupInRegion(final String region) {
+   protected Function<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup, SecurityGroupInRegion> groupToGroupInRegion(
+       final Iterable<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup> allGroups, final String region) {
+
       return new Function<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup, SecurityGroupInRegion>() {
          @Override
          public SecurityGroupInRegion apply(org.jclouds.openstack.nova.v2_0.domain.SecurityGroup group) {
-            return new SecurityGroupInRegion(group, region);
+            return new SecurityGroupInRegion(group, region, allGroups);
          }
       };
    }

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CreateSecurityGroupIfNeeded.java

@@ -38,6 +38,7 @@ import org.jclouds.openstack.nova.v2_0.extensions.SecurityGroupApi;
 
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
+import com.google.common.collect.FluentIterable;
 
 @Singleton
 public class CreateSecurityGroupIfNeeded implements Function<RegionSecurityGroupNameAndPorts, SecurityGroupInRegion> {
@@ -58,9 +59,9 @@ public class CreateSecurityGroupIfNeeded implements Function<RegionSecurityGroup
       String regionId = regionSecurityGroupNameAndPorts.getRegion();
       Optional<? extends SecurityGroupApi> api = novaApi.getSecurityGroupApi(regionId);
       checkArgument(api.isPresent(), "Security groups are required, but the extension is not available in region %s!", regionId);
+      final FluentIterable<SecurityGroup> allGroups = api.get().list();
       logger.debug(">> creating securityGroup %s", regionSecurityGroupNameAndPorts);
       try {
-
          SecurityGroup securityGroup = api.get().createWithDescription(
             regionSecurityGroupNameAndPorts.getName(), regionSecurityGroupNameAndPorts.getName());
 
@@ -68,13 +69,12 @@ public class CreateSecurityGroupIfNeeded implements Function<RegionSecurityGroup
          for (int port : regionSecurityGroupNameAndPorts.getPorts()) {
             authorizeGroupToItselfAndAllIPsToTCPPort(api.get(), securityGroup, port);
          }
-         return new SecurityGroupInRegion(api.get().get(securityGroup.getId()), regionId);
+         return new SecurityGroupInRegion(api.get().get(securityGroup.getId()), regionId, allGroups);
       } catch (IllegalStateException e) {
          logger.trace("<< trying to find securityGroup(%s): %s", regionSecurityGroupNameAndPorts, e.getMessage());
-         SecurityGroup group = find(api.get().list(), nameEquals(regionSecurityGroupNameAndPorts
+         SecurityGroup group = find(allGroups, nameEquals(regionSecurityGroupNameAndPorts.getName()));
-                  .getName()));
          logger.debug("<< reused securityGroup(%s)", group.getId());
-         return new SecurityGroupInRegion(group, regionId);
+         return new SecurityGroupInRegion(group, regionId, allGroups);
       }
    }
 

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroup.java

@@ -18,7 +18,10 @@ package org.jclouds.openstack.nova.v2_0.compute.functions;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
 
+import java.util.Collection;
 import java.util.Map;
 
 import javax.annotation.Resource;
@@ -30,12 +33,18 @@ import org.jclouds.compute.domain.SecurityGroupBuilder;
 import org.jclouds.compute.reference.ComputeServiceConstants;
 import org.jclouds.domain.Location;
 import org.jclouds.logging.Logger;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule;
+import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName;
 import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion;
 
 import com.google.common.base.Function;
+import com.google.common.base.Predicates;
 import com.google.common.base.Supplier;
+import com.google.common.collect.Iterables;
 import com.google.inject.Inject;
 
+
 /**
  * A function for transforming a Nova-specific SecurityGroup into a generic
  * SecurityGroup object.
@@ -46,27 +55,81 @@ public class NovaSecurityGroupInRegionToSecurityGroup implements Function<Securi
     @Named(ComputeServiceConstants.COMPUTE_LOGGER)
     protected Logger logger = Logger.NULL;
 
-   protected final Function<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup, SecurityGroup> baseConverter;
     protected final Supplier<Map<String, Location>> locationIndex;
 
     @Inject
-   public NovaSecurityGroupInRegionToSecurityGroup(Function<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup, SecurityGroup> baseConverter,
+    public NovaSecurityGroupInRegionToSecurityGroup(Supplier<Map<String, Location>> locationIndex) {
-                                                 Supplier<Map<String, Location>> locationIndex) {
-      this.baseConverter = checkNotNull(baseConverter, "baseConverter");
         this.locationIndex = checkNotNull(locationIndex, "locationIndex");
     }
 
     @Override
-   public SecurityGroup apply(SecurityGroupInRegion group) {
+    public SecurityGroup apply(final SecurityGroupInRegion groupInRegion) {
-      SecurityGroupBuilder builder = SecurityGroupBuilder.fromSecurityGroup(baseConverter.apply(group.getSecurityGroup()));
+        SecurityGroupBuilder builder = new SecurityGroupBuilder();
 
-      Location region = locationIndex.get().get(group.getRegion());
+        final org.jclouds.openstack.nova.v2_0.domain.SecurityGroup group = groupInRegion.getSecurityGroup();
-      checkState(region != null, "location %s not in locationIndex: %s", group.getRegion(), locationIndex.get());
+        builder.id(group.getId());
+        builder.providerId(group.getId());
+        builder.ownerId(group.getTenantId());
+        builder.name(group.getName());
+        if (group.getRules() != null) {
+           builder.ipPermissions(filter(transform(group.getRules(), new Function<SecurityGroupRule, IpPermission>() {
+              @Override
+              public IpPermission apply(SecurityGroupRule input) {
+                 return securityGroupRuleToIpPermission(groupInRegion, input);
+              }
+           }), Predicates.notNull()));
+        }
+
+        final String regionId = groupInRegion.getRegion();
+        Location region = locationIndex.get().get(regionId);
+        checkState(region != null, "location %s not in locationIndex: %s", regionId, locationIndex.get());
 
         builder.location(region);
 
-      builder.id(group.getRegion() + "/" + group.getSecurityGroup().getId());
+        builder.id(regionId + "/" + group.getId());
 
         return builder.build();
     }
+
+    private IpPermission securityGroupRuleToIpPermission(SecurityGroupInRegion groupInRegion, SecurityGroupRule rule) {
+        IpPermission.Builder builder = IpPermission.builder();
+        builder.ipProtocol(rule.getIpProtocol());
+        builder.fromPort(rule.getFromPort());
+        builder.toPort(rule.getToPort());
+        final TenantIdAndName ruleGroup = rule.getGroup();
+        if (ruleGroup != null) {
+           final org.jclouds.openstack.nova.v2_0.domain.SecurityGroup owningGroup =
+              groupInRegion.getSecurityGroup();
+           final Collection<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup> referredGroup =
+              groupInRegion.getGroupsByName().get(ruleGroup);
+           if (null == referredGroup) {
+              logger.warn("Unknown group {} used in security rule, refusing to add it to {} ({})",
+                 ruleGroup, owningGroup.getName(), owningGroup.getId());
+              return null;
+           }
+           /*  Checking referredGroup.size(), see comments on SecurityGroupInRegion.getGroupsByName(). If there are
+               duplicate groups with the same tenant-id-and-name as that of ruleGroup then it is not possible
+               with the Nova /v2/12345/os-security-groups API to know which group is intended, as the only
+               information it returns about referred groups is the tenant id and name:
+                        "group": {
+                           "tenant_id": "a0ade3ca76784719845363979dc1014e",
+                           "name": "jclouds-qa-scheduler-docker-entity"
+                        },
+              Rather than pick one group at random and risk using the wrong group, here we fall back to the
+              least-worst option(?) and refuse to add any rule.
+
+              See https://issues.apache.org/jira/browse/JCLOUDS-1234.
+           */
+           if (referredGroup.size() != 1) {
+              logger.warn("Ambiguous group %s used in security rule, refusing to add it to %s (%s)",
+                 ruleGroup, owningGroup.getName(), owningGroup.getId());
+              return null;
+           }
+           builder.groupId(groupInRegion.getRegion() + "/" + Iterables.getOnlyElement(referredGroup).getId());
+        }
+        if (rule.getIpRange() != null) {
+            builder.cidrBlock(rule.getIpRange());
+        }
+        return builder.build();
+    }
 }

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroup.java

@@ -1,66 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * 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.openstack.nova.v2_0.compute.functions;
-
-import static com.google.common.collect.Iterables.transform;
-
-import javax.annotation.Resource;
-import javax.inject.Named;
-import javax.inject.Singleton;
-
-import org.jclouds.compute.domain.SecurityGroup;
-import org.jclouds.compute.domain.SecurityGroupBuilder;
-import org.jclouds.compute.reference.ComputeServiceConstants;
-import org.jclouds.logging.Logger;
-import org.jclouds.net.domain.IpPermission;
-import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule;
-
-import com.google.common.base.Function;
-import com.google.inject.Inject;
-
-/**
- * A function for transforming a Nova-specific SecurityGroup into a generic
- * SecurityGroup object.
- */
-@Singleton
-public class NovaSecurityGroupToSecurityGroup implements Function<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup, SecurityGroup> {
-   @Resource
-   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
-   protected Logger logger = Logger.NULL;
-
-   protected Function<SecurityGroupRule, IpPermission> ruleToPermission;
-
-   @Inject
-   public NovaSecurityGroupToSecurityGroup(Function<SecurityGroupRule, IpPermission> ruleToPermission) {
-      this.ruleToPermission = ruleToPermission;
-   }
-
-   @Override
-   public SecurityGroup apply(org.jclouds.openstack.nova.v2_0.domain.SecurityGroup group) {
-      SecurityGroupBuilder builder = new SecurityGroupBuilder();
-
-      builder.id(group.getId());
-      builder.providerId(group.getId());
-      builder.ownerId(group.getTenantId());
-      builder.name(group.getName());
-      if (group.getRules() != null) {
-         builder.ipPermissions(transform(group.getRules(), ruleToPermission));
-      }
-
-      return builder.build();
-   }
-}

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermission.java

@@ -1,96 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * 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.openstack.nova.v2_0.compute.functions;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.collect.Iterables.filter;
-import static com.google.common.collect.Iterables.getFirst;
-
-import java.util.Map;
-import java.util.concurrent.atomic.AtomicReference;
-
-import javax.annotation.Resource;
-import javax.inject.Inject;
-import javax.inject.Named;
-
-import org.jclouds.compute.reference.ComputeServiceConstants;
-import org.jclouds.domain.Location;
-import org.jclouds.logging.Logger;
-import org.jclouds.net.domain.IpPermission;
-import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule;
-import org.jclouds.openstack.nova.v2_0.domain.regionscoped.RegionAndName;
-import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion;
-
-import com.google.common.base.Function;
-import com.google.common.base.Predicate;
-import com.google.common.base.Supplier;
-import com.google.common.cache.LoadingCache;
-import com.google.common.util.concurrent.Atomics;
-
-/**
- * A function for transforming a nova-specific SecurityGroupRule into a generic
- * IpPermission object.
- */
-public class SecurityGroupRuleToIpPermission implements Function<SecurityGroupRule, IpPermission> {
-   @Resource
-   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
-   protected Logger logger = Logger.NULL;
-   protected final Predicate<AtomicReference<RegionAndName>> returnSecurityGroupExistsInRegion;
-   protected final Supplier<Map<String, Location>> locationIndex;
-   LoadingCache<RegionAndName, SecurityGroupInRegion> groupMap;
-
-   @Inject
-   public SecurityGroupRuleToIpPermission(@Named("SECURITYGROUP_PRESENT") Predicate<AtomicReference<RegionAndName>> returnSecurityGroupExistsInRegion,
-                                          Supplier<Map<String, Location>> locationIndex,
-                                          LoadingCache<RegionAndName, SecurityGroupInRegion> groupMap) {
-      this.returnSecurityGroupExistsInRegion = checkNotNull(returnSecurityGroupExistsInRegion,
-              "returnSecurityGroupExistsInRegion");
-      this.locationIndex = checkNotNull(locationIndex, "locationIndex");
-      this.groupMap = checkNotNull(groupMap, "groupMap");
-   }
-
-   @Override
-   public IpPermission apply(SecurityGroupRule rule) {
-      IpPermission.Builder builder = IpPermission.builder();
-      builder.ipProtocol(rule.getIpProtocol());
-      builder.fromPort(rule.getFromPort());
-      builder.toPort(rule.getToPort());
-      if (rule.getGroup() != null) {
-         String region = getFirst(filter(locationIndex.get().keySet(), isSecurityGroupInRegion(rule.getGroup().getName())),
-                 null);
-         if (region != null) {
-            SecurityGroupInRegion group = groupMap.getUnchecked(RegionAndName.fromRegionAndName(region, rule.getGroup().getName()));
-            builder.groupId(region + "/" + group.getSecurityGroup().getId());
-         }
-      }
-      if (rule.getIpRange() != null)
-         builder.cidrBlock(rule.getIpRange());
-
-      return builder.build();
-   }
-
-   protected Predicate<String> isSecurityGroupInRegion(final String groupName) {
-      return new Predicate<String>() {
-
-         @Override
-         public boolean apply(String region) {
-            AtomicReference<RegionAndName> securityGroupInRegionRef = Atomics.newReference(RegionAndName.fromRegionAndName(region, groupName));
-            return returnSecurityGroupExistsInRegion.apply(securityGroupInRegionRef);
-         }
-      };
-   }
-}

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/regionscoped/SecurityGroupInRegion.java

@@ -18,28 +18,58 @@ package org.jclouds.openstack.nova.v2_0.domain.regionscoped;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
+import java.util.Collection;
+import java.util.Map;
+
 import org.jclouds.openstack.nova.v2_0.domain.SecurityGroup;
+import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName;
 
 import com.google.common.base.Objects.ToStringHelper;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimap;
 
 public class SecurityGroupInRegion extends RegionAndName {
    protected final SecurityGroup securityGroup;
 
-   public SecurityGroupInRegion(SecurityGroup securityGroup, String regionId) {
+   protected final Multimap<TenantIdAndName, SecurityGroup> groupsByName;
+
+   public SecurityGroupInRegion(SecurityGroup securityGroup, String regionId, Iterable<SecurityGroup> allGroupsInRegion) {
       super(regionId, checkNotNull(securityGroup, "securityGroup").getName());
       this.securityGroup = securityGroup;
+      this.groupsByName = HashMultimap.create();
+      for (SecurityGroup groupInRegion : allGroupsInRegion) {
+         final TenantIdAndName tenantIdAndName = TenantIdAndName.builder()
+            .tenantId(groupInRegion.getTenantId())
+            .name(groupInRegion.getName())
+            .build();
+         this.groupsByName.put(tenantIdAndName, groupInRegion);
+      }
    }
 
    public SecurityGroup getSecurityGroup() {
       return securityGroup;
    }
 
+   /**
+    * Returns a map of group {@link TenantIdAndName}s to groups.
+    *
+    * The returned value is a collection, to take into account the possibility that certain clouds
+    * may permit duplicate group names.
+    *
+    * @return The map of names to (collections of) groups.
+    */
+   public Map<TenantIdAndName, Collection<SecurityGroup>> getGroupsByName() {
+       return groupsByName.asMap();
+   }
+
    // superclass hashCode/equals are good enough, and help us use RegionAndName and SecurityGroupInRegion
    // interchangeably as Map keys
 
    @Override
    protected ToStringHelper string() {
-      return super.string().add("securityGroup", securityGroup);
+      return super.string()
+          .add("securityGroup", securityGroup)
+          .add("groupsByName", groupsByName);
    }
 
    @Override

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/predicates/FindSecurityGroupWithNameAndReturnTrue.java

@@ -35,6 +35,7 @@ import org.jclouds.rest.ResourceNotFoundException;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.Iterables;
 import com.google.inject.Inject;
 
@@ -63,7 +64,8 @@ public class FindSecurityGroupWithNameAndReturnTrue implements Predicate<AtomicR
 
       logger.trace("looking for security group %s", securityGroupInRegion.slashEncode());
       try {
-         SecurityGroup returnVal = Iterables.find(api.get().list(), new Predicate<SecurityGroup>() {
+         final FluentIterable<SecurityGroup> allGroups = api.get().list();
+         SecurityGroup returnVal = Iterables.find(allGroups, new Predicate<SecurityGroup>() {
 
             @Override
             public boolean apply(SecurityGroup input) {
@@ -71,7 +73,7 @@ public class FindSecurityGroupWithNameAndReturnTrue implements Predicate<AtomicR
             }
 
          });
-         securityGroupInRegionRef.set(new SecurityGroupInRegion(returnVal, securityGroupInRegion.getRegion()));
+         securityGroupInRegionRef.set(new SecurityGroupInRegion(returnVal, securityGroupInRegion.getRegion(), allGroups));
          return true;
       } catch (ResourceNotFoundException e) {
          return false;

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtensionExpectTest.java

@@ -255,10 +255,23 @@ public class NovaSecurityGroupExtensionExpectTest extends BaseNovaComputeService
               payloadFromResource("/securitygroup_details_extension.json")).build();
 
 
-      SecurityGroupExtension extension = orderedRequestsSendResponses(ImmutableList.of(keystoneAuthWithUsernameAndPasswordAndTenantName,
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
-              extensionsOfNovaRequest, getSecurityGroup, createRule, getSecurityGroup, list, list),
+         ImmutableList.of(
-              ImmutableList.of(responseWithKeystoneAccess, extensionsOfNovaResponse, getSecurityGroupNoRulesResponse,
+            keystoneAuthWithUsernameAndPasswordAndTenantName,
-                      createRuleResponse, getSecurityGroupResponse, listResponse, listResponse)).getSecurityGroupExtension().get();
+            extensionsOfNovaRequest,
+            list,
+            getSecurityGroup,
+            createRule,
+            list,
+            getSecurityGroup),
+         ImmutableList.of(
+            responseWithKeystoneAccess,
+            extensionsOfNovaResponse,
+            listResponse,
+            getSecurityGroupNoRulesResponse,
+            createRuleResponse,
+            listResponse,
+            getSecurityGroupResponse)).getSecurityGroupExtension().get();
 
       IpPermission.Builder builder = IpPermission.builder();
 
@@ -304,10 +317,24 @@ public class NovaSecurityGroupExtensionExpectTest extends BaseNovaComputeService
               payloadFromResource("/securitygroup_details_extension.json")).build();
 
 
-      SecurityGroupExtension extension = orderedRequestsSendResponses(ImmutableList.of(keystoneAuthWithUsernameAndPasswordAndTenantName,
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
-              extensionsOfNovaRequest, getSecurityGroup, createRule, getSecurityGroup, list, list),
+         ImmutableList.of(
-              ImmutableList.of(responseWithKeystoneAccess, extensionsOfNovaResponse, getSecurityGroupNoRulesResponse,
+            keystoneAuthWithUsernameAndPasswordAndTenantName,
-                      createRuleResponse, getSecurityGroupResponse, listResponse, listResponse)).getSecurityGroupExtension().get();
+            extensionsOfNovaRequest,
+            list,
+            getSecurityGroup,
+            createRule,
+            list,
+            getSecurityGroup),
+         ImmutableList.of(
+            responseWithKeystoneAccess,
+            extensionsOfNovaResponse,
+            listResponse,
+            getSecurityGroupNoRulesResponse,
+            createRuleResponse,
+            listResponse,
+            getSecurityGroupResponse))
+         .getSecurityGroupExtension().get();
 
       SecurityGroup origGroup = extension.getSecurityGroupById(region + "/160");
 
@@ -349,11 +376,23 @@ public class NovaSecurityGroupExtensionExpectTest extends BaseNovaComputeService
       HttpResponse getSecurityGroupResponse = HttpResponse.builder().statusCode(200).payload(
               payloadFromResource("/securitygroup_details_extension.json")).build();
 
-
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
-      SecurityGroupExtension extension = orderedRequestsSendResponses(ImmutableList.of(keystoneAuthWithUsernameAndPasswordAndTenantName,
+         ImmutableList.of(
-              extensionsOfNovaRequest, getSecurityGroup, createRule, getSecurityGroup, list, list),
+            keystoneAuthWithUsernameAndPasswordAndTenantName,
-              ImmutableList.of(responseWithKeystoneAccess, extensionsOfNovaResponse, getSecurityGroupNoRulesResponse,
+            extensionsOfNovaRequest,
-                      createRuleResponse, getSecurityGroupResponse, listResponse, listResponse)).getSecurityGroupExtension().get();
+            list,
+            getSecurityGroup,
+            createRule,
+            list,
+            getSecurityGroup),
+         ImmutableList.of(
+            responseWithKeystoneAccess,
+            extensionsOfNovaResponse,
+            listResponse,
+            getSecurityGroupNoRulesResponse,
+            createRuleResponse,
+            listResponse,
+            getSecurityGroupResponse)).getSecurityGroupExtension().get();
 
       IpPermission.Builder builder = IpPermission.builder();
 
@@ -399,10 +438,24 @@ public class NovaSecurityGroupExtensionExpectTest extends BaseNovaComputeService
               payloadFromResource("/securitygroup_details_extension.json")).build();
 
 
-      SecurityGroupExtension extension = orderedRequestsSendResponses(ImmutableList.of(keystoneAuthWithUsernameAndPasswordAndTenantName,
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
-              extensionsOfNovaRequest, getSecurityGroup, createRule, getSecurityGroup, list, list),
+         ImmutableList.of(
-              ImmutableList.of(responseWithKeystoneAccess, extensionsOfNovaResponse, getSecurityGroupNoRulesResponse,
+            keystoneAuthWithUsernameAndPasswordAndTenantName,
-                      createRuleResponse, getSecurityGroupResponse, listResponse, listResponse)).getSecurityGroupExtension().get();
+            extensionsOfNovaRequest,
+            list,
+            getSecurityGroup,
+            createRule,
+            list,
+            getSecurityGroup),
+         ImmutableList.of(
+            responseWithKeystoneAccess,
+            extensionsOfNovaResponse,
+            listResponse,
+            getSecurityGroupNoRulesResponse,
+            createRuleResponse,
+            listResponse,
+            getSecurityGroupResponse))
+         .getSecurityGroupExtension().get();
 
       SecurityGroup origGroup = extension.getSecurityGroupById(region + "/160");
 

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtensionLiveTest.java

@@ -16,17 +16,103 @@
  */
 package org.jclouds.openstack.nova.v2_0.compute.extensions;
 
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+
+import java.util.Date;
+import java.util.Random;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import javax.annotation.Resource;
+import javax.inject.Named;
+
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.SecurityGroup;
+import org.jclouds.compute.extensions.SecurityGroupExtension;
 import org.jclouds.compute.extensions.internal.BaseSecurityGroupExtensionLiveTest;
+import org.jclouds.compute.reference.ComputeServiceConstants;
+import org.jclouds.logging.Logger;
 import org.testng.annotations.Test;
 
+import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
+
 /**
  * Live test for openstack-nova {@link org.jclouds.compute.extensions.SecurityGroupExtension} implementation.
  */
 @Test(groups = "live", singleThreaded = true, testName = "NovaSecurityGroupExtensionLiveTest")
 public class NovaSecurityGroupExtensionLiveTest extends BaseSecurityGroupExtensionLiveTest {
 
+
+   @Resource
+   @Named(ComputeServiceConstants.COMPUTE_LOGGER)
+   protected Logger logger = Logger.CONSOLE;
+
    public NovaSecurityGroupExtensionLiveTest() {
       provider = "openstack-nova";
    }
 
+   @Test(groups = {"integration", "live"}, singleThreaded = true)
+   public void testListSecurityGroups() throws Exception {
+      skipIfSecurityGroupsNotSupported();
+
+      final long begin = new Date().getTime();
+      ComputeService computeService = view.getComputeService();
+      Optional<SecurityGroupExtension> securityGroupExtension = computeService.getSecurityGroupExtension();
+      assertTrue(securityGroupExtension.isPresent(), "security extension was not present");
+
+      logger.info("Loading security groups");
+      final SecurityGroupExtension security = securityGroupExtension.get();
+      Set<SecurityGroup> beforeAdd = security.listSecurityGroups();
+      int countBeforeAdd = beforeAdd.size();
+      logger.info("Found %d security groups", countBeforeAdd);
+
+      String someUnlikelyName = String.valueOf(new Random().nextInt(1000000) + 1000000);
+      logger.info("Adding security group %s", someUnlikelyName);
+      final SecurityGroup testGroup = security.createSecurityGroup(someUnlikelyName, getNodeTemplate().getLocation());
+
+      try {
+         verifyAndDeleteSecurityGroup(security, countBeforeAdd, testGroup);
+      } catch (Exception e) {
+         logger.error(e, "Exception caught, live test leaking security group %s", testGroup.getName());
+         throw e;
+      }
+
+      final long end = new Date().getTime();
+
+      assertTrue(end - begin < TimeUnit.MINUTES.toMillis(5)); // see https://issues.apache.org/jira/browse/JCLOUDS-1235
+
+   }
+
+   private void verifyAndDeleteSecurityGroup(SecurityGroupExtension security, int countBeforeAdd,
+                                             final SecurityGroup testGroup) {
+      logger.info("Loading security groups");
+      Set<SecurityGroup> afterAdd = security.listSecurityGroups();
+      final int countAfterAdd = afterAdd.size();
+      logger.info("Found %d security groups", countAfterAdd);
+
+      assertEquals(countAfterAdd, countBeforeAdd + 1);
+      final Predicate<SecurityGroup> findTestGroup = new Predicate<SecurityGroup>() {
+         @Override
+         public boolean apply(SecurityGroup input) {
+            return input.getName().equals(testGroup.getName());
+         }
+      };
+      final SecurityGroup created = Iterables.find(afterAdd, findTestGroup);
+      assertNotNull(created, "Did not find security group created as expected");
+
+      logger.info("Removing %s", testGroup.getName());
+      security.removeSecurityGroup(testGroup.getId());
+
+      logger.info("Loading security groups");
+      Set<SecurityGroup> afterRemove = security.listSecurityGroups();
+      final int sizeAfterRemove = afterRemove.size();
+      logger.info("Found %d security groups", sizeAfterRemove);
+      assertEquals(sizeAfterRemove, countBeforeAdd);
+      final Optional<SecurityGroup> removed = Iterables.tryFind(afterRemove, findTestGroup);
+      assertTrue(!removed.isPresent(), "Did not remove test security group as expected");
+   }
 }

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroupTest.java

@@ -16,10 +16,8 @@
  */
 package org.jclouds.openstack.nova.v2_0.compute.functions;
 
-import static com.google.common.collect.Iterables.transform;
-import static org.jclouds.openstack.nova.v2_0.compute.functions.NovaSecurityGroupToSecurityGroupTest.securityGroupWithCidr;
-import static org.jclouds.openstack.nova.v2_0.compute.functions.NovaSecurityGroupToSecurityGroupTest.securityGroupWithGroup;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
 
 import java.util.Map;
 
@@ -27,13 +25,19 @@ import org.jclouds.compute.domain.SecurityGroup;
 import org.jclouds.domain.Location;
 import org.jclouds.domain.LocationBuilder;
 import org.jclouds.domain.LocationScope;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule;
+import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName;
 import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+
 
 @Test(groups = "unit", testName = "NovaSecurityGroupInRegionToSecurityGroupTest")
 public class NovaSecurityGroupInRegionToSecurityGroupTest {
@@ -45,12 +49,67 @@ public class NovaSecurityGroupInRegionToSecurityGroupTest {
    Supplier<Map<String, Location>> locationIndex = Suppliers.<Map<String, Location>> ofInstance(ImmutableMap
            .<String, Location>of("az-1.region-a.geo-1", region));
 
+   public static final String SOME_GROUP_ID = "some-group-id";
+   public static final String SOME_OTHER_GROUP_ID = "some-other-group-id";
+   public static final String IP_RANGE = "0.0.0.0/0";
+   public static final String SOME_OTHER_GROUP = "some-other-group";
+   public static final String SOME_GROUP = "some-group";
+
+   public static final ImmutableList<org.jclouds.openstack.nova.v2_0.domain.SecurityGroup> allGroups =
+      ImmutableList.of(securityGroupWithGroup(), securityGroupWithCidr());
+
+   public static org.jclouds.openstack.nova.v2_0.domain.SecurityGroup securityGroupWithGroup() {
+      TenantIdAndName group = TenantIdAndName.builder().tenantId("tenant").name(SOME_OTHER_GROUP).build();
+
+      SecurityGroupRule ruleToConvert = SecurityGroupRule.builder()
+         .id("some-rule-id")
+         .ipProtocol(IpProtocol.TCP)
+         .fromPort(10)
+         .toPort(20)
+         .group(group)
+         .parentGroupId(SOME_GROUP_ID)
+         .build();
+
+      org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup =
+         org.jclouds.openstack.nova.v2_0.domain.SecurityGroup.builder()
+            .tenantId("tenant")
+            .id(SOME_GROUP_ID)
+            .name(SOME_GROUP)
+            .description("some-description")
+            .rules(ruleToConvert)
+            .build();
+
+      return origGroup;
+   }
+
+   public static org.jclouds.openstack.nova.v2_0.domain.SecurityGroup securityGroupWithCidr() {
+      SecurityGroupRule ruleToConvert = SecurityGroupRule.builder()
+         .id("some-other-rule-id")
+         .ipProtocol(IpProtocol.TCP)
+         .fromPort(10)
+         .toPort(20)
+         .ipRange(IP_RANGE)
+         .parentGroupId(SOME_OTHER_GROUP_ID)
+         .build();
+
+      org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup =
+         org.jclouds.openstack.nova.v2_0.domain.SecurityGroup.builder()
+            .tenantId("tenant")
+            .id(SOME_OTHER_GROUP_ID)
+            .name(SOME_OTHER_GROUP)
+            .description("some-description")
+            .rules(ruleToConvert)
+            .build();
+
+      return origGroup;
+   }
 
    @Test
    public void testApplyWithGroup() {
       NovaSecurityGroupInRegionToSecurityGroup parser = createGroupParser();
 
-      SecurityGroupInRegion origGroup = new SecurityGroupInRegion(securityGroupWithGroup(), region.getId());
+      final org.jclouds.openstack.nova.v2_0.domain.SecurityGroup otherGroup = securityGroupWithCidr();
+      SecurityGroupInRegion origGroup = new SecurityGroupInRegion(securityGroupWithGroup(), region.getId(), allGroups);
 
       SecurityGroup newGroup = parser.apply(origGroup);
 
@@ -58,8 +117,11 @@ public class NovaSecurityGroupInRegionToSecurityGroupTest {
       assertEquals(newGroup.getProviderId(), origGroup.getSecurityGroup().getId());
       assertEquals(newGroup.getName(), origGroup.getSecurityGroup().getName());
       assertEquals(newGroup.getOwnerId(), origGroup.getSecurityGroup().getTenantId());
-      assertEquals(newGroup.getIpPermissions(), ImmutableSet.copyOf(transform(origGroup.getSecurityGroup().getRules(),
+      final IpPermission permission = Iterables.getOnlyElement(newGroup.getIpPermissions());
-              NovaSecurityGroupToSecurityGroupTest.ruleConverter)));
+      assertEquals(Iterables.getOnlyElement(permission.getGroupIds()), region.getId() + "/" + otherGroup.getId());
+      assertEquals(permission.getFromPort(), 10);
+      assertEquals(permission.getToPort(), 20);
+      assertTrue(permission.getCidrBlocks().isEmpty());
       assertEquals(newGroup.getLocation().getId(), origGroup.getRegion());
    }
 
@@ -68,7 +130,7 @@ public class NovaSecurityGroupInRegionToSecurityGroupTest {
 
       NovaSecurityGroupInRegionToSecurityGroup parser = createGroupParser();
 
-      SecurityGroupInRegion origGroup = new SecurityGroupInRegion(securityGroupWithCidr(), region.getId());
+      SecurityGroupInRegion origGroup = new SecurityGroupInRegion(securityGroupWithCidr(), region.getId(), allGroups);
 
       SecurityGroup newGroup = parser.apply(origGroup);
 
@@ -76,15 +138,17 @@ public class NovaSecurityGroupInRegionToSecurityGroupTest {
       assertEquals(newGroup.getProviderId(), origGroup.getSecurityGroup().getId());
       assertEquals(newGroup.getName(), origGroup.getSecurityGroup().getName());
       assertEquals(newGroup.getOwnerId(), origGroup.getSecurityGroup().getTenantId());
-      assertEquals(newGroup.getIpPermissions(), ImmutableSet.copyOf(transform(origGroup.getSecurityGroup().getRules(),
+      final IpPermission permission = Iterables.getOnlyElement(newGroup.getIpPermissions());
-              NovaSecurityGroupToSecurityGroupTest.ruleConverter)));
+      assertEquals(permission.getFromPort(), 10);
+      assertEquals(permission.getToPort(), 20);
+      assertEquals(Iterables.getOnlyElement(permission.getCidrBlocks()), IP_RANGE);
+      assertTrue(permission.getGroupIds().isEmpty());
       assertEquals(newGroup.getLocation().getId(), origGroup.getRegion());
    }
 
    private NovaSecurityGroupInRegionToSecurityGroup createGroupParser() {
-      NovaSecurityGroupToSecurityGroup baseParser = new NovaSecurityGroupToSecurityGroup(NovaSecurityGroupToSecurityGroupTest.ruleConverter);
 
-      NovaSecurityGroupInRegionToSecurityGroup parser = new NovaSecurityGroupInRegionToSecurityGroup(baseParser, locationIndex);
+      NovaSecurityGroupInRegionToSecurityGroup parser = new NovaSecurityGroupInRegionToSecurityGroup(locationIndex);
 
       return parser;
    }

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroupTest.java

@@ -1,152 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * 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.openstack.nova.v2_0.compute.functions;
-
-import static com.google.common.collect.Iterables.transform;
-import static org.testng.Assert.assertEquals;
-
-import java.util.Map;
-import java.util.concurrent.atomic.AtomicReference;
-
-import org.jclouds.compute.domain.SecurityGroup;
-import org.jclouds.domain.Location;
-import org.jclouds.domain.LocationBuilder;
-import org.jclouds.domain.LocationScope;
-import org.jclouds.net.domain.IpProtocol;
-import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule;
-import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName;
-import org.jclouds.openstack.nova.v2_0.domain.regionscoped.RegionAndName;
-import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion;
-import org.testng.annotations.Test;
-
-import com.google.common.base.Functions;
-import com.google.common.base.Predicate;
-import com.google.common.base.Predicates;
-import com.google.common.base.Supplier;
-import com.google.common.base.Suppliers;
-import com.google.common.cache.CacheBuilder;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-
-@Test(groups = "unit", testName = "NovaSecurityGroupToSecurityGroupTest")
-public class NovaSecurityGroupToSecurityGroupTest {
-
-   private static final Location provider = new LocationBuilder().scope(LocationScope.PROVIDER).id("openstack-nova")
-           .description("openstack-nova").build();
-   private static final Location region = new LocationBuilder().id("az-1.region-a.geo-1").description("az-1.region-a.geo-1")
-           .scope(LocationScope.REGION).parent(provider).build();
-   private static final Supplier<Map<String, Location>> locationIndex = Suppliers.<Map<String, Location>> ofInstance(ImmutableMap
-           .<String, Location>of("az-1.region-a.geo-1", region));
-
-
-   private static final Predicate<AtomicReference<RegionAndName>> returnSecurityGroupExistsInRegion = Predicates.alwaysTrue();
-
-   private static final Map<RegionAndName, SecurityGroupInRegion> groupMap = ImmutableMap.of(
-           RegionAndName.fromRegionAndName("az-1.region-a.geo-1", "some-group"), new SecurityGroupInRegion(securityGroupWithGroup(), "az-1.region-a.geo-1"),
-           RegionAndName.fromRegionAndName("az-1.region-a.geo-1", "some-other-group"), new SecurityGroupInRegion(securityGroupWithCidr(), "az-1.region-a.geo-1"));
-
-   // weird compilation error means have to declare extra generics for call to build() - see https://bugs.eclipse.org/bugs/show_bug.cgi?id=365818
-   private static final Supplier <LoadingCache<RegionAndName, SecurityGroupInRegion>> groupCache = Suppliers.<LoadingCache<RegionAndName, SecurityGroupInRegion>> ofInstance(
-           CacheBuilder.newBuilder().<RegionAndName, SecurityGroupInRegion>build(CacheLoader.from(Functions.forMap(groupMap))));
-
-   public static final SecurityGroupRuleToIpPermission ruleConverter = new SecurityGroupRuleToIpPermission(returnSecurityGroupExistsInRegion, locationIndex,
-           groupCache.get());
-
-   public static org.jclouds.openstack.nova.v2_0.domain.SecurityGroup securityGroupWithGroup() {
-      TenantIdAndName group = TenantIdAndName.builder().tenantId("tenant").name("some-other-group").build();
-
-      SecurityGroupRule ruleToConvert = SecurityGroupRule.builder()
-              .id("some-id")
-              .ipProtocol(IpProtocol.TCP)
-              .fromPort(10)
-              .toPort(20)
-              .group(group)
-              .parentGroupId("some-other-id")
-              .build();
-
-      org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = org.jclouds.openstack.nova.v2_0.domain.SecurityGroup.builder()
-              .tenantId("tenant")
-              .id("some-id")
-              .name("some-group")
-              .description("some-description")
-              .rules(ruleToConvert)
-              .build();
-
-      return origGroup;
-   }
-
-   public static org.jclouds.openstack.nova.v2_0.domain.SecurityGroup securityGroupWithCidr() {
-      SecurityGroupRule ruleToConvert = SecurityGroupRule.builder()
-              .id("some-id")
-              .ipProtocol(IpProtocol.TCP)
-              .fromPort(10)
-              .toPort(20)
-              .ipRange("0.0.0.0/0")
-              .parentGroupId("some-other-id")
-              .build();
-
-      org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = org.jclouds.openstack.nova.v2_0.domain.SecurityGroup.builder()
-              .tenantId("tenant")
-              .id("some-id")
-              .name("some-other-group")
-              .description("some-description")
-              .rules(ruleToConvert)
-              .build();
-
-      return origGroup;
-   }
-
-   @Test
-   public void testApplyWithGroup() {
-      NovaSecurityGroupToSecurityGroup parser = createGroupParser();
-
-      org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = securityGroupWithGroup();
-
-      SecurityGroup newGroup = parser.apply(origGroup);
-
-      assertEquals(newGroup.getId(), origGroup.getId());
-      assertEquals(newGroup.getProviderId(), origGroup.getId());
-      assertEquals(newGroup.getName(), origGroup.getName());
-      assertEquals(newGroup.getOwnerId(), origGroup.getTenantId());
-      assertEquals(newGroup.getIpPermissions(), ImmutableSet.copyOf(transform(origGroup.getRules(), ruleConverter)));
-   }
-
-   @Test
-   public void testApplyWithCidr() {
-
-      NovaSecurityGroupToSecurityGroup parser = createGroupParser();
-
-      org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = securityGroupWithCidr();
-
-      SecurityGroup group = parser.apply(origGroup);
-
-      assertEquals(group.getId(), origGroup.getId());
-      assertEquals(group.getProviderId(), origGroup.getId());
-      assertEquals(group.getName(), origGroup.getName());
-      assertEquals(group.getOwnerId(), origGroup.getTenantId());
-      assertEquals(group.getIpPermissions(), ImmutableSet.copyOf(transform(origGroup.getRules(), ruleConverter)));
-   }
-
-   private NovaSecurityGroupToSecurityGroup createGroupParser() {
-      NovaSecurityGroupToSecurityGroup parser = new NovaSecurityGroupToSecurityGroup(ruleConverter);
-
-      return parser;
-   }
-
-}

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermissionTest.java

@@ -1,79 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * 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.openstack.nova.v2_0.compute.functions;
-
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertTrue;
-
-import org.jclouds.net.domain.IpPermission;
-import org.jclouds.net.domain.IpProtocol;
-import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule;
-import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName;
-import org.testng.annotations.Test;
-
-import com.google.common.collect.ImmutableSet;
-
-
-/**
- * Tests for the function for transforming a nova specific SecurityGroupRule into a generic
- * IpPermission object.
- */
-public class SecurityGroupRuleToIpPermissionTest {
-
-   @Test
-   public void testApplyWithGroup() {
-
-      TenantIdAndName group = TenantIdAndName.builder().tenantId("tenant").name("some-group").build();
-      
-      SecurityGroupRule ruleToConvert = SecurityGroupRule.builder()
-         .id("some-id")
-         .ipProtocol(IpProtocol.TCP)
-         .fromPort(10)
-         .toPort(20)
-         .group(group)
-         .parentGroupId("some-other-id")
-         .build();
-
-      IpPermission convertedPerm = NovaSecurityGroupToSecurityGroupTest.ruleConverter.apply(ruleToConvert);
-
-      assertEquals(convertedPerm.getIpProtocol(), ruleToConvert.getIpProtocol());
-      assertEquals(convertedPerm.getFromPort(), ruleToConvert.getFromPort());
-      assertEquals(convertedPerm.getToPort(), ruleToConvert.getToPort());
-      assertTrue(convertedPerm.getGroupIds().contains("az-1.region-a.geo-1/some-id"));
-      assertEquals(convertedPerm.getCidrBlocks().size(), 0);
-   }
-
-   @Test
-   public void testApplyWithCidr() {
-      SecurityGroupRule ruleToConvert = SecurityGroupRule.builder()
-         .id("some-id")
-         .ipProtocol(IpProtocol.TCP)
-         .fromPort(10)
-         .toPort(20)
-         .ipRange("0.0.0.0/0")
-         .parentGroupId("some-other-id")
-         .build();
-
-      IpPermission convertedPerm = NovaSecurityGroupToSecurityGroupTest.ruleConverter.apply(ruleToConvert);
-
-      assertEquals(convertedPerm.getIpProtocol(), ruleToConvert.getIpProtocol());
-      assertEquals(convertedPerm.getFromPort(), ruleToConvert.getFromPort());
-      assertEquals(convertedPerm.getToPort(), ruleToConvert.getToPort());
-      assertEquals(convertedPerm.getCidrBlocks(), ImmutableSet.of("0.0.0.0/0"));
-      assertEquals(convertedPerm.getTenantIdGroupNamePairs().size(), 0);
-   }
-}

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/CreateSecurityGroupIfNeededTest.java

@@ -24,6 +24,7 @@ import org.jclouds.http.HttpRequest;
 import org.jclouds.http.HttpResponse;
 import org.jclouds.openstack.nova.v2_0.NovaApi;
 import org.jclouds.openstack.nova.v2_0.compute.functions.CreateSecurityGroupIfNeeded;
+import org.jclouds.openstack.nova.v2_0.domain.SecurityGroup;
 import org.jclouds.openstack.nova.v2_0.domain.regionscoped.RegionSecurityGroupNameAndPorts;
 import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion;
 import org.jclouds.openstack.nova.v2_0.internal.BaseNovaApiExpectTest;
@@ -47,13 +48,14 @@ public class CreateSecurityGroupIfNeededTest extends BaseNovaApiExpectTest {
                               "{\"security_group\":{\"name\":\"jclouds_mygroup\",\"description\":\"jclouds_mygroup\"}}",
                               "application/json")).build();
 
+   private final int groupId = 2769;
+
    public void testCreateNewGroup() throws Exception {
 
       Builder<HttpRequest, HttpResponse> builder = ImmutableMap.builder();
 
       builder.put(keystoneAuthWithUsernameAndPasswordAndTenantName, responseWithKeystoneAccess);
       builder.put(extensionsOfNovaRequest, extensionsOfNovaResponse);
-      int groupId = 2769;
 
       HttpResponse createResponse = HttpResponse.builder().statusCode(200)
                .payload(
@@ -113,15 +115,24 @@ public class CreateSecurityGroupIfNeededTest extends BaseNovaApiExpectTest {
 
       builder.put(getSecurityGroup, getSecurityGroupResponse);
 
+
+      HttpRequest listSecurityGroups = HttpRequest.builder().method("GET").endpoint(
+         URI.create("https://az-1.region-a.geo-1.compute.hpcloudsvc.com/v2/3456/os-security-groups")).headers(
+         ImmutableMultimap.<String, String> builder().put("Accept", "application/json").put("X-Auth-Token",
+            authToken).build()).build();
+      HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200).payload(
+         payloadFromResource("/securitygroup_list_details_computeservice_typical.json")).build();
+      builder.put(listSecurityGroups, listSecurityGroupsResponse);
+
       NovaApi apiCanCreateSecurityGroup = requestsSendResponses(builder.build());
 
       CreateSecurityGroupIfNeeded fn = new CreateSecurityGroupIfNeeded(apiCanCreateSecurityGroup);
 
       // we can find it
-      assertEquals(fn.apply(
+      final SecurityGroup expected = new ParseComputeServiceTypicalSecurityGroupTest().expected();
-               new RegionSecurityGroupNameAndPorts("az-1.region-a.geo-1", "jclouds_mygroup", ImmutableSet.of(22, 8080)))
+      assertEquals(
-               .toString(), new SecurityGroupInRegion(new ParseComputeServiceTypicalSecurityGroupTest().expected(),
+         fn.apply(new RegionSecurityGroupNameAndPorts("az-1.region-a.geo-1", "jclouds_mygroup", ImmutableSet.of(22, 8080))).toString(),
-               "az-1.region-a.geo-1").toString());
+         new SecurityGroupInRegion(expected, "az-1.region-a.geo-1", ImmutableList.of(expected)).toString());
 
    }
 
@@ -155,10 +166,9 @@ public class CreateSecurityGroupIfNeededTest extends BaseNovaApiExpectTest {
       CreateSecurityGroupIfNeeded fn = new CreateSecurityGroupIfNeeded(apiWhenSecurityGroupsExist);
 
       // we can find it
-      assertEquals(fn.apply(
+      final SecurityGroup expected = new ParseComputeServiceTypicalSecurityGroupTest().expected();
-               new RegionSecurityGroupNameAndPorts("az-1.region-a.geo-1", "jclouds_mygroup", ImmutableSet.of(22, 8080)))
+      assertEquals(
-               .toString(), new SecurityGroupInRegion(new ParseComputeServiceTypicalSecurityGroupTest().expected(),
+         fn.apply(new RegionSecurityGroupNameAndPorts("az-1.region-a.geo-1", "jclouds_mygroup", ImmutableSet.of(22, 8080))).toString(),
-               "az-1.region-a.geo-1").toString());
+         new SecurityGroupInRegion(expected, "az-1.region-a.geo-1", ImmutableList.of(expected)).toString());
-
    }
 }

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/FindSecurityGroupWithNameAndReturnTrueExpectTest.java

@@ -21,11 +21,13 @@ import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
 
 import java.net.URI;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.jclouds.http.HttpRequest;
 import org.jclouds.http.HttpResponse;
 import org.jclouds.openstack.nova.v2_0.NovaApi;
+import org.jclouds.openstack.nova.v2_0.domain.SecurityGroup;
 import org.jclouds.openstack.nova.v2_0.domain.regionscoped.RegionAndName;
 import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion;
 import org.jclouds.openstack.nova.v2_0.internal.BaseNovaApiExpectTest;
@@ -63,9 +65,10 @@ public class FindSecurityGroupWithNameAndReturnTrueExpectTest extends BaseNovaAp
       assertTrue(predicate.apply(securityGroupInRegionRef));
 
       // the reference is now up to date, and includes the actual group found.
-      assertEquals(securityGroupInRegionRef.get().toString(), new SecurityGroupInRegion(Iterables
+      final Set<SecurityGroup> expected = new ParseSecurityGroupListTest().expected();
-               .getOnlyElement(new ParseSecurityGroupListTest().expected()), "az-1.region-a.geo-1").toString());
+      assertEquals(
-
+         securityGroupInRegionRef.get().toString(),
+         new SecurityGroupInRegion(Iterables.getOnlyElement(expected), "az-1.region-a.geo-1", expected).toString());
    }
 
    public void testDoesNotUpdateReferenceWhenSecurityGroupListMissingGroupName() throws Exception {

core/src/test/java/org/jclouds/rest/internal/BaseRestApiExpectTest.java

@@ -337,7 +337,8 @@ public abstract class BaseRestApiExpectTest<S> {
                         String.format("request %s is out of range (%s)", index, requests.size())).payload(
                         Payloads.newStringPayload(renderRequest(input))).build();
             if (!httpRequestsAreEqual(input, requests.get(index))) {
-               assertEquals(renderRequest(input), renderRequest(requests.get(index)));
+               assertEquals(renderRequest(input), renderRequest(requests.get(index)),
+                  "Actual request did not match expected request " + index);
             }
             return responses.get(index);
          }