Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: metadata modifications are not considered as change (with regards to subscription alterationTypes) if notifyOnMetadataChange is false #4612

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES_NEXT_RELEASE
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
- Fix: $max and $min operators were not supported with DateTime attributes (#4585)
- Fix: wrong date values should not allowed in subscription's expires field (#4541)
- Fix: do not raise DB alarm in case of wrong GeoJSON in client request
- Fix: metadata modifications are not considered as change (with regards to subscription alterationTypes) if notifyOnMetadataChange is false (#4605)
- Upgrade cjexl version from 0.3.0 to 0.4.0 (new transformations: now, getTime and toIsoString)
- Upgrade Debian version from 12.4 to 12.6 in Dockerfile
- Fix: invalid date in expires field of subscription (#2303)
17 changes: 15 additions & 2 deletions src/lib/apiTypesV2/Subscription.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ ngsiv2::SubAltType parseAlterationType(const std::string& altType)
{
if (altType == "entityChange")
{
return ngsiv2::SubAltType::EntityChange;
return ngsiv2::SubAltType::EntityChangeBothValueAndMetadata;
}
else if (altType == "entityUpdate")
{
Expand All @@ -64,13 +64,26 @@ ngsiv2::SubAltType parseAlterationType(const std::string& altType)



/* ****************************************************************************
*
* isChangeAltType -
*/
bool isChangeAltType(ngsiv2::SubAltType altType)
{
return (altType == ngsiv2::SubAltType::EntityChangeBothValueAndMetadata) ||
(altType == ngsiv2::SubAltType::EntityChangeOnlyMetadata) ||
(altType == ngsiv2::SubAltType::EntityChangeOnlyValue);
}



/* ****************************************************************************
*
* subAltType2string -
*/
std::string subAltType2string(ngsiv2::SubAltType altType)
{
if (altType == ngsiv2::SubAltType::EntityChange)
if (isChangeAltType(altType))
{
return "entityChange";
}
Expand Down
14 changes: 13 additions & 1 deletion src/lib/apiTypesV2/Subscription.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ typedef enum NotificationType
*/
typedef enum SubAltType
{
EntityChange,
// EntityChange has been specialized into three sub-types in order to solve #4605
// (EntityChangeBothValueAndMetadata is thre reference one used in parsing/rendering logic)
EntityChangeBothValueAndMetadata,
EntityChangeOnlyValue,
EntityChangeOnlyMetadata,
EntityUpdate,
EntityCreate,
EntityDelete,
Expand Down Expand Up @@ -192,4 +196,12 @@ extern std::string subAltType2string(ngsiv2::SubAltType altType);



/* ****************************************************************************
*
* isChangeAltType -
*/
extern bool isChangeAltType(ngsiv2::SubAltType altType);



#endif // SRC_LIB_APITYPESV2_SUBSCRIPTION_H_
22 changes: 15 additions & 7 deletions src/lib/cache/subCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ static bool matchAltType(CachedSubscription* cSubP, ngsiv2::SubAltType targetAlt
// If subAltTypeV size == 0 default alteration types are update with change and create
if (cSubP->subAltTypeV.size() == 0)
{
if ((targetAltType == ngsiv2::SubAltType::EntityChange) || (targetAltType == ngsiv2::SubAltType::EntityCreate))
if ((isChangeAltType(targetAltType)) || (targetAltType == ngsiv2::SubAltType::EntityCreate))
{
return true;
}
Expand All @@ -417,9 +417,9 @@ static bool matchAltType(CachedSubscription* cSubP, ngsiv2::SubAltType targetAlt
ngsiv2::SubAltType altType = cSubP->subAltTypeV[ix];

// EntityUpdate is special, it is a "sub-type" of EntityChange
if (targetAltType == ngsiv2::SubAltType::EntityChange)
if (isChangeAltType(targetAltType))
{
if ((altType == ngsiv2::SubAltType::EntityUpdate) || (altType == ngsiv2::SubAltType::EntityChange))
if ((altType == ngsiv2::SubAltType::EntityUpdate) || (isChangeAltType(altType)))
{
return true;
}
Expand Down Expand Up @@ -452,6 +452,12 @@ static bool subMatch
ngsiv2::SubAltType targetAltType
)
{
// If notifyOnMetadataChange is false and only metadata has been changed, we "downgrade" to ngsiv2::EntityUpdate
if (!cSubP->notifyOnMetadataChange && (targetAltType == ngsiv2::EntityChangeOnlyMetadata))
{
targetAltType = ngsiv2::EntityUpdate;
}

// Check alteration type
if (!matchAltType(cSubP, targetAltType))
{
Expand Down Expand Up @@ -490,7 +496,6 @@ static bool subMatch
return false;
}


//
// If one of the attribute names in the scope vector
// of the subscription has the same name as the incoming attribute. there is a match.
Expand All @@ -507,10 +512,13 @@ static bool subMatch
return false;
}
}
else if ((targetAltType == ngsiv2::EntityChange) || (targetAltType == ngsiv2::EntityCreate))
else if ((isChangeAltType(targetAltType)) || (targetAltType == ngsiv2::EntityCreate))
{
if (!attributeMatch(cSubP, attrsWithModifiedValue) &&
!(cSubP->notifyOnMetadataChange && attributeMatch(cSubP, attrsWithModifiedMd)))
// No match if: 1) there is no change in the *value* of attributes listed in conditions.attrs and 2) there is no change
// in the *metadata* of the attributes listed in conditions.attrs (the 2) only if notifyOnMetadataChange is true)
bool b1 = attributeMatch(cSubP, attrsWithModifiedValue);
bool b2 = attributeMatch(cSubP, attrsWithModifiedMd);
if (!b1 && !(cSubP->notifyOnMetadataChange && b2))
{
LM_T(LmtSubCacheMatch, ("No match due to attributes"));
return false;
Expand Down
53 changes: 36 additions & 17 deletions src/lib/mongoBackend/MongoCommonUpdate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,7 @@ static bool matchAltType(orion::BSONObj sub, ngsiv2::SubAltType targetAltType)
// change and create. Maybe this could be check at MongoDB query stage, but seems be more complex
if (altTypeStrings.size() == 0)
{
if ((targetAltType == ngsiv2::SubAltType::EntityChange) || (targetAltType == ngsiv2::SubAltType::EntityCreate))
if ((isChangeAltType(targetAltType)) || (targetAltType == ngsiv2::SubAltType::EntityCreate))
{
return true;
}
Expand All @@ -1513,9 +1513,9 @@ static bool matchAltType(orion::BSONObj sub, ngsiv2::SubAltType targetAltType)
else
{
// EntityUpdate is special, it is a "sub-type" of EntityChange
if (targetAltType == ngsiv2::SubAltType::EntityChange)
if (isChangeAltType(targetAltType))
{
if ((altType == ngsiv2::SubAltType::EntityUpdate) || (altType == ngsiv2::SubAltType::EntityChange))
if ((altType == ngsiv2::SubAltType::EntityUpdate) || (isChangeAltType(altType)))
{
return true;
}
Expand Down Expand Up @@ -1641,15 +1641,21 @@ static bool addTriggeredSubscriptions_noCache

if (subs.count(subIdStr) == 0)
{
// Early extraction of fiedl from DB document. The rest of fields are got later
bool notifyOnMetadataChange = sub.hasField(CSUB_NOTIFYONMETADATACHANGE)? getBoolFieldF(sub, CSUB_NOTIFYONMETADATACHANGE) : true;

// If notifyOnMetadataChange is false and only metadata has been changed, we "downgrade" to ngsiv2::EntityUpdate
if (!notifyOnMetadataChange && (targetAltType == ngsiv2::EntityChangeOnlyMetadata))
{
targetAltType = ngsiv2::EntityUpdate;
}

// Check alteration type
if (!matchAltType(sub, targetAltType))
{
continue;
}

// Early extraction of fiedl from DB document. The rest of fields are got later
bool notifyOnMetadataChange = sub.hasField(CSUB_NOTIFYONMETADATACHANGE)? getBoolFieldF(sub, CSUB_NOTIFYONMETADATACHANGE) : true;

// Depending of the alteration type, we use the list of attributes in the request or the list
// with effective modifications. Note that EntityDelete doesn't check the list
if (targetAltType == ngsiv2::EntityUpdate)
Expand All @@ -1659,11 +1665,14 @@ static bool addTriggeredSubscriptions_noCache
continue;
}
}
else if ((targetAltType == ngsiv2::EntityChange) || (targetAltType == ngsiv2::EntityCreate))
else if ((isChangeAltType(targetAltType)) || (targetAltType == ngsiv2::EntityCreate))
{
// Skip if: 1) there is no change in the *value* of attributes listed in conditions.attrs and 2) there is no change
// in the *metadata* of the attributes listed in conditions.attrs (the 2) only if notifyOnMetadtaChange is true)
if (!condValueAttrMatch(sub, attrsWithModifiedValue) && !(notifyOnMetadataChange && condValueAttrMatch(sub, attrsWithModifiedMd)))
// in the *metadata* of the attributes listed in conditions.attrs (the 2) only if notifyOnMetadataChange is true)
bool b1 = condValueAttrMatch(sub, attrsWithModifiedValue);
bool b2 = condValueAttrMatch(sub, attrsWithModifiedMd);

if (!b1 && !(notifyOnMetadataChange && b2))
{
continue;
}
Expand Down Expand Up @@ -2766,24 +2775,34 @@ static bool processContextAttributeVector
{
attrsWithModifiedValue.push_back(ca->name);
attrsWithModifiedMd.push_back(ca->name);
targetAltType = ngsiv2::SubAltType::EntityChangeBothValueAndMetadata;
}
else if (changeType == CHANGE_ONLY_VALUE)
{
attrsWithModifiedValue.push_back(ca->name);
if ((targetAltType == ngsiv2::SubAltType::EntityChangeBothValueAndMetadata) || (targetAltType == ngsiv2::SubAltType::EntityChangeOnlyMetadata))
{
targetAltType = ngsiv2::SubAltType::EntityChangeBothValueAndMetadata;
}
else
{
targetAltType = ngsiv2::SubAltType::EntityChangeOnlyValue;
}
}
else if (changeType == CHANGE_ONLY_MD)
{
attrsWithModifiedMd.push_back(ca->name);
if ((targetAltType == ngsiv2::SubAltType::EntityChangeBothValueAndMetadata) || (targetAltType == ngsiv2::SubAltType::EntityChangeOnlyValue))
{
targetAltType = ngsiv2::SubAltType::EntityChangeBothValueAndMetadata;
}
else
{
targetAltType = ngsiv2::SubAltType::EntityChangeOnlyMetadata;
}
}

attributes.push_back(ca->name);

/* If actual update then targetAltType changes from EntityUpdate (the value used to initialize
* the variable) to EntityChange */
if (changeType != NO_CHANGE)
{
targetAltType = ngsiv2::SubAltType::EntityChange;
}
}

/* Add triggered subscriptions */
Expand Down Expand Up @@ -3948,7 +3967,7 @@ static unsigned int updateEntity
* previous addTriggeredSubscriptions() invocations. Before that, we add
* builtin attributes and metadata (both NGSIv1 and NGSIv2 as this is
* for notifications and NGSIv2 builtins can be used in NGSIv1 notifications) */
addBuiltins(notifyCerP, subAltType2string(ngsiv2::SubAltType::EntityChange));
addBuiltins(notifyCerP, subAltType2string(ngsiv2::SubAltType::EntityChangeBothValueAndMetadata));
unsigned int notifSent = processSubscriptions(subsToNotify,
notifyCerP,
tenant,
Expand Down
Loading
Loading