Fixed service port create failure

Also enhanced service network and port codec and added more unit tests

Change-Id: I523acc49dc1472520bd15a47f9a591cd95297ea0
diff --git a/src/main/java/org/opencord/cordvtn/api/net/SegmentId.java b/src/main/java/org/opencord/cordvtn/api/net/SegmentId.java
index 45568d1..3b9cf51 100644
--- a/src/main/java/org/opencord/cordvtn/api/net/SegmentId.java
+++ b/src/main/java/org/opencord/cordvtn/api/net/SegmentId.java
@@ -38,6 +38,10 @@
      * @return segmentation identifier
      */
     public static SegmentId of(Long id) {
+        if (id <= 0) {
+            throw new IllegalArgumentException("Value 0 or less then 0 is not" +
+                    "allowed for segment ID");
+        }
         return new SegmentId(id);
     }
 }
diff --git a/src/main/java/org/opencord/cordvtn/codec/ServiceNetworkCodec.java b/src/main/java/org/opencord/cordvtn/codec/ServiceNetworkCodec.java
index 331bffe..02e27b7 100644
--- a/src/main/java/org/opencord/cordvtn/codec/ServiceNetworkCodec.java
+++ b/src/main/java/org/opencord/cordvtn/codec/ServiceNetworkCodec.java
@@ -33,6 +33,7 @@
 import java.util.Map;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.lang.Boolean.FALSE;
 import static java.lang.Boolean.TRUE;
 import static org.opencord.cordvtn.api.net.ServiceNetwork.DependencyType.BIDIRECTIONAL;
@@ -87,134 +88,113 @@
         return result;
     }
 
