JSON Services Registry R/W Problems

[sarcasticsys]

Encountering two problems:

1) Anything new added via the CAS-provided Services Manager gets added to the json file with an id of "-1". Subsequent services added also receive a -1 for the id. The services do seem to validate/allow authentication okay, but it causes a display issue in the Services Manager where you can only see one of them.
2) If you modify an existing service, you wind up with two copies of the service (the old one and the new one).

Using the non read-write version of the DAO does not show this behavior (granted, it doesn't actually write anything out, but the id of subsequent added services and what not don't stick with -1).

My java is a little rusty, but I poked around the code a bit and the inline patch I'll do it as an attachment, but I'm not sure how) clears it up in my test environment well enough; just not sure if this is an actual bug, expected behavior, some quirk of my environment, or me misunderstanding the configuration/use-case.

diff --git a/src/main/java/net/unicon/cas/addons/serviceregistry/ReadWriteJsonServiceRegistryDao.java b/src/main/java/net/unicon/cas/addons/serviceregistry/ReadWriteJsonServiceRegistryDao.java
index 351b977..cf5c132 100644
--- a/src/main/java/net/unicon/cas/addons/serviceregistry/ReadWriteJsonServiceRegistryDao.java
+++ b/src/main/java/net/unicon/cas/addons/serviceregistry/ReadWriteJsonServiceRegistryDao.java
@@ -34,10 +34,19 @@ public final class ReadWriteJsonServiceRegistryDao extends JsonServiceRegistryDa
         logger.debug("Loading service definitions from resource [{}]", this.servicesConfigFile.getFilename());
         final List<RegisteredService> resolvedServices = super.loadServices();
         final List<RegisteredService> col = new ArrayList<RegisteredService>(resolvedServices);
-        col.add(registeredService);
-
+        /* this means it is a modification so we need to remove the old entry */
+        if (registeredService.getId() > -1 ) {
+          /* need to search by ID because the new service object won't match the old one we want to remove */ 
+          final RegisteredService regServiceToDelete = findServiceById(registeredService.getId());
+          col.remove(regServiceToDelete);
+        }
+        /* call to super uses the CAS-standard in memory object registry which makes sure the ID incremeting
+         * when adding a new service logic takes place, may be to better import the simple get highest ID logic
+         * instead.  Planning to consult with upstream. */
+        final RegisteredService actualRegisteredService = super.saveInternal(registeredService);
+        col.add(actualRegisteredService);
         saveListOfRegisteredServices(col);
-        return registeredService;
+        return actualRegisteredService; 
     }

     @Override

[mmoayyed]

I'll review this afternoon. Thanks for the report.

[mmoayyed]

I have fixed the issue. The registry should now auto-assign positive identifiers when adding services. The issue with duplicate ids is now fixed.

Another tweak I made is to ignore writing back attributes that have empty values.

[mmoayyed]

There are some extra test cases to account for this behavior.