should that be a new patch?
On Wed, Jul 3, 2013 at 4:29 PM, Daan Hoogland <daan.hoogl...@gmail.com>wrote: > ai, > > will fix > > > On Wed, Jul 3, 2013 at 3:49 PM, David Nalley <da...@gnsa.us> wrote: > >> License header missing in the test >> >> --David >> >> On Wed, Jul 3, 2013 at 9:35 AM, <chipchild...@apache.org> wrote: >> > Updated Branches: >> > refs/heads/master 90a15bfff -> 141fbc7ef >> > >> > >> > cidr compare fixme addressed >> > >> > >> > Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo >> > Commit: >> http://git-wip-us.apache.org/repos/asf/cloudstack/commit/141fbc7e >> > Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/141fbc7e >> > Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/141fbc7e >> > >> > Branch: refs/heads/master >> > Commit: 141fbc7ef7a89ee1e046366f1e2e6dffbe4e5be7 >> > Parents: 90a15bf >> > Author: Daan Hoogland <d...@onecht.net> >> > Authored: Wed Jul 3 14:03:30 2013 +0200 >> > Committer: Chip Childers <chipchild...@apache.org> >> > Committed: Wed Jul 3 09:28:49 2013 -0400 >> > >> > ---------------------------------------------------------------------- >> > .../security/SecurityGroupManagerImpl.java | 13 +++- >> > .../security/SecurityGroupManagerImplTest.java | 62 >> ++++++++++++++++++++ >> > 2 files changed, 74 insertions(+), 1 deletion(-) >> > ---------------------------------------------------------------------- >> > >> > >> > >> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/141fbc7e/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java >> > ---------------------------------------------------------------------- >> > diff --git >> a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java >> b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java >> > index 1c189c4..ce6f8ac 100755 >> > --- >> a/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java >> > +++ >> b/server/src/com/cloud/network/security/SecurityGroupManagerImpl.java >> > @@ -322,7 +322,18 @@ public class SecurityGroupManagerImpl extends >> ManagerBase implements SecurityGro >> > >> > @Override >> > public int compare(String cidr1, String cidr2) { >> > - return cidr1.compareTo(cidr2); // FIXME >> > + // parse both to find significance first (low number of >> bits is high) >> > + // if equal then just do a string compare >> > + if(significance(cidr1) == significance(cidr2)) { >> > + return cidr1.compareTo(cidr2); >> > + } >> > + else { >> > + return significance(cidr2) - significance(cidr1); >> > + } >> > + } >> > + private int significance(String cidr) >> > + { >> > + return >> Integer.parseInt(cidr.substring(cidr.indexOf('/')+1)); >> > } >> > >> > } >> > >> > >> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/141fbc7e/server/test/com/cloud/network/security/SecurityGroupManagerImplTest.java >> > ---------------------------------------------------------------------- >> > diff --git >> a/server/test/com/cloud/network/security/SecurityGroupManagerImplTest.java >> b/server/test/com/cloud/network/security/SecurityGroupManagerImplTest.java >> > new file mode 100644 >> > index 0000000..8a6b95b >> > --- /dev/null >> > +++ >> b/server/test/com/cloud/network/security/SecurityGroupManagerImplTest.java >> > @@ -0,0 +1,62 @@ >> > +/** >> > + * >> > + */ >> > +package com.cloud.network.security; >> > + >> > +import java.util.Set; >> > +import java.util.TreeSet; >> > + >> > +import javax.inject.Inject; >> > + >> > +import junit.framework.TestCase; >> > + >> > +import org.junit.Before; >> > +import org.junit.Test; >> > +import org.junit.runner.RunWith; >> > +import org.springframework.test.context.ContextConfiguration; >> > +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; >> > + >> > +import >> com.cloud.network.security.SecurityGroupManagerImpl.CidrComparator; >> > + >> > +/** >> > + * @author daan >> > + * >> > + */ >> > +@RunWith(SpringJUnit4ClassRunner.class) >> > +@ContextConfiguration(locations = >> "classpath:/SecurityGroupManagerTestContext.xml") >> > +public class SecurityGroupManagerImplTest extends TestCase { >> > + @Inject >> > + SecurityGroupManagerImpl2 _sgMgr = null; >> > + Set<String> cidrs; >> > + @Before >> > + public void setup() throws Exception { >> > + cidrs = new TreeSet<String>(new CidrComparator()); >> > + } >> > + @Test (expected=NumberFormatException.class) >> > + public void emptyCidrCompareTest() { >> > + cidrs.add(""); >> > + cidrs.add(""); >> > + } >> > + @Test (expected=NumberFormatException.class) >> > + public void faultyCidrCompareTest() { >> > + cidrs.add("111.222.333.444"); >> > + cidrs.add("111.222.333.444"); >> > + } >> > + @Test >> > + public void sameCidrCompareTest() { >> > + cidrs.add("1.2.3.4/5"); >> > + cidrs.add("1.2.3.4/5"); >> > + assertEquals("only one element expected",1,cidrs.size()); >> > + CidrComparator cmp = new CidrComparator(); >> > + assertEquals("should be 0",0,cmp.compare("1.2.3.4/5"," >> 1.2.3.4/5")); >> > + } >> > + @Test >> > + public void CidrCompareTest() { >> > + cidrs.add("1.2.3.4/5"); >> > + cidrs.add("1.2.3.4/6"); >> > + assertEquals("two element expected",2,cidrs.size()); >> > + CidrComparator cmp = new CidrComparator(); >> > + assertEquals("should be 1",1,cmp.compare("1.2.3.4/5"," >> 1.2.3.4/6")); >> > + assertEquals("should be -2",-2,cmp.compare("1.2.3.4/5"," >> 1.2.3.4/3")); >> > + } >> > +} >> > >> > >
