Clean up handling of lat/long geo-coordinates.
Change-Id: I64fca56c7deb9a8baa6c68558365ec2a8c38168c
diff --git a/core/api/src/main/java/org/onosproject/net/config/basics/BasicElementConfig.java b/core/api/src/main/java/org/onosproject/net/config/basics/BasicElementConfig.java
index 3033b3c..74257e6 100644
--- a/core/api/src/main/java/org/onosproject/net/config/basics/BasicElementConfig.java
+++ b/core/api/src/main/java/org/onosproject/net/config/basics/BasicElementConfig.java
@@ -30,12 +30,13 @@
protected static final String RACK_ADDRESS = "rackAddress";
protected static final String OWNER = "owner";
- protected static final double DEFAULT_COORD = -1.0;
+ protected static final double ZERO_THRESHOLD = Double.MIN_VALUE * 2.0;
+ private static final double DEFAULT_COORD = 0.0;
/**
* Returns friendly label for the element.
*
- * @return friendly label or element id itself if not set
+ * @return friendly label or element identifier itself if not set
*/
public String name() {
return get(NAME, subject.toString());
@@ -51,10 +52,25 @@
return (BasicElementConfig) setOrClear(NAME, name);
}
+ private static boolean doubleIsZero(double value) {
+ return value >= -ZERO_THRESHOLD && value <= ZERO_THRESHOLD;
+ }
+
+ /**
+ * Returns true if the geographical coordinates (latitude and longitude)
+ * are set on this element.
+ *
+ * @return true if geo-coordinates are set
+ */
+ public boolean geoCoordsSet() {
+ return !doubleIsZero(latitude()) || !doubleIsZero(longitude());
+ }
+
/**
* Returns element latitude.
*
- * @return element latitude; -1 if not set
+ * @return element latitude; 0.0 if (possibly) not set
+ * @see #geoCoordsSet()
*/
public double latitude() {
return get(LATITUDE, DEFAULT_COORD);
@@ -71,9 +87,10 @@
}
/**
- * Returns element latitude.
+ * Returns element longitude.
*
- * @return element latitude; -1 if not set
+ * @return element longitude; 0 if (possibly) not set
+ * @see #geoCoordsSet()
*/
public double longitude() {
return get(LONGITUDE, DEFAULT_COORD);
diff --git a/core/api/src/test/java/org/onosproject/net/config/basics/BasicElementConfigTest.java b/core/api/src/test/java/org/onosproject/net/config/basics/BasicElementConfigTest.java
new file mode 100644
index 0000000..63040af
--- /dev/null
+++ b/core/api/src/test/java/org/onosproject/net/config/basics/BasicElementConfigTest.java
@@ -0,0 +1,98 @@
+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed 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.onosproject.net.config.basics;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.onosproject.net.config.basics.BasicElementConfig.ZERO_THRESHOLD;
+
+/**
+ * Unit tests for {@link BasicElementConfig}.
+ */
+public class BasicElementConfigTest {
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ private static final String E1 = "e1";
+
+ // concrete subclass of abstract class we are testing
+ private static class ElmCfg extends BasicElementConfig<String> {
+ ElmCfg() {
+ object = MAPPER.createObjectNode();
+ }
+
+ @Override
+ public String toString() {
+ return object.toString();
+ }
+ }
+
+ private static void print(String fmt, Object... args) {
+ System.out.println(String.format(fmt, args));
+ }
+
+ private static void print(Object o) {
+ print("%s", o);
+ }
+
+ private BasicElementConfig<?> cfg;
+
+ @Before
+ public void setUp() {
+ cfg = new ElmCfg().name(E1);
+ }
+
+ @Test
+ public void basicNoGeo() {
+ print(cfg);
+ assertFalse("geo set?", cfg.geoCoordsSet());
+ assertEquals("lat", 0.0, cfg.latitude(), ZERO_THRESHOLD);
+ assertEquals("lon", 0.0, cfg.longitude(), ZERO_THRESHOLD);
+ }
+
+ @Test
+ public void geoLatitudeOnly() {
+ cfg.latitude(0.1);
+ print(cfg);
+ assertTrue("geo NOT set", cfg.geoCoordsSet());
+ assertEquals("lat", 0.1, cfg.latitude(), ZERO_THRESHOLD);
+ assertEquals("lon", 0.0, cfg.longitude(), ZERO_THRESHOLD);
+ }
+
+ @Test
+ public void geoLongitudeOnly() {
+ cfg.longitude(-0.1);
+ print(cfg);
+ assertTrue("geo NOT set", cfg.geoCoordsSet());
+ assertEquals("lat", 0.0, cfg.latitude(), ZERO_THRESHOLD);
+ assertEquals("lon", -0.1, cfg.longitude(), ZERO_THRESHOLD);
+ }
+
+ @Test
+ public void geoLatLong() {
+ cfg.latitude(3.1415).longitude(2.71828);
+ print(cfg);
+ assertTrue("geo NOT set", cfg.geoCoordsSet());
+ assertEquals("lat", 3.1415, cfg.latitude(), ZERO_THRESHOLD);
+ assertEquals("lon", 2.71828, cfg.longitude(), ZERO_THRESHOLD);
+ }
+}
diff --git a/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java b/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
index 9645c5f..255ee46 100644
--- a/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
+++ b/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
@@ -34,8 +34,6 @@
*/
public final class BasicHostOperator implements ConfigOperator {
- protected static final double DEFAULT_COORD = -1.0;
-
private BasicHostOperator() {
}
@@ -81,10 +79,8 @@
if (cfg.name() != null) {
newBuilder.set(AnnotationKeys.NAME, cfg.name());
}
- if (cfg.latitude() != DEFAULT_COORD) {
+ if (cfg.geoCoordsSet()) {
newBuilder.set(AnnotationKeys.LATITUDE, Double.toString(cfg.latitude()));
- }
- if (cfg.longitude() != DEFAULT_COORD) {
newBuilder.set(AnnotationKeys.LONGITUDE, Double.toString(cfg.longitude()));
}
if (cfg.rackAddress() != null) {
diff --git a/core/net/src/test/java/org/onosproject/net/host/impl/BasicHostOperatorTest.java b/core/net/src/test/java/org/onosproject/net/host/impl/BasicHostOperatorTest.java
index caf36d1..3d1331a 100644
--- a/core/net/src/test/java/org/onosproject/net/host/impl/BasicHostOperatorTest.java
+++ b/core/net/src/test/java/org/onosproject/net/host/impl/BasicHostOperatorTest.java
@@ -54,18 +54,20 @@
private static final BasicHostConfig BHC = new BasicHostConfig();
private static final String NAME = "testhost";
private static final double LAT = 40.96;
+ private static final double LON = 0.0;
@Before
public void setUp() {
BHC.init(ID, "test", JsonNodeFactory.instance.objectNode(), mapper, delegate);
BHC.name(NAME).latitude(40.96);
+ // if you set lat or long, the other becomes valid as 0.0 (not null)
}
@Test
public void testDescOps() {
HostDescription desc = BasicHostOperator.combine(BHC, HOST);
assertEquals(NAME, desc.annotations().value(AnnotationKeys.NAME));
- assertEquals(null, desc.annotations().value(AnnotationKeys.LONGITUDE));
+ assertEquals(String.valueOf(LON), desc.annotations().value(AnnotationKeys.LONGITUDE));
assertEquals(String.valueOf(LAT), desc.annotations().value(AnnotationKeys.LATITUDE));
}
}