corenova / yang-js

YANG parser and composer
Apache License 2.0
56 stars 18 forks source link

Question: Event Subscription #33

Closed ramukima closed 8 years ago

ramukima commented 8 years ago

Model

module foo {
  list foo {
    key "name";
    leaf name {
      type string;
    }
    list bar {
      leaf name {
        type string;
      }
    }
  }
}

Event Subscription

model.on 'update', '/foo:foo/bar', (prop, prev) ->
    console.log 'updated:', prop.path.toString()

Seed data for application

{
  "foo:foo": [
    { 
      "name": "foo1"
    }
  ]
}

YangExpress Invocation

1. curl -i -X POST localhost:5050/foo:foo/foo1/bar -H 'content-type: application/json' -d '{ "name": "bar1" }'
2. curl -i -X POST localhost:5050/foo:foo/foo1/bar -H 'content-type: application/json' -d '[{ "name": "bar1" }]'

Result: None of these trigger my event subscription. I also tried modifying subscriptions to use '/foo/bar' or '/foo:foo/foo:bar' or '/foo:foo/bar/', none of those worked.

Should I expect the event triggered in this example, what am I doing wrong here ?

sekur commented 8 years ago

You should expect the event to fire as you've expressed. Looks like a bug. Let me reproduce and find out what the issue is.

ramukima commented 8 years ago

Only the following work -

foo.on 'update', (prop, prev) ->
    console.log "update for #{prop.path}"

foo.on 'update', '/foo:foo', (prop, prev) ->
    console.log "update for #{prop.path}"

foo.on 'update', '/foo:foo/', (prop, prev) ->
    console.log "update for #{prop.path}"
ramukima commented 8 years ago

If it helps, it worked in previous version - yang-js 0.15.16 (I have it locally, but some commits to even this revision broke the same version from npm)

sekur commented 8 years ago

Ok, found the issue. It's because of recent optimization in how I've changed the YPATH parsing logic to take place within the XPATH instance. Here's the snippet from debug log I've just added:

$$$: check /foo:foo[key() = foo1]/bar in /foo:foo/.[key() = 'foo1']

The first XPATH is from the prop.path, the second XPATH is from the event filter.

The above two XPATH expressions are actually identical when evaluated but the XPath contains/locate logic doesn't detect them as the same. Let me apply a new XPath.normalize routine which will collapse the /foo:foo/.[key() = 'foo1'] to become /foo:foo[key() = 'foo1'] so that the compare routine will properly identify them as matching.

ramukima commented 8 years ago

I was reaching at similar conclusion after adding debug messages to 'xpath.locate' method. Also noticed 'foo1' vs foo in the following, hope thats not a problem for locate/contains

$$$: check /foo:foo[key() = foo1]/bar in /foo:foo/.[key() = 'foo1']
ramukima commented 8 years ago

Thanks for troubleshooting the issue. Should I expect a patch/npm build soon today so that I can test ?

sekur commented 8 years ago

Yes - I'm currently working on #32 so hopefully today's build will have a experimental/functional Store but there will be an update that includes this fix.

ramukima commented 8 years ago

Ok, Not sure if you noticed, in my event filter, I do not have any 'foo key' specified, so its not an XPATH, rather its a YPAH '/foo:foo/bar' instead of "/foo:foo[key()='foo1']/bar". I still expect my subscriber to be triggered.

sekur commented 8 years ago

