From 161962dad34fe6dfccb60cad97dfdc209cae8098 Mon Sep 17 00:00:00 2001 From: Ryan Izard <rizard@g.clemson.edu> Date: Thu, 7 Aug 2014 11:05:04 -0700 Subject: [PATCH] Forgot to update overridden equals() for many classes in devicemanager package. Devices are now compared correctly and are learned and not added repeatedly. --- .../internal/AttachmentPoint.java | 9 ++- .../devicemanager/internal/Device.java | 58 +++++++------------ .../devicemanager/internal/Entity.java | 6 +- .../devicemanager/internal/IndexedEntity.java | 14 ++--- 4 files changed, 32 insertions(+), 55 deletions(-) diff --git a/src/main/java/net/floodlightcontroller/devicemanager/internal/AttachmentPoint.java b/src/main/java/net/floodlightcontroller/devicemanager/internal/AttachmentPoint.java index 46c2408fa..da7490f1e 100644 --- a/src/main/java/net/floodlightcontroller/devicemanager/internal/AttachmentPoint.java +++ b/src/main/java/net/floodlightcontroller/devicemanager/internal/AttachmentPoint.java @@ -39,8 +39,7 @@ public class AttachmentPoint { public static final long OPENFLOW_TO_EXTERNAL_TIMEOUT = 30000; // 30 seconds public static final long CONSISTENT_TIMEOUT = 30000; // 30 seconds - public AttachmentPoint(DatapathId sw, OFPort port, Date activeSince, - Date lastSeen) { + public AttachmentPoint(DatapathId sw, OFPort port, Date activeSince, Date lastSeen) { this.sw = sw; this.port = port; this.activeSince = activeSince; @@ -55,7 +54,7 @@ public class AttachmentPoint { } public AttachmentPoint(AttachmentPoint ap) { - this.sw = ap.sw; + this.sw = ap.getSw(); this.port = ap.port; this.activeSince = ap.activeSince; this.lastSeen = ap.lastSeen; @@ -113,9 +112,9 @@ public class AttachmentPoint { if (getClass() != obj.getClass()) return false; AttachmentPoint other = (AttachmentPoint) obj; - if (port != other.port) + if (port.getPortNumber() != other.port.getPortNumber()) return false; - if (sw != other.sw) + if (sw.getLong() != other.sw.getLong()) return false; return true; } diff --git a/src/main/java/net/floodlightcontroller/devicemanager/internal/Device.java b/src/main/java/net/floodlightcontroller/devicemanager/internal/Device.java index 318ff60c5..5b663e324 100755 --- a/src/main/java/net/floodlightcontroller/devicemanager/internal/Device.java +++ b/src/main/java/net/floodlightcontroller/devicemanager/internal/Device.java @@ -53,8 +53,7 @@ import net.floodlightcontroller.topology.ITopologyService; */ @JsonSerialize(using=DeviceSerializer.class) public class Device implements IDevice { - protected static Logger log = - LoggerFactory.getLogger(Device.class); + protected static Logger log = LoggerFactory.getLogger(Device.class); private final Long deviceKey; protected final DeviceManagerImpl deviceManager; @@ -88,10 +87,8 @@ public class Device implements IDevice { * @param entity the initial entity for the device * @param entityClass the entity classes associated with the entity */ - public Device(DeviceManagerImpl deviceManager, - Long deviceKey, - Entity entity, - IEntityClass entityClass) { + public Device(DeviceManagerImpl deviceManager, Long deviceKey, + Entity entity, IEntityClass entityClass) { this.deviceManager = deviceManager; this.deviceKey = deviceKey; this.entities = new Entity[] {entity}; @@ -138,12 +135,10 @@ public class Device implements IDevice { this.oldAPs = null; this.attachmentPoints = null; if (oldAPs != null) { - this.oldAPs = - new ArrayList<AttachmentPoint>(oldAPs); + this.oldAPs = new ArrayList<AttachmentPoint>(oldAPs); } if (attachmentPoints != null) { - this.attachmentPoints = - new ArrayList<AttachmentPoint>(attachmentPoints); + this.attachmentPoints = new ArrayList<AttachmentPoint>(attachmentPoints); } this.macAddressString = this.entities[0].getMacAddress().toString(); this.entityClass = entityClass; @@ -163,31 +158,26 @@ public class Device implements IDevice { * new entity should be inserted. If negative we will compute the * correct insertion point */ - public Device(Device device, - Entity newEntity, - int insertionpoint) { + public Device(Device device, Entity newEntity, int insertionpoint) { this.deviceManager = device.deviceManager; this.deviceKey = device.deviceKey; this.dhcpClientName = device.dhcpClientName; this.entities = new Entity[device.entities.length + 1]; if (insertionpoint < 0) { - insertionpoint = -(Arrays.binarySearch(device.entities, - newEntity)+1); + insertionpoint = -(Arrays.binarySearch(device.entities, newEntity) + 1); } if (insertionpoint > 0) { // insertion point is not the beginning: // copy up to insertion point - System.arraycopy(device.entities, 0, - this.entities, 0, - insertionpoint); + System.arraycopy(device.entities, 0, this.entities, 0, insertionpoint); } if (insertionpoint < device.entities.length) { // insertion point is not the end // copy from insertion point System.arraycopy(device.entities, insertionpoint, - this.entities, insertionpoint+1, - device.entities.length-insertionpoint); + this.entities, insertionpoint + 1, + device.entities.length - insertionpoint); } this.entities[insertionpoint] = newEntity; /* @@ -198,13 +188,11 @@ public class Device implements IDevice { */ this.oldAPs = null; if (device.oldAPs != null) { - this.oldAPs = - new ArrayList<AttachmentPoint>(device.oldAPs); + this.oldAPs = new ArrayList<AttachmentPoint>(device.oldAPs); } this.attachmentPoints = null; if (device.attachmentPoints != null) { - this.attachmentPoints = - new ArrayList<AttachmentPoint>(device.attachmentPoints); + this.attachmentPoints = new ArrayList<AttachmentPoint>(device.attachmentPoints); } this.macAddressString = this.entities[0].getMacAddress().toString(); @@ -251,7 +239,7 @@ public class Device implements IDevice { List<AttachmentPoint>tempAP = new ArrayList<AttachmentPoint>(); for(AttachmentPoint ap: oldAP) { - if (deviceManager.isValidAttachmentPoint(ap.getSw(), ap.getPort())){ + if (deviceManager.isValidAttachmentPoint(ap.getSw(), ap.getPort())) { tempAP.add(ap); } } @@ -265,8 +253,7 @@ public class Device implements IDevice { for(int i=0; i<oldAP.size(); ++i) { AttachmentPoint ap = oldAP.get(i); // if this is not a valid attachment point, continue - if (!deviceManager.isValidAttachmentPoint(ap.getSw(), - ap.getPort())) + if (!deviceManager.isValidAttachmentPoint(ap.getSw(), ap.getPort())) continue; DatapathId id = topology.getL2DomainId(ap.getSw()); @@ -290,9 +277,9 @@ public class Device implements IDevice { if (apList == null) return false; for(AttachmentPoint ap: apList) { - if (ap.getLastSeen().getTime() + AttachmentPoint.INACTIVITY_INTERVAL < - System.currentTimeMillis()) + if (ap.getLastSeen().getTime() + AttachmentPoint.INACTIVITY_INTERVAL < System.currentTimeMillis()) { expiredAPs.add(ap); + } } if (expiredAPs.size() > 0) { apList.removeAll(expiredAPs); @@ -312,17 +299,15 @@ public class Device implements IDevice { * @param apMap * @return */ - List<AttachmentPoint> getDuplicateAttachmentPoints(List<AttachmentPoint>oldAPList, - Map<DatapathId, AttachmentPoint>apMap) { + List<AttachmentPoint> getDuplicateAttachmentPoints(List<AttachmentPoint>oldAPList, Map<DatapathId, AttachmentPoint>apMap) { ITopologyService topology = deviceManager.topology; List<AttachmentPoint> dupAPs = new ArrayList<AttachmentPoint>(); - long timeThreshold = System.currentTimeMillis() - - AttachmentPoint.INACTIVITY_INTERVAL; + long timeThreshold = System.currentTimeMillis() - AttachmentPoint.INACTIVITY_INTERVAL; if (oldAPList == null || apMap == null) return dupAPs; - for(AttachmentPoint ap: oldAPList) { + for(AttachmentPoint ap : oldAPList) { DatapathId id = topology.getL2DomainId(ap.getSw()); AttachmentPoint trueAP = apMap.get(id); @@ -362,8 +347,7 @@ public class Device implements IDevice { // Prepare the new attachment point list. if (moved) { log.info("updateAttachmentPoint: ap {} newmap {} ", attachmentPoints, newMap); - List<AttachmentPoint> newAPList = - new ArrayList<AttachmentPoint>(); + List<AttachmentPoint> newAPList = new ArrayList<AttachmentPoint>(); if (newMap != null) newAPList.addAll(newMap.values()); this.attachmentPoints = newAPList; } @@ -695,7 +679,7 @@ public class Device implements IDevice { if (e.switchDPID == swp.getSwitchDPID() && e.switchPort == swp.getPort()) { if (e.getVlan() == null) - vals.add(VlanVid.ZERO); //TODO @Ryan is this the correct way to represent an untagged vlan? + vals.add(VlanVid.ofVlan(-1)); //TODO @Ryan is this the correct way to represent an untagged vlan? else vals.add(e.getVlan()); } diff --git a/src/main/java/net/floodlightcontroller/devicemanager/internal/Entity.java b/src/main/java/net/floodlightcontroller/devicemanager/internal/Entity.java index 39a8d79cd..963226639 100644 --- a/src/main/java/net/floodlightcontroller/devicemanager/internal/Entity.java +++ b/src/main/java/net/floodlightcontroller/devicemanager/internal/Entity.java @@ -167,9 +167,7 @@ public class Entity implements Comparable<Entity> { * @see {@link Entity#activeSince} */ public void setLastSeenTimestamp(Date lastSeenTimestamp) { - if (activeSince == null || - (activeSince.getTime() + ACTIVITY_TIMEOUT) < - lastSeenTimestamp.getTime()) + if (activeSince == null || (activeSince.getTime() + ACTIVITY_TIMEOUT) < lastSeenTimestamp.getTime()) this.activeSince = lastSeenTimestamp; this.lastSeenTimestamp = lastSeenTimestamp; } @@ -208,7 +206,7 @@ public class Entity implements Comparable<Entity> { if (ipv4Address == null) { if (other.ipv4Address != null) return false; } else if (!ipv4Address.equals(other.ipv4Address)) return false; - if (macAddress != other.macAddress) return false; + if (!macAddress.equals(other.macAddress)) return false; if (switchDPID == null) { if (other.switchDPID != null) return false; } else if (!switchDPID.equals(other.switchDPID)) return false; diff --git a/src/main/java/net/floodlightcontroller/devicemanager/internal/IndexedEntity.java b/src/main/java/net/floodlightcontroller/devicemanager/internal/IndexedEntity.java index a1e7160b2..6fb1dda13 100644 --- a/src/main/java/net/floodlightcontroller/devicemanager/internal/IndexedEntity.java +++ b/src/main/java/net/floodlightcontroller/devicemanager/internal/IndexedEntity.java @@ -134,32 +134,28 @@ public class IndexedEntity { for (IDeviceService.DeviceField f : keyFields) { switch (f) { case MAC: - if (entity.macAddress != other.entity.macAddress) + if (!entity.macAddress.equals(other.entity.macAddress)) return false; break; case IPV4: if (entity.ipv4Address == null) { if (other.entity.ipv4Address != null) return false; - } else if (!entity.ipv4Address. - equals(other.entity.ipv4Address)) return false; + } else if (!entity.ipv4Address.equals(other.entity.ipv4Address)) return false; break; case SWITCH: if (entity.switchDPID == null) { if (other.entity.switchDPID != null) return false; - } else if (!entity.switchDPID. - equals(other.entity.switchDPID)) return false; + } else if (!entity.switchDPID.equals(other.entity.switchDPID)) return false; break; case PORT: if (entity.switchPort == null) { if (other.entity.switchPort != null) return false; - } else if (!entity.switchPort. - equals(other.entity.switchPort)) return false; + } else if (!entity.switchPort.equals(other.entity.switchPort)) return false; break; case VLAN: if (entity.vlan == null) { if (other.entity.vlan != null) return false; - } else if (!entity.vlan. - equals(other.entity.vlan)) return false; + } else if (!entity.vlan.equals(other.entity.vlan)) return false; break; } } -- GitLab