+    // TODO allow removing existing value when explicit null received
     @Override
     public ServiceNetwork decode(ObjectNode json, CodecContext context) {
-        validateJson(json);
+        checkArgument(json != null && json.isObject(), ERR_JSON);
+        checkArgument(!json.path(ID).isMissingNode() && !json.path(ID).isNull(), ERR_ID);
+        checkArgument(!Strings.isNullOrEmpty(json.path(ID).asText()), ERR_ID);
+
         ServiceNetwork.Builder snetBuilder = DefaultServiceNetwork.builder()
                 .id(NetworkId.of(json.get(ID).asText()));
 
-        // TODO remove existing values when explicit null received
-        if (json.get(NAME) != null && !json.get(NAME).isNull()) {
-            snetBuilder.name(json.get(NAME).asText());
-        }
-        if (json.get(TYPE) != null && !json.get(TYPE).isNull()) {
-            snetBuilder.type(valueOf(json.get(TYPE).asText().toUpperCase()));
-        }
-        if (json.get(SEGMENT_ID) != null && !json.get(SEGMENT_ID).isNull()) {
-            snetBuilder.segmentId(SegmentId.of(json.get(SEGMENT_ID).asLong()));
-        }
-        if (json.get(SUBNET) != null && !json.get(SUBNET).isNull()) {
-            snetBuilder.subnet(IpPrefix.valueOf(json.get(SUBNET).asText()));
-        }
-        if (json.get(SERVICE_IP) != null && !json.get(SERVICE_IP).isNull()) {
-            snetBuilder.serviceIp(IpAddress.valueOf(json.get(SERVICE_IP).asText()));
-        }
-        if (json.get(PROVIDERS) != null) {
-            if (json.get(PROVIDERS).isNull()) {
-                snetBuilder.providers(ImmutableMap.of());
-            } else {
-                Map<NetworkId, DependencyType> providers = Maps.newHashMap();
-                json.get(PROVIDERS).forEach(provider -> {
-                    DependencyType type = provider.get(DEP_TYPE).asBoolean() ?
-                            BIDIRECTIONAL : UNIDIRECTIONAL;
-                    providers.put(NetworkId.of(provider.get(ID).asText()), type);
-                });
-                snetBuilder.providers(providers);
-            }
-        }
-        if (json.get(PROVIDER_NETWORKS) != null) {
-            if (json.get(PROVIDER_NETWORKS).isNull()) {
-                snetBuilder.providers(ImmutableMap.of());
-            } else {
-                Map<NetworkId, DependencyType> providers = Maps.newHashMap();
-                json.get(PROVIDER_NETWORKS).forEach(provider -> {
-                    DependencyType type = provider.get(DEP_TYPE).asBoolean() ?
-                            BIDIRECTIONAL : UNIDIRECTIONAL;
-                    providers.put(NetworkId.of(provider.get(ID).asText()), type);
-                });
-                snetBuilder.providers(providers);
-            }
-        }
-        return snetBuilder.build();
-    }
-
-    private void validateJson(ObjectNode json) {
-        checkArgument(json != null && json.isObject(), ERR_JSON);
-        checkArgument(json.get(ID) != null && !json.get(ID).isNull(), ERR_ID);
-        checkArgument(!Strings.isNullOrEmpty(json.get(ID).asText()), ERR_ID);
-
-        // allow explicit null for removing the existing value
-        if (json.get(NAME) != null && !json.get(NAME).isNull()) {
-            if (Strings.isNullOrEmpty(json.get(NAME).asText())) {
+        if (!json.path(NAME).isMissingNode()) {
+            if (json.path(NAME).isNull() || isNullOrEmpty(json.path(NAME).asText())) {
                 final String error = "Null or empty ServiceNetwork name received";
                 throw new IllegalArgumentException(error);
+            } else {
+                snetBuilder.name(json.get(NAME).asText());
+            }
+        }
+
+        if (!json.path(TYPE).isMissingNode()) {
+            try {
+                snetBuilder.type(valueOf(json.get(TYPE).asText().toUpperCase()));
+            } catch (IllegalArgumentException | NullPointerException e) {
+                final String error = "Invalid ServiceNetwork type received";
+                throw new IllegalArgumentException(error);
             }
         }
 
-        if (json.get(TYPE) != null && !json.get(TYPE).isNull()) {
+        if (!json.path(SEGMENT_ID).isMissingNode()) {
             try {
-                valueOf(json.get(TYPE).asText().toUpperCase());
-            } catch (IllegalArgumentException e) {
-                final String error = "Invalid ServiceNetwork type received: ";
-                throw new IllegalArgumentException(error + json.get(TYPE).asText());
+                snetBuilder.segmentId(SegmentId.of(json.path(SEGMENT_ID).asLong()));
+            } catch (IllegalArgumentException | NullPointerException e) {
+                final String error = "Invalid ServiecNetwork segment ID received";
+                throw new IllegalArgumentException(error);
             }
         }
 
-        if (json.get(SEGMENT_ID) != null && !json.get(SEGMENT_ID).isNull()) {
-            if (json.get(SEGMENT_ID).asLong() == 0) {
-                final String error = "Invalid ServiecNetwork segment ID received: ";
-                throw new IllegalArgumentException(error + json.get(SEGMENT_ID).asText());
-            }
-        }
-
-        if (json.get(SUBNET) != null && !json.get(SUBNET).isNull()) {
+        if (!json.path(SUBNET).isMissingNode()) {
             try {
-                IpPrefix.valueOf(json.get(SUBNET).asText());
-            } catch (IllegalArgumentException e) {
-                final String error = "Invalid ServiceNetwork subnet received: ";
-                throw new IllegalArgumentException(error + json.get(SUBNET).asText());
+                snetBuilder.subnet(IpPrefix.valueOf(json.path(SUBNET).asText()));
+            } catch (IllegalArgumentException | NullPointerException e) {
+                final String error = "Invalid ServiceNetwork subnet received";
+                throw new IllegalArgumentException(error);
             }
         }
 
-        if (json.get(SERVICE_IP) != null && !json.get(SERVICE_IP).isNull()) {
+        if (!json.path(SERVICE_IP).isMissingNode()) {
             try {
-                IpAddress.valueOf(json.get(SERVICE_IP).asText());
-            } catch (IllegalArgumentException e) {
-                final String error = "Invalid ServiceNetwork service IP address received: ";
-                throw new IllegalArgumentException(error + json.get(SERVICE_IP).asText());
+                snetBuilder.serviceIp(IpAddress.valueOf(json.path(SERVICE_IP).asText()));
+            } catch (IllegalArgumentException | NullPointerException e) {
+                final String error = "Invalid ServiceNetwork service IP address received";
+                throw new IllegalArgumentException(error);
             }
         }
 
-        if (json.get(PROVIDERS) != null && !json.get(PROVIDERS).isNull()) {
-            json.get(PROVIDERS).forEach(provider -> {
-                if (provider.get(ID) == null || provider.get(ID).isNull() ||
-                        Strings.isNullOrEmpty(provider.get(ID).asText())) {
-                    final String error = "Null or empty provider network ID received";
-                    throw new IllegalArgumentException(error);
+        if (!json.path(PROVIDERS).isMissingNode() && json.path(PROVIDERS).isNull()) {
+            snetBuilder.providers(ImmutableMap.of());
+        } else if (!json.path(PROVIDERS).isMissingNode()) {
+            Map<NetworkId, DependencyType> providers = Maps.newHashMap();
+            json.path(PROVIDERS).forEach(provider -> {
+                if (provider.path(ID).isMissingNode() ||
+                        provider.path(ID).isNull() ||
+                        Strings.isNullOrEmpty(provider.path(ID).asText()) ||
+                        provider.path(DEP_TYPE).isMissingNode() ||
+                        provider.path(DEP_TYPE).isNull()) {
+                    final String error = "Invalid provider received: ";
+                    throw new IllegalArgumentException(error + provider.asText());
                 }
 
-                if (provider.get(DEP_TYPE) == null || provider.get(DEP_TYPE).isNull()
-                        || !provider.get(DEP_TYPE).isBoolean()) {
-                    final String error = "Non-boolean bidirectional received";
-                    throw new IllegalArgumentException(error);
+                try {
+                    DependencyType type = provider.path(DEP_TYPE).asBoolean() ?
+                            BIDIRECTIONAL : UNIDIRECTIONAL;
+                    providers.put(NetworkId.of(provider.path(ID).asText()), type);
+                } catch (IllegalArgumentException e) {
+                    final String error = "Invalid provider received: ";
+                    throw new IllegalArgumentException(error + provider.asText());
                 }
             });
+            snetBuilder.providers(providers);
         }
 
-        if (json.get(PROVIDER_NETWORKS) != null && !json.get(PROVIDER_NETWORKS).isNull()) {
-            json.get(PROVIDER_NETWORKS).forEach(provider -> {
-                if (provider.get(ID) == null || provider.get(ID).isNull() ||
-                        Strings.isNullOrEmpty(provider.get(ID).asText())) {
-                    final String error = "Null or empty provider network ID received";
-                    throw new IllegalArgumentException(error);
+        if (!json.path(PROVIDER_NETWORKS).isMissingNode() &&
+                json.path(PROVIDER_NETWORKS).isNull()) {
+            snetBuilder.providers(ImmutableMap.of());
+        } else if (!json.path(PROVIDER_NETWORKS).isMissingNode()) {
+            Map<NetworkId, DependencyType> providers = Maps.newHashMap();
+            json.path(PROVIDER_NETWORKS).forEach(provider -> {
+                if (provider.path(ID).isMissingNode() ||
+                        provider.path(ID).isNull() ||
+                        Strings.isNullOrEmpty(provider.path(ID).asText()) ||
+                        provider.path(DEP_TYPE).isMissingNode() ||
+                        provider.path(DEP_TYPE).isNull()) {
+                    final String error = "Invalid provider received: ";
+                    throw new IllegalArgumentException(error + provider.asText());
                 }
 
-                if (provider.get(DEP_TYPE) == null || provider.get(DEP_TYPE).isNull()
-                        || !provider.get(DEP_TYPE).isBoolean()) {
-                    final String error = "Non-boolean bidirectional received";
-                    throw new IllegalArgumentException(error);
+                try {
+                    DependencyType type = provider.path(DEP_TYPE).asBoolean() ?
+                            BIDIRECTIONAL : UNIDIRECTIONAL;
+                    providers.put(NetworkId.of(provider.path(ID).asText()), type);
+                } catch (IllegalArgumentException e) {
+                    final String error = "Invalid provider received: ";
+                    throw new IllegalArgumentException(error + provider.asText());
                 }
             });
+            snetBuilder.providers(providers);
         }
+        return snetBuilder.build();
     }
 }
diff --git a/src/main/java/org/opencord/cordvtn/codec/ServicePortCodec.java b/src/main/java/org/opencord/cordvtn/codec/ServicePortCodec.java
index 5292604..01a104a 100644
--- a/src/main/java/org/opencord/cordvtn/codec/ServicePortCodec.java
+++ b/src/main/java/org/opencord/cordvtn/codec/ServicePortCodec.java
@@ -17,7 +17,6 @@
 
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
-import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import org.onlab.packet.IpAddress;
@@ -26,6 +25,7 @@
 import org.onosproject.codec.CodecContext;
 import org.onosproject.codec.JsonCodec;
 import org.opencord.cordvtn.api.net.AddressPair;
+import org.opencord.cordvtn.api.net.NetworkId;
 import org.opencord.cordvtn.api.net.PortId;
 import org.opencord.cordvtn.api.net.ServicePort;
 import org.opencord.cordvtn.impl.DefaultServicePort;
@@ -33,6 +33,7 @@
 import java.util.Set;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Strings.isNullOrEmpty;
 
 /**
  * Service port JSON codec.
@@ -52,13 +53,12 @@
 
     @Override
     public ObjectNode encode(ServicePort sport, CodecContext context) {
-        ObjectNode result = context.mapper().createObjectNode()
-                .put(ID, sport.id().id());
+        ObjectNode result = context.mapper().createObjectNode().put(ID, sport.id().id());
 
         if (sport.networkId() != null) {
             result.put(NETWORK_ID, sport.networkId().id());
         }
-        if (!Strings.isNullOrEmpty(sport.name())) {
+        if (!isNullOrEmpty(sport.name())) {
             result.put(NAME, sport.name());
         }
         if (sport.vlanId() != null) {
@@ -82,103 +82,86 @@
         return result;
     }
 
+    // TODO allow removing existing value when explicit null received
     @Override
     public ServicePort decode(ObjectNode json, CodecContext context) {
-        validateJson(json);
-        ServicePort.Builder sportBuilder = DefaultServicePort.builder()
-                .id(PortId.of(json.get(ID).asText()));
-
-        // TODO allow removing existing value when explicit null received
-        if (json.get(NAME) != null && !json.get(NAME).isNull()) {
-            sportBuilder.name(json.get(NAME).asText());
-        }
-        if (json.get(MAC_ADDRESS) != null && !json.get(MAC_ADDRESS).isNull()) {
-            sportBuilder.mac(MacAddress.valueOf(json.get(MAC_ADDRESS).asText()));
-        }
-        if (json.get(IP_ADDRESS) != null && !json.get(IP_ADDRESS).isNull()) {
-            sportBuilder.ip(IpAddress.valueOf(json.get(IP_ADDRESS).asText()));
-        }
-        if (json.get(VLAN_ID) != null && !json.get(VLAN_ID).isNull()) {
-            sportBuilder.vlanId(VlanId.vlanId(json.get(VLAN_ID).asText()));
-        }
-        if (json.get(FLOATING_ADDRESS_PAIRS).isNull()) {
-            sportBuilder.addressPairs(ImmutableSet.of());
-        } else if (json.get(FLOATING_ADDRESS_PAIRS) != null) {
-            Set<AddressPair> addressPairs = Sets.newHashSet();
-            json.get(FLOATING_ADDRESS_PAIRS).forEach(pair -> {
-                AddressPair addrPair = AddressPair.of(
-                        IpAddress.valueOf(pair.get(IP_ADDRESS).asText()),
-                        MacAddress.valueOf(pair.get(MAC_ADDRESS).asText()));
-                addressPairs.add(addrPair);
-            });
-            sportBuilder.addressPairs(addressPairs);
-        }
-        return sportBuilder.build();
-    }
-
-    private void validateJson(ObjectNode json) {
         checkArgument(json != null && json.isObject(), ERR_JSON);
-        checkArgument(json.get(ID) != null && !json.get(ID).isNull(), ERR_ID);
+        checkArgument(!json.path(ID).isMissingNode() && !json.path(ID).isNull(), ERR_ID);
 
-        // allow explicit null for removing the existing value
-        if (json.get(NAME) != null && !json.get(NAME).isNull()) {
-            if (Strings.isNullOrEmpty(json.get(NAME).asText())) {
-                final String error = "Null or empty ServiceNetwork name received";
+        ServicePort.Builder sportBuilder =
+                DefaultServicePort.builder().id(PortId.of(json.get(ID).asText()));
+
+        if (!json.path(NETWORK_ID).isMissingNode()) {
+            if  (json.path(NETWORK_ID).isNull() ||
+                    isNullOrEmpty(json.path(NETWORK_ID).asText())) {
+                final String error = "Null or empty ServicePort network ID received";
+                throw new IllegalArgumentException(error);
+            } else {
+                sportBuilder.networkId(NetworkId.of(json.get(NETWORK_ID).asText()));
+            }
+        }
+
+        if (!json.path(NAME).isMissingNode()) {
+           if (json.path(NAME).isNull() || isNullOrEmpty(json.path(NAME).asText())) {
+               final String error = "Null or empty ServicePort name received";
+               throw new IllegalArgumentException(error);
+           } else {
+               sportBuilder.name(json.get(NAME).asText());
+           }
+        }
+
+        if (!json.path(MAC_ADDRESS).isMissingNode()) {
+            try {
+                sportBuilder.mac(MacAddress.valueOf(json.path(MAC_ADDRESS).asText()));
+            } catch (IllegalArgumentException | NullPointerException e) {
+                final String error = "Invalid ServicePort MAC address received";
                 throw new IllegalArgumentException(error);
             }
         }
 
-        if (json.get(MAC_ADDRESS) != null && !json.get(MAC_ADDRESS).isNull()) {
+        if (!json.path(IP_ADDRESS).isMissingNode()) {
             try {
-                MacAddress.valueOf(json.get(MAC_ADDRESS).asText());
-            } catch (IllegalArgumentException e) {
-                final String error = "Invalid ServicePort MAC address received: ";
-                throw new IllegalArgumentException(error + json.get(MAC_ADDRESS).asText());
+                sportBuilder.ip(IpAddress.valueOf(json.get(IP_ADDRESS).asText()));
+            } catch (IllegalArgumentException | NullPointerException e) {
+                final String error = "Invalid ServicePort IP address received";
+                throw new IllegalArgumentException(error);
             }
         }
 
-        if (json.get(IP_ADDRESS) != null && !json.get(IP_ADDRESS).isNull()) {
+        if (!json.path(VLAN_ID).isMissingNode()) {
             try {
-                IpAddress.valueOf(json.get(IP_ADDRESS).asText());
-            } catch (IllegalArgumentException e) {
-                final String error = "Invalid ServicePort IP address received: ";
-                throw new IllegalArgumentException(error + json.get(IP_ADDRESS).asText());
+                sportBuilder.vlanId(VlanId.vlanId(json.get(VLAN_ID).asText()));
+            } catch (IllegalArgumentException | NullPointerException e) {
+                final String error = "Invalid VLAN ID is received";
+                throw new IllegalArgumentException(error);
             }
         }
 
-        if (json.get(VLAN_ID) != null && !json.get(VLAN_ID).isNull()) {
-            try {
-                VlanId.vlanId(json.get(VLAN_ID).asText());
-            } catch (IllegalArgumentException e) {
-                final String error = "Invalid VLAN ID is received: ";
-                throw new IllegalArgumentException(error + json.get(VLAN_ID).asText());
-            }
-        }
-
-        if (json.get(FLOATING_ADDRESS_PAIRS) != null &&
-                !json.get(FLOATING_ADDRESS_PAIRS).isNull()) {
-            json.get(FLOATING_ADDRESS_PAIRS).forEach(pair -> {
-                if (pair.get(IP_ADDRESS) == null ||
-                        pair.get(IP_ADDRESS).isNull() ||
-                        pair.get(MAC_ADDRESS) == null ||
-                        pair.get(MAC_ADDRESS).isNull()) {
-                    final String error = "Invalid floating address pair received";
-                    throw new IllegalArgumentException(error);
+        if (!json.path(FLOATING_ADDRESS_PAIRS).isMissingNode() &&
+                json.path(FLOATING_ADDRESS_PAIRS).isNull()) {
+            sportBuilder.addressPairs(ImmutableSet.of());
+        } else if (!json.path(FLOATING_ADDRESS_PAIRS).isMissingNode()) {
+            Set<AddressPair> addressPairs = Sets.newHashSet();
+            json.path(FLOATING_ADDRESS_PAIRS).forEach(pair -> {
+                if (pair.path(IP_ADDRESS).isMissingNode() ||
+                        pair.path(IP_ADDRESS).isNull() ||
+                        pair.path(MAC_ADDRESS).isMissingNode() ||
+                        pair.path(MAC_ADDRESS).isNull()) {
+                    final String error = "Invalid floating address pair received: ";
+                    throw new IllegalArgumentException(error + pair.asText());
                 }
                 try {
-                    IpAddress.valueOf(pair.get(IP_ADDRESS).asText());
+                    AddressPair addrPair = AddressPair.of(
+                            IpAddress.valueOf(pair.get(IP_ADDRESS).asText()),
+                            MacAddress.valueOf(pair.get(MAC_ADDRESS).asText()));
+                    addressPairs.add(addrPair);
                 } catch (IllegalArgumentException e) {
-                    final String error = "Invalid floating address pair IP: ";
-                    throw new IllegalArgumentException(error + pair.get(IP_ADDRESS).asText());
-                }
-
-                try {
-                    MacAddress.valueOf(pair.get(MAC_ADDRESS).asText());
-                } catch (IllegalArgumentException e) {
-                    final String error = "Invalid floating address pair MAC: ";
-                    throw new IllegalArgumentException(error + pair.get(MAC_ADDRESS).asText());
+                    final String error = "Invalid floating address pair received: ";
+                    throw new IllegalArgumentException(error + pair.asText());
                 }
             });
+            sportBuilder.addressPairs(addressPairs);
         }
+        return sportBuilder.build();
     }
 }