Yes, that should be fine - the current XPATH contains method checks for entity path match and ignores predicates/filters. So /foo:foo/bar will be considered to be contained by /foo:foo[key() = 'foo1'/bar XPATH expression.

sekur commented 8 years ago

the fix will be available once a new version is published to NPM

sekur commented 8 years ago

Latest yang-js and yang-express published with new Store entity. Please refer to yang-express/example/petstore.coffee for new convention on setting up the dynamic routes.

You can attach events to Model instance as well as Store instance. They should have equivalent behavior although the preferred hook is now to listen on the Store.

ramukima commented 8 years ago

Looks like the changes to Store broke a few things... One I could find was -

I can no more curl on '/foo:foo/foo1/name' or any other attributes in the list through express. Store.in probably does NOT honor YPATH like' /foo:foo/foo1/name' and is expecting me to send an XPATH like '/foo:foo[key()='foo1']/name ?

ramukima commented 8 years ago

Here is an output from petstore example:

bash-4.3$ curl localhost:5000/pet
{
  "petstore:pet": [
    {
      "id": 1,
      "name": "happy",
      "tag": "friendly"
    },
    {
      "id": 2,
      "name": "boba",
      "tag": "hyper"
    }
  ]
}bash-4.3$ curl localhost:5000/pet/1
{
  "petstore:pet": {
    "id": 1,
    "name": "happy",
    "tag": "friendly"
  }
}bash-4.3$ curl localhost:5000/pet/1/id
Cannot GET /pet/1/id
bash-4.3$ curl localhost:5000/pet/1/name
Cannot GET /pet/1/name
bash-4.3$
sekur commented 8 years ago

Yes, I noticed that late last night - somehow leaf elements lost their magic. But /foo:foo/name should work (give you list of all "name" values in the list)? On Fri, Aug 26, 2016 at 9:00 AM Amit Kumar notifications@github.com wrote:

Looks like the changes to Store broke a few things... One I could find was

I can no more curl on /foo:foo/foo1/name or any other attributes in the list.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/corenova/yang-js/issues/33#issuecomment-242776253, or mute the thread https://github.com/notifications/unsubscribe-auth/AA6KbKCY7KXo58j8fdVt2NhIMju_gckJks5qjw2MgaJpZM4JtV86 .

Peter Lee Corenova Technologies +1 310 400 6450 peter@corenova.com

ramukima commented 8 years ago
bash-4.3$ curl localhost:5000/pet/name
[
  {
    "name": "happy"
  },
  {
    "name": "boba"
  }
]bash-4.3$
ramukima commented 8 years ago

If it helps, here is a difference between the output of those two invocations...

xpath: /pet[key() = '1']/name
content: [ { id: [Getter/Setter],
    name: [Getter/Setter],
    tag: [Getter/Setter] },
  { id: [Getter/Setter],
    name: [Getter/Setter],
    tag: [Getter/Setter] } ]
data: [{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]
data: [{"petstore:pet":[{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]}]
data: [{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]
data2: ["happy","boba"]
data: ["happy","boba"]
data2: []
data2: []
data2: []
xpath: /pet/name
content: [ { id: [Getter/Setter],
    name: [Getter/Setter],
    tag: [Getter/Setter] },
  { id: [Getter/Setter],
    name: [Getter/Setter],
    tag: [Getter/Setter] } ]
data: [{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]
data: [{"petstore:pet":[{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]}]
data: [{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]
data2: ["happy","boba"]
data2: ["happy","boba"]
data2: ["happy","boba"]
[restjson:petstore] calling GET on /pet/name
xpath: /pet/name
content: [ { id: [Getter/Setter],
    name: [Getter/Setter],
    tag: [Getter/Setter] },
  { id: [Getter/Setter],
    name: [Getter/Setter],
    tag: [Getter/Setter] } ]
data: [{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]
data: [{"petstore:pet":[{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]}]
data: [{"id":1,"name":"happy","tag":"friendly"},{"id":2,"name":"boba","tag":"hyper"}]
data2: ["happy","boba"]
data2: ["happy","boba"]
data2: ["happy","boba"]

The data is printed right from expression.litcoffee

console.log "data:", JSON.stringify(data)
    data = @source.construct.call this, data
    console.log "data2:", JSON.stringify(data)
sekur commented 8 years ago

BTW - you can get a fairly verbose debug output if you run with env 'yang_debug=1'

ramukima commented 8 years ago

Yeah, I noticed it in YangForge, somehow had forgotten, thanks for reminding.

sekur commented 8 years ago

Issue has been fixed and verified on yang-express. new packages published for yang-js@0.15.24 and yang-express@0.3.14.

The problem was introduced when I normalized the XPATH, I had to explicitly ensure that the predicate filters were run before proceeding to run the sub-xpath expressions.

ramukima commented 8 years ago

Perfect, thank you.