Fixed issue #1706

CHANGES_NEXT_RELEASE

@@ -1,5 +1,6 @@
 ## Fixed Issues:
-  #XXXX: Fixed XXX
+  #1706: Fixed a bug in the logic merging attributes from the URL parameter (attrs) with attributes from registrations
+  #XXXX: Fixed a bug for system attributes which were sometimes included for sub-attributes in responses even when not requested
 
 ## New Features:
   #XXXX: The context cache now ignores the protocol (http, https) in the URLs and thus avoids storing two copies of the very same @context

src/lib/orionld/regMatch/regMatchAttributesForGet.cpp

@@ -95,8 +95,37 @@ StringArray* regMatchAttributesForGet
 {
   bool allAttributes = (propertyNamesP == NULL) && (relationshipNamesP == NULL);
 
+#ifdef DEBUG
   LM_T(LmtDistOpAttributes, ("Creating the union of attributes GET URL-Param vs Registered Attributes"));
 
+  if (attrListP != NULL)
+  {
+    LM_T(LmtDistOpAttributes, ("URI param 'attrs':"));
+    for (int ix = 0; ix < attrListP->items; ix++)
+    {
+      LM_T(LmtDistOpAttributes, ("  o %s", attrListP->array[ix]));
+    }
+  }
+
+  if (propertyNamesP != NULL)
+  {
+    LM_T(LmtDistOpAttributes, ("Reg 'propertyNames':"));
+    for (KjNode* propertyP = propertyNamesP->value.firstChildP; propertyP != NULL; propertyP = propertyP->next)
+    {
+      LM_T(LmtDistOpAttributes, ("  o %s", propertyP->value.s));
+    }
+  }
+
+  if (relationshipNamesP != NULL)
+  {
+    LM_T(LmtDistOpAttributes, ("Reg 'relationshipNames':"));
+    for (KjNode* relationshipP = relationshipNamesP->value.firstChildP; relationshipP != NULL; relationshipP = relationshipP->next)
+    {
+      LM_T(LmtDistOpAttributes, ("  o %s", relationshipP->value.s));
+    }
+  }
+#endif
+
   if (allAttributes == true)
   {
     //
@@ -118,7 +147,7 @@ StringArray* regMatchAttributesForGet
   StringArray* sList = (StringArray*) kaAlloc(&orionldState.kalloc, sizeof(StringArray));
   int          items = 0;
 
-  if (allAttributes == true)  // We know that attrListP == NULL (otherwise it would have returned already a few lines up)
+  if (allAttributes == true)  // We know that attrListP == NULL (otherwise this function would have ended already a few lines up)
   {
     // Everything matches - return an empty array
     sList->items = 0;
@@ -127,7 +156,10 @@ StringArray* regMatchAttributesForGet
     return sList;
   }
   else if ((attrListP != NULL) && (attrListP->items > 0))
+  {
     items = attrListP->items;
+    LM_T(LmtDistOpAttributes, ("%d attributes in URL param", items));
+  }
   else
   {
     // Count items in propertyNamesP + relationshipNamesP
@@ -168,24 +200,34 @@ StringArray* regMatchAttributesForGet
   else
   {
     int matches = 0;
+    LM_T(LmtDistOpAttributes, ("Matching %d URL attrs", attrListP->items));
     for (int ix = 0; ix < attrListP->items; ix++)
     {
       bool match = false;
 
+      LM_T(LmtDistOpAttributes, ("Matching URL attr '%s' with propertyNames", attrListP->array[ix]));
       if (propertyNamesP != NULL)
         match = (kjStringValueLookupInArray(propertyNamesP, attrListP->array[ix]) != NULL);
 
+      LM_T(LmtDistOpAttributes, ("Matching URL attr '%s' with relationshipNames", attrListP->array[ix]));
       if ((match == false) && (relationshipNamesP != NULL))
         match = (kjStringValueLookupInArray(relationshipNamesP, attrListP->array[ix]) != NULL);
 
       if (match == false)
+      {
+        LM_T(LmtDistOpAttributes, ("%s is not a match", attrListP->array[ix]));
         continue;
+      }
 
       LM_T(LmtDistOpAttributes, ("Adding '%s' to the attrList of the DistOp", attrListP->array[ix]));
       sList->array[matches++]  = attrListP->array[ix];
 
       if (regP->mode == RegModeExclusive)
+      {
         stringArrayRemoveItem(attrListP, ix);
+        --ix;  // Compensating for the item in attrListP that was just removed
+      }
+      LM_T(LmtDistOpAttributes, ("Matching %d (ix is %d) URL attrs", attrListP->items, ix));
     }
 
     if (matches == 0)

src/lib/orionld/serviceRoutines/orionldGetEntitiesPage.cpp

@@ -57,38 +57,44 @@ extern "C"
 //
 static void cleanupSysAttrs(void)
 {
-  LM_T(LmtSR, ("In cleanupSysAttrs: orionldState.responseTree: %p", orionldState.responseTree));
+  LM_T(LmtSysAttrs, ("orionldState.responseTree: %p", orionldState.responseTree));
 
   for (KjNode* entityP = orionldState.responseTree->value.firstChildP; entityP != NULL; entityP = entityP->next)
   {
-    KjNode* attrP = entityP->value.firstChildP;
-    KjNode* nextAttrP;
+    KjNode*     idP = kjLookup(entityP, "id");
+    const char* id  = (idP != NULL)? idP->value.s : "unidentified";
 
-    while (attrP != NULL)
+    LM_T(LmtSysAttrs, ("Removing sysAttrs for the entity '%s'", id));
+
+    KjNode* createdAtP  = kjLookup(entityP, "createdAt");
+    KjNode* modifiedAtP = kjLookup(entityP, "modifiedAt");
+    if (createdAtP  != NULL) kjChildRemove(entityP, createdAtP);
+    if (modifiedAtP != NULL) kjChildRemove(entityP, modifiedAtP);
+
+    for (KjNode* attrP = entityP->value.firstChildP; attrP != NULL; attrP = attrP->next)
     {
-      nextAttrP = attrP->next;
+      if (attrP->type != KjObject)
+        continue;
 
-      if      (strcmp(attrP->name, "createdAt")  == 0)  kjChildRemove(entityP, attrP);
-      else if (strcmp(attrP->name, "modifiedAt") == 0)  kjChildRemove(entityP, attrP);
-      else if (attrP->type == KjObject)
-      {
-        // It's an attribute
+      LM_T(LmtSysAttrs, ("Removing sysAttrs for the attribute '%s'", attrP->name));
 
-        KjNode* subAttrP = attrP->value.firstChildP;
-        KjNode* nextSubAttrP;
+      KjNode* createdAtP  = kjLookup(attrP, "createdAt");
+      KjNode* modifiedAtP = kjLookup(attrP, "modifiedAt");
+      if (createdAtP  != NULL) kjChildRemove(attrP, createdAtP);
+      if (modifiedAtP != NULL) kjChildRemove(attrP, modifiedAtP);
 
-        while (subAttrP != NULL)
-        {
-          nextSubAttrP = subAttrP->next;
+      for (KjNode* subAttrP = attrP->value.firstChildP; subAttrP != NULL; subAttrP = subAttrP->next)
+      {
+        if (subAttrP->type != KjObject)
+          continue;
 
-          if      (strcmp(subAttrP->name, "createdAt")  == 0)  kjChildRemove(attrP, subAttrP);
-          else if (strcmp(subAttrP->name, "modifiedAt") == 0)  kjChildRemove(attrP, subAttrP);
+        LM_T(LmtSysAttrs, ("Removing sysAttrs for the sub-attribute '%s'", subAttrP->name));
 
-          subAttrP = nextSubAttrP;
-        }
+        KjNode* createdAtP  = kjLookup(subAttrP, "createdAt");
+        KjNode* modifiedAtP = kjLookup(subAttrP, "modifiedAt");
+        if (createdAtP  != NULL) kjChildRemove(subAttrP, createdAtP);
+        if (modifiedAtP != NULL) kjChildRemove(subAttrP, modifiedAtP);
       }
-
-      attrP = nextAttrP;
     }
   }
 }