From: Michal Rehak Date: Fri, 7 Feb 2014 15:37:35 +0000 (+0100) Subject: BUG 408 X-Git-Tag: autorelease-tag-v20140601202136_82eb3f9~509^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=2a32748fe64ddeeec7f0a3973d709e822a7df7cf BUG 408 - added null safe comparison - added unit test - added SLF4j - added log message in method StrIpToIntIp in case of exception Change-Id: Id15dc4f80d419df5e73ab79fea4e8ab25ec82968 Signed-off-by: Michal Rehak --- diff --git a/opendaylight/md-sal/statistics-manager/pom.xml b/opendaylight/md-sal/statistics-manager/pom.xml index 0f90ecac60..5c3fc2329a 100644 --- a/opendaylight/md-sal/statistics-manager/pom.xml +++ b/opendaylight/md-sal/statistics-manager/pom.xml @@ -39,6 +39,17 @@ org.eclipse.xtend org.eclipse.xtend.lib + + + junit + junit + + + org.slf4j + slf4j-log4j12 + ${slf4j.version} + test + diff --git a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatisticsUpdateCommiter.java b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatisticsUpdateCommiter.java index 5f264abc2c..a7c00d4591 100644 --- a/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatisticsUpdateCommiter.java +++ b/opendaylight/md-sal/statistics-manager/src/main/java/org/opendaylight/controller/md/statistics/manager/StatisticsUpdateCommiter.java @@ -127,13 +127,17 @@ public class StatisticsUpdateCommiter implements OpendaylightGroupStatisticsList OpendaylightFlowTableStatisticsListener, OpendaylightQueueStatisticsListener{ - public final static Logger sucLogger = LoggerFactory.getLogger(StatisticsUpdateCommiter.class); + private final static Logger sucLogger = LoggerFactory.getLogger(StatisticsUpdateCommiter.class); private final StatisticsProvider statisticsManager; private final MultipartMessageManager messageManager; private int unaccountedFlowsCounter = 1; + /** + * default ctor + * @param manager + */ public StatisticsUpdateCommiter(final StatisticsProvider manager){ this.statisticsManager = manager; @@ -701,7 +705,7 @@ public class StatisticsUpdateCommiter implements OpendaylightGroupStatisticsList } - private NodeRef getNodeRef(NodeKey nodeKey){ + private static NodeRef getNodeRef(NodeKey nodeKey){ InstanceIdentifierBuilder builder = InstanceIdentifier.builder(Nodes.class).child(Node.class, nodeKey); return new NodeRef(builder.toInstance()); } @@ -861,34 +865,65 @@ public class StatisticsUpdateCommiter implements OpendaylightGroupStatisticsList return true; } - private boolean layer3MatchEquals(Layer3Match statsLayer3Match, Layer3Match storedLayer3Match){ - + protected static boolean layer3MatchEquals(Layer3Match statsLayer3Match, Layer3Match storedLayer3Match){ + boolean verdict = true; if(statsLayer3Match instanceof Ipv4Match && storedLayer3Match instanceof Ipv4Match){ Ipv4Match statsIpv4Match = (Ipv4Match)statsLayer3Match; Ipv4Match storedIpv4Match = (Ipv4Match)storedLayer3Match; - - if (storedIpv4Match.getIpv4Destination()== null) { - if (statsIpv4Match.getIpv4Destination()!= null) { - return false; - } - } else if(!IpAddressEquals(statsIpv4Match.getIpv4Destination(),storedIpv4Match.getIpv4Destination())){ - return false; + + if (verdict) { + verdict = compareNullSafe( + storedIpv4Match.getIpv4Destination(), statsIpv4Match.getIpv4Destination()); } - if (storedIpv4Match.getIpv4Source() == null) { - if (statsIpv4Match.getIpv4Source() != null) { - return false; - } - } else if(!IpAddressEquals(statsIpv4Match.getIpv4Source(),storedIpv4Match.getIpv4Source())) { - return false; + if (verdict) { + verdict = compareNullSafe( + statsIpv4Match.getIpv4Source(), storedIpv4Match.getIpv4Source()); + } + } else { + Boolean nullCheckOut = checkNullValues(storedLayer3Match, statsLayer3Match); + if (nullCheckOut != null) { + verdict = nullCheckOut; + } else { + verdict = storedLayer3Match.equals(statsLayer3Match); } - - return true; - }else{ - return storedLayer3Match.equals(statsLayer3Match); } + + return verdict; } - private boolean IpAddressEquals(Ipv4Prefix statsIpAddress, Ipv4Prefix storedIpAddress) { + private static boolean compareNullSafe(Ipv4Prefix statsIpv4, Ipv4Prefix storedIpv4) { + boolean verdict = true; + Boolean checkDestNullValuesOut = checkNullValues(storedIpv4, statsIpv4); + if (checkDestNullValuesOut != null) { + verdict = checkDestNullValuesOut; + } else if(!IpAddressEquals(statsIpv4, storedIpv4)){ + verdict = false; + } + + return verdict; + } + + private static Boolean checkNullValues(Object v1, Object v2) { + Boolean verdict = null; + if (v1 == null && v2 != null) { + verdict = Boolean.FALSE; + } else if (v1 != null && v2 == null) { + verdict = Boolean.FALSE; + } else if (v1 == null && v2 == null) { + verdict = Boolean.TRUE; + } + + return verdict; + } + + /** + * TODO: why don't we use the default Ipv4Prefix.equals()? + * + * @param statsIpAddress + * @param storedIpAddress + * @return true if IPv4prefixes equals + */ + private static boolean IpAddressEquals(Ipv4Prefix statsIpAddress, Ipv4Prefix storedIpAddress) { IntegerIpAddress statsIpAddressInt = StrIpToIntIp(statsIpAddress.getValue()); IntegerIpAddress storedIpAddressInt = StrIpToIntIp(storedIpAddress.getValue()); @@ -901,19 +936,19 @@ public class StatisticsUpdateCommiter implements OpendaylightGroupStatisticsList return false; } - private boolean IpAndMaskBasedMatch(IntegerIpAddress statsIpAddressInt,IntegerIpAddress storedIpAddressInt){ + private static boolean IpAndMaskBasedMatch(IntegerIpAddress statsIpAddressInt,IntegerIpAddress storedIpAddressInt){ return ((statsIpAddressInt.getIp() & statsIpAddressInt.getMask()) == (storedIpAddressInt.getIp() & storedIpAddressInt.getMask())); } - private boolean IpBasedMatch(IntegerIpAddress statsIpAddressInt,IntegerIpAddress storedIpAddressInt){ + private static boolean IpBasedMatch(IntegerIpAddress statsIpAddressInt,IntegerIpAddress storedIpAddressInt){ return (statsIpAddressInt.getIp() == storedIpAddressInt.getIp()); } - /* + /** * Method return integer version of ip address. Converted int will be mask if * mask specified */ - private IntegerIpAddress StrIpToIntIp(String ipAddresss){ + private static IntegerIpAddress StrIpToIntIp(String ipAddresss){ String[] parts = ipAddresss.split("/"); String ip = parts[0]; @@ -925,23 +960,26 @@ public class StatisticsUpdateCommiter implements OpendaylightGroupStatisticsList prefix = Integer.parseInt(parts[1]); } - Inet4Address addr =null; + IntegerIpAddress integerIpAddress = null; try { - addr = (Inet4Address) InetAddress.getByName(ip); - } catch (UnknownHostException e){} - - byte[] addrBytes = addr.getAddress(); - int ipInt = ((addrBytes[0] & 0xFF) << 24) | - ((addrBytes[1] & 0xFF) << 16) | - ((addrBytes[2] & 0xFF) << 8) | - ((addrBytes[3] & 0xFF) << 0); - - int mask = 0xffffffff << 32 - prefix; + Inet4Address addr = (Inet4Address) InetAddress.getByName(ip); + byte[] addrBytes = addr.getAddress(); + int ipInt = ((addrBytes[0] & 0xFF) << 24) | + ((addrBytes[1] & 0xFF) << 16) | + ((addrBytes[2] & 0xFF) << 8) | + ((addrBytes[3] & 0xFF) << 0); + + int mask = 0xffffffff << 32 - prefix; + + integerIpAddress = new IntegerIpAddress(ipInt, mask); + } catch (UnknownHostException e){ + sucLogger.error("Failed to determine host IP address by name: {}", e.getMessage(), e); + } - return new IntegerIpAddress(ipInt, mask); + return integerIpAddress; } - class IntegerIpAddress{ + static class IntegerIpAddress{ int ip; int mask; public IntegerIpAddress(int ip, int mask) { diff --git a/opendaylight/md-sal/statistics-manager/src/test/java/org/opendaylight/controller/md/statistics/manager/StatisticsUpdateCommiterTest.java b/opendaylight/md-sal/statistics-manager/src/test/java/org/opendaylight/controller/md/statistics/manager/StatisticsUpdateCommiterTest.java new file mode 100644 index 0000000000..5da6ef3490 --- /dev/null +++ b/opendaylight/md-sal/statistics-manager/src/test/java/org/opendaylight/controller/md/statistics/manager/StatisticsUpdateCommiterTest.java @@ -0,0 +1,115 @@ +/** + * Copyright (c) 2014 Cisco Systems, Inc. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ + +package org.opendaylight.controller.md.statistics.manager; + +import org.junit.Assert; +import org.junit.Test; +import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.Ipv4Prefix; +import org.opendaylight.yang.gen.v1.urn.opendaylight.model.match.types.rev131026.match.layer._3.match.Ipv4Match; +import org.opendaylight.yang.gen.v1.urn.opendaylight.model.match.types.rev131026.match.layer._3.match.Ipv4MatchBuilder; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * + */ +public class StatisticsUpdateCommiterTest { + + private static final Logger LOG = LoggerFactory + .getLogger(StatisticsUpdateCommiterTest.class); + + /** + * Test method for {@link org.opendaylight.controller.md.statistics.manager.StatisticsUpdateCommiter#layer3MatchEquals(org.opendaylight.yang.gen.v1.urn.opendaylight.model.match.types.rev131026.match.Layer3Match, org.opendaylight.yang.gen.v1.urn.opendaylight.model.match.types.rev131026.match.Layer3Match)}. + */ + @Test + public void testLayer3MatchEquals() { + String[][][] matchSeeds = new String[][][] { + {{"10.1.2.0/24", "10.1.2.0/24"}, {"10.1.2.0/24", "10.1.2.0/24"}}, + {{"10.1.2.0/24", "10.1.2.0/24"}, {"10.1.2.0/24", "10.1.1.0/24"}}, + {{"10.1.1.0/24", "10.1.2.0/24"}, {"10.1.2.0/24", "10.1.2.0/24"}}, + {{"10.1.1.0/24", "10.1.1.0/24"}, {"10.1.2.0/24", "10.1.2.0/24"}}, + + {{"10.1.1.0/24", null}, {"10.1.1.0/24", "10.1.2.0/24"}}, + {{"10.1.1.0/24", null}, {"10.1.2.0/24", "10.1.2.0/24"}}, + {{"10.1.1.0/24", null}, {"10.1.2.0/24", null}}, + {{"10.1.1.0/24", null}, {"10.1.1.0/24", null}}, + + {{null, "10.1.1.0/24"}, {"10.1.2.0/24", "10.1.1.0/24"}}, + {{null, "10.1.1.0/24"}, {"10.1.2.0/24", "10.1.2.0/24"}}, + {{null, "10.1.1.0/24"}, {null, "10.1.2.0/24"}}, + {{null, "10.1.1.0/24"}, {null, "10.1.1.0/24"}}, + + {{null, null}, {null, "10.1.1.0/24"}}, + {{null, null}, {null, null}}, + }; + + boolean[] matches = new boolean[] { + true, + false, + false, + false, + + false, + false, + false, + true, + + false, + false, + false, + true, + + false, + true + }; + + for (int i = 0; i < matches.length; i++) { + checkComparisonOfL3Match( + matchSeeds[i][0][0], matchSeeds[i][0][1], + matchSeeds[i][1][0], matchSeeds[i][1][1], + matches[i]); + } + } + + /** + * @param m1Source match1 - src + * @param m1Destination match1 - dest + * @param m2Source match2 - src + * @param msDestination match2 - dest + * @param matches expected match output + * + */ + private static void checkComparisonOfL3Match(String m1Source, String m1Destination, + String m2Source, String msDestination, boolean matches) { + Ipv4Match m1Layer3 = prepareIPv4Match(m1Source, m1Destination); + Ipv4Match m2Layer3 = prepareIPv4Match(m2Source, msDestination); + boolean comparisonResult; + try { + comparisonResult = StatisticsUpdateCommiter.layer3MatchEquals(m1Layer3, m2Layer3); + Assert.assertEquals("failed to compare: "+m1Layer3+" vs. "+m2Layer3, + matches, comparisonResult); + } catch (Exception e) { + LOG.error("failed to compare: {} vs. {}", m1Layer3, m2Layer3, e); + Assert.fail(e.getMessage()); + } + } + + private static Ipv4Match prepareIPv4Match(String source, String destination) { + Ipv4MatchBuilder ipv4MatchBuilder = new Ipv4MatchBuilder(); + if (source != null) { + ipv4MatchBuilder.setIpv4Source(new Ipv4Prefix(source)); + } + if (destination != null) { + ipv4MatchBuilder.setIpv4Destination(new Ipv4Prefix(destination)); + } + + return ipv4MatchBuilder.build(); + } + +} diff --git a/opendaylight/md-sal/statistics-manager/src/test/resources/log4j-test.xml b/opendaylight/md-sal/statistics-manager/src/test/resources/log4j-test.xml new file mode 100644 index 0000000000..bd3bf3cd2c --- /dev/null +++ b/opendaylight/md-sal/statistics-manager/src/test/resources/log4j-test.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + + + + + +