You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2021/03/03 18:28:31 UTC
[GitHub] [celix] pnoltes opened a new pull request #328: Fixes pubsub scope issue
pnoltes opened a new pull request #328:
URL: https://github.com/apache/celix/pull/328
This PR fixes a pubsub scope issue.
The celix_filter_findAttribute now also find attributes used in a negative ldap expression (!(expr)).
pubsub did not correctly handle scope=* filter entries.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes edited a comment on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
pnoltes edited a comment on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-790012238
> > Yes a service dependency with a `(topic=t)(scope=*)` filter will result in a publisher with no scope property. The dependency will match all publishers with topic=t and a scope, but not the publisher just registered.
>
> Ah, so it does match all publishers, regardless of scope? I guess it goes to show how hard it is to explain the current behaviour.
>
`(scope=*)` will match all publisher with _any_ scope, but will not match publisher with no scope. The <attr>=* is present check. But yeah for this situation (or other services on demand usage) too confusing.
Filter ABNF (https://www.ietf.org/rfc/rfc2254.txt)
```
filter = "(" filtercomp ")"
filtercomp = and / or / not / item
and = "&" filterlist
or = "|" filterlist
not = "!" filter
filterlist = 1*filter
item = simple / present / substring / extensible
simple = attr filtertype value
filtertype = equal / approx / greater / less
equal = "="
approx = "~="
greater = ">="
less = "<="
extensible = attr [":dn"] [":" matchingrule] ":=" value
/ [":dn"] ":" matchingrule ":=" value
present = attr "=*"
substring = attr "=" [initial] any [final]
initial = value
any = "*" *(value "*")
final = value
attr = AttributeDescription from Section 4.1.5 of [1]
matchingrule = MatchingRuleId from Section 4.1.9 of [1]
value = AttributeValue from Section 4.1.6 of [1]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
pnoltes commented on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-790012238
> > Yes a service dependency with a `(topic=t)(scope=*)` filter will result in a publisher with no scope property. The dependency will match all publishers with topic=t and a scope, but not the publisher just registered.
>
> Ah, so it does match all publishers, regardless of scope? I guess it goes to show how hard it is to explain the current behaviour.
>
(scope=*) will match all publisher with _any_ scope, but will not match publisher with no scope. The <attr>=* is present check. But yeah for this situation (or other services on demand usage) too confusing.
Filter ABNF (https://www.ietf.org/rfc/rfc2254.txt)
```
filter = "(" filtercomp ")"
filtercomp = and / or / not / item
and = "&" filterlist
or = "|" filterlist
not = "!" filter
filterlist = 1*filter
item = simple / present / substring / extensible
simple = attr filtertype value
filtertype = equal / approx / greater / less
equal = "="
approx = "~="
greater = ">="
less = "<="
extensible = attr [":dn"] [":" matchingrule] ":=" value
/ [":dn"] ":" matchingrule ":=" value
present = attr "=*"
substring = attr "=" [initial] any [final]
initial = value
any = "*" *(value "*")
final = value
attr = AttributeDescription from Section 4.1.5 of [1]
matchingrule = MatchingRuleId from Section 4.1.9 of [1]
value = AttributeValue from Section 4.1.6 of [1]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes edited a comment on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
pnoltes edited a comment on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-790012238
> > Yes a service dependency with a `(topic=t)(scope=*)` filter will result in a publisher with no scope property. The dependency will match all publishers with topic=t and a scope, but not the publisher just registered.
>
> Ah, so it does match all publishers, regardless of scope? I guess it goes to show how hard it is to explain the current behaviour.
>
`(scope=*)` will match all publisher with _any_ scope, but will not match publisher with no scope. The <attr>=* is a "is present" check. But yeah for this situation (or other services on demand usage) too confusing.
Filter ABNF (https://www.ietf.org/rfc/rfc2254.txt)
```
filter = "(" filtercomp ")"
filtercomp = and / or / not / item
and = "&" filterlist
or = "|" filterlist
not = "!" filter
filterlist = 1*filter
item = simple / present / substring / extensible
simple = attr filtertype value
filtertype = equal / approx / greater / less
equal = "="
approx = "~="
greater = ">="
less = "<="
extensible = attr [":dn"] [":" matchingrule] ":=" value
/ [":dn"] ":" matchingrule ":=" value
present = attr "=*"
substring = attr "=" [initial] any [final]
initial = value
any = "*" *(value "*")
final = value
attr = AttributeDescription from Section 4.1.5 of [1]
matchingrule = MatchingRuleId from Section 4.1.9 of [1]
value = AttributeValue from Section 4.1.6 of [1]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes commented on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
pnoltes commented on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-790000812
> This would mean that a filter like `(topic=t)(scope=*)` would be marked as not having a scope. This would then filter for any pubsub channel that also has no scope marked but ignoring any pubsub channel that does have a scope.
>
> I'm unsure if this behaviour is desired.
Yes a service dependency with a `(topic=t)(scope=*)` filter will result in a publisher with no scope property. The dependency will match all publisher with topic=t and a scope, but not the publisher just registered.
I am also not sure if the behavior is desired. I am leaning towards making topic and scope mandatory (for requesting publishers and providing subscriber) again. This was a far more simpler to handle and less error prone.
But reverting this behavior is also impactful, especially to align on endpoint discovery between Java and C.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-789983380
This would mean that a filter like `(topic=t)(scope=*)` would be marked as not having a scope. This would then filter for any pubsub channel that also has no scope marked but ignoring any pubsub channel that does have a scope.
I'm unsure if this behaviour is desired.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes edited a comment on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
pnoltes edited a comment on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-790000812
> This would mean that a filter like `(topic=t)(scope=*)` would be marked as not having a scope. This would then filter for any pubsub channel that also has no scope marked but ignoring any pubsub channel that does have a scope.
>
> I'm unsure if this behaviour is desired.
Yes a service dependency with a `(topic=t)(scope=*)` filter will result in a publisher with no scope property. The dependency will match all publishers with topic=t and a scope, but not the publisher just registered.
I am also not sure if the behavior is desired. I am leaning towards making topic and scope mandatory (for requesting publishers and providing subscriber) again. This was far more simpler to handle and less error prone.
But reverting this behavior is also impactful, especially to align on endpoint discovery between Java and C.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes edited a comment on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
pnoltes edited a comment on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-790012238
> > Yes a service dependency with a `(topic=t)(scope=*)` filter will result in a publisher with no scope property. The dependency will match all publishers with topic=t and a scope, but not the publisher just registered.
>
> Ah, so it does match all publishers, regardless of scope? I guess it goes to show how hard it is to explain the current behaviour.
>
`(scope=*)` will match all publisher with _any_ scope, but will not match publisher with no scope. The `<attr>=*` is a "is present" check. But yeah for this situation (or other services on demand usage) too confusing.
Filter ABNF (https://www.ietf.org/rfc/rfc2254.txt)
```
filter = "(" filtercomp ")"
filtercomp = and / or / not / item
and = "&" filterlist
or = "|" filterlist
not = "!" filter
filterlist = 1*filter
item = simple / present / substring / extensible
simple = attr filtertype value
filtertype = equal / approx / greater / less
equal = "="
approx = "~="
greater = ">="
less = "<="
extensible = attr [":dn"] [":" matchingrule] ":=" value
/ [":dn"] ":" matchingrule ":=" value
present = attr "=*"
substring = attr "=" [initial] any [final]
initial = value
any = "*" *(value "*")
final = value
attr = AttributeDescription from Section 4.1.5 of [1]
matchingrule = MatchingRuleId from Section 4.1.9 of [1]
value = AttributeValue from Section 4.1.6 of [1]
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes edited a comment on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
pnoltes edited a comment on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-790000812
> This would mean that a filter like `(topic=t)(scope=*)` would be marked as not having a scope. This would then filter for any pubsub channel that also has no scope marked but ignoring any pubsub channel that does have a scope.
>
> I'm unsure if this behaviour is desired.
Yes a service dependency with a `(topic=t)(scope=*)` filter will result in a publisher with no scope property. The dependency will match all publishers with topic=t and a scope, but not the publisher just registered.
I am also not sure if the behavior is desired. I am leaning towards making topic and scope mandatory (for requesting publishers and providing subscriber) again. This was a far more simpler to handle and less error prone.
But reverting this behavior is also impactful, especially to align on endpoint discovery between Java and C.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] pnoltes merged pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
pnoltes merged pull request #328:
URL: https://github.com/apache/celix/pull/328
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [celix] Oipo commented on pull request #328: Fixes pubsub scope issue
Posted by GitBox <gi...@apache.org>.
Oipo commented on pull request #328:
URL: https://github.com/apache/celix/pull/328#issuecomment-790006249
> Yes a service dependency with a `(topic=t)(scope=*)` filter will result in a publisher with no scope property. The dependency will match all publishers with topic=t and a scope, but not the publisher just registered.
Ah, so it does match all publishers, regardless of scope? I guess it goes to show how hard it is to explain the current behaviour.
> I am also not sure if the behavior is desired. I am leaning towards making topic and scope mandatory (for requesting publishers and providing subscriber) again. This was far more simpler to handle and less error prone.
>
> But reverting this behavior is also impactful, especially to align on endpoint discovery between Java and C.
I would agree on making it less error prone and also on doing it outside of this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org