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

ONANYCHANGE subscriptions #1679

Merged
merged 10 commits into from
Jan 18, 2016
Merged

Conversation

fgalan
Copy link
Member

@fgalan fgalan commented Jan 15, 2016

This PR stated as preparation for expression q filter in NGSIv2 subscriptions (thus the name of the branch, referring #1316) but at the end has become an implementation of ONANYCHANGE subscriptions, i.e. #350 (functionality that it is also needed for NGSIv2 subscriptions).

(Commit history is a bit weird, maybe due to a not-so-good rebase, but the diff agains develop seems to be ok anyway).

@@ -0,0 +1,422 @@
# Copyright 2016 Telefonica Investigacion y Desarrollo, S.A.U
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 4 tests included in this PR are almost identical. Thus, you can review just one and check in the others just the following bits:

  • brokerStart line uses -noCache in the proper files
  • That condValues is [] or has been omitted in the proper files.

@fgalan fgalan mentioned this pull request Jan 15, 2016
*
* matchExpression
*/
static bool matchExpresion(ContextElementResponse* cerP, const std::string& q)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

matchExpression (one 's' missing)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 72c5ebc

@@ -1206,6 +1206,10 @@ static bool addTriggeredSubscriptions_withCache
aList,
cSubP->subscriptionId,
cSubP->tenant);

// FIXME P10 #1316: expression needs to be stored in the cache
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P10 ...
Is this something you marked to not forget and then forgot?
Looks important enough to be included in this PR ... ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to be included in a next PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NTC

@kzangeli
Copy link
Member

LGTM

kzangeli pushed a commit that referenced this pull request Jan 18, 2016
…b_in_subs_ngsiv2

ONANYCHANGE subscriptions
@kzangeli kzangeli merged commit 2f72008 into develop Jan 18, 2016
@kzangeli kzangeli deleted the feature/1316_expresion_stub_in_subs_ngsiv2 branch January 18, 2016 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants