Should extraData contain the old version of an item when a 'modify' notification occurs?

[Robin Wilson]

Hi all,

I have a bit of a strange problem - although I may be doing something wrong.

As far as I can see from the Zotero source code, when a notification is posted for a change in an item (using Zotero.Notifier.notify), the extraData field is a dictionary/hash (or a list of hashes) with a 'old' key, which contains the Item's data before the modification took place. Is that correct? I'm trying to get access to this 'old' data, as I need it as part of my extension, but I can't seem to get it.

I have an extension which has the following code:

notifierCallback: {

   notify: function(event, type, ids, extraData) {

   dump("Event:\t" + event + "\t" + type + "\t" + ids + "\t" + JSON.stringify(extraData) + "\n");

}

When I run Zotero with this extension and modify an item a couple of times and then delete it I get the following output:

Event: modify item 224 {}

Event: modify item 249 {}

Event: trash item 249 {}

That is, the extraData parameter doesn't seem to have anything in it!

Is this how it should be? Am I doing something wrong in the way I try and get the extraData? Or is there a bug somewhere (in my code or in Zotero itself)?

Cheers,

Robin

[Robin Wilson]

Hi all,

I've been doing some more investigation of this over the last few days and just can't seem to find any way to get anything out of the extraData parameter. I'm wondering whether this a bug in my code somewhere, or a bug in Zotero somewhere.

Does anyone have any suggestions I can try to see where the problem lies?

Happy (belated) Christmas and (early) New Year,

Robin

[Frank Bennett]

On Sunday, December 30, 2012 2:41:01 AM UTC+9, Robin Wilson wrote:

Hi all,

I've been doing some more investigation of this over the last few days and just can't seem to find any way to get anything out of the extraData parameter. I'm wondering whether this a bug in my code somewhere, or a bug in Zotero somewhere.

Does anyone have any suggestions I can try to see where the problem lies?

It looks like this is a bug in Zotero. In chrome/content/zotero/xpcom/notifier.js, events are pooled in a _queue object when a transaction is in progress, for batch execution when the transaction is committed. The extraData on an event is set in the queue at line 139:

  https://github.com/zotero/zotero/blob/master/chrome/content/zotero/xpcom/notifier.js#L139

On a simple test, the keys for setting an extraData segment show up as: "item" "modify" "old", and the object contains the original data stored on the item.

The extraData segment is read off the queue at line 260:

  https://github.com/zotero/zotero/blob/master/chrome/content/zotero/xpcom/notifier.js#L260

The keys called in the same test read off as: "item" "modify" "46", and the object returns undefined.

I don't know the code well enough to propose a fix -- possibly the object at line 139 might contain data for multiple objects (?), and so would need to be broken out for storage on individual item ID segments in the queue. But the absence of extraData when the event fires does not seem to be intended.

Frank



 

[Frank Bennett]

I've filed a pull request on the Zotero 3.0 branch that should fix this:

  https://github.com/zotero/zotero/pull/219

We'll see how it goes.

Frank

[Frank Bennett]

The initial pull request pointed at some early point 3.0 branch development, and wanted to apply 200+ patches to the source. I've refiled. The new URL is:

  https://github.com/zotero/zotero/pull/220

Sorry for any confusion.

Frank

[Robin Wilson]

Frank,

Thank you very much - I was assuming I'd done something silly (as usual), so I'm glad to know that it wasn't me. The fact that extraData should contain something is also very good because it means the extension I'm trying to write is actually possible (hooray!).

I've never actually dealt with running Zotero itself from the source, but I'll try and apply your patch and then run it.

Best regards,

Robin

[Frank Bennett]

As an alternative you could just install the MLZ XPI, which now
includes the patch.

http://citationstylist.org

Frank

> --
> You received this message because you are subscribed to the Google Groups
> "zotero-dev" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/zotero-dev/-/Tygefhafs94J.
> To post to this group, send email to zoter...@googlegroups.com.
> To unsubscribe from this group, send email to
> zotero-dev+...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/zotero-dev?hl=en.

[Robin Wilson]

Frank,

I can confirm that this fix does fix the problem - the extraData field is now there and contains what I thought it should contain. I managed to apply the patch to the Zotero source using git, and then got Firefox to load it from source.

Thanks for letting me know about MLZ - I didn't even realise that existed. What is its status with regards to 'original Zotero' - are they likely to be merged at some point?

Also, do you (or anyone else?) know when this fix is likely to be merged into the main Zotero code and a new XPI produced? It won't be much use if I release an extension for Zotero that requires a bug fix version of Zotero which no-one can get an XPI of!

Cheers,

Robin

[Frank Bennett]

On Sun, Dec 30, 2012 at 9:27 PM, Robin Wilson <ro...@rtwilson.com> wrote:
> Frank,
>

> I can confirm that this fix does fix the problem - the extraData field is
> now there and contains what I thought it should contain. I managed to apply
> the patch to the Zotero source using git, and then got Firefox to load it
> from source.
>
> Thanks for letting me know about MLZ - I didn't even realise that existed.
> What is its status with regards to 'original Zotero' - are they likely to be
> merged at some point?

https://groups.google.com/forum/?fromgroups=#!topic/zotero-dev/jH2WxMyjSPU
http://www.imdb.com/title/tt0043249/

:-)

Frank

> https://groups.google.com/d/msg/zotero-dev/-/F_ufJ-Dl2-UJ.

[Robin Wilson]

Ah yes - I should have looked for that thread first.

Is there anyone particular I should contact to ask about when the bug fix will be officially released into the XPI?

Cheers,

Robin

[Frank Bennett]

On Sunday, December 30, 2012 9:54:11 PM UTC+9, Robin Wilson wrote:

Ah yes - I should have looked for that thread first.

I added the smiley to let the team know that I meant the second link in good humour -- it's an anti-sarcasm smiley.
 

Is there anyone particular I should contact to ask about when the bug fix will be officially released into the XPI?

Dan and Simon will read all traffic on zotero-dev, so the question is already up. It might take some time for a response to emerge -- they are (we hope) enjoying a well-deserved holiday.

Frank

 

[Dan Stillman]

On 12/30/12 6:10 AM, Robin Wilson wrote:
> The fact that extraData should contain something is also very good
> because it means the extension I'm trying to write is actually
> possible (hooray!).

Could you elaborate a bit on what you're using this for? I'm not sure
when this broke, but the extraData parameter has always been a bit
problematic, so I might want to take the opportunity of this being
broken to devise something better. (And if not, thanks to Frank for the
quick patch.)

[Robin Wilson]

Of course - if there's a possibility of replacing it with something else then it'd be good to know before I write all of my code to use the extraData parameter!

I am currently re-writing my AutoZotBib (see www.rtwilson.com/academic/autozotbib) extension from scratch. AutoZotBib is designed to automatically synchronise changes in Zotero to a BibTeX file, so that Zotero can easily be used with LaTeX documents. The previous (ie. currently available) version did this in a very naive way - basically whenever anything changed it exported all Zotero items to the BibTeX file. This worked fine for small Zotero libraries, but got *very* slow for larger libraries.

The new version does things in a far more intelligent way - it looks at what has changed, and then removes the relevant entries from the BibTeX file and exports anything that needs exporting (rather than exporting all items). There is a bit more to it than that, as due to the way that BibTeX keys work, we need to delete and then re-export all items with a certain first-author/year combination whenever one of them changes (this is the only way of doing it so that it doesn't break things with BibTeX). Anyway, as part of this I need to look at all 'modify' events and see whether the author and/or year was changed - and that's what I'm using extraData for.

So, basically I'm just using the 'old' field of extraData to see what has changed in the 'modify' event, and so I can deal with things properly. In the future I'd like to make things even more efficient by only processing the event at all if one of the BibTeX relevant fields (such as title, creators, date, journalTitle etc - and not including things like attachments, related items, tags etc) has changed.

Does that help?

Cheers,

Robin

[Robin Wilson]

Hi all,

Has there been any progress on deciding what will happen to extraData? Obviously there's not much point in my writing code to use extraData if the issues with it aren't going to be fixed, and instead it will be replaced with something else - but I was hoping to release my extension in the not-too-distant future.

Any thoughts?

Cheers,

Robin

[Dan Stillman]

On 12/31/12 2:18 PM, Robin Wilson wrote:
> The new version does things in a far more intelligent way - it looks
> at what has changed, and then removes the relevant entries from the
> BibTeX file and exports anything that needs exporting (rather than
> exporting all items). There is a bit more to it than that, as due to
> the way that BibTeX keys work, we need to delete and then re-export
> all items with a certain first-author/year combination whenever one of
> them changes (this is the only way of doing it so that it doesn't
> break things with BibTeX). Anyway, as part of this I need to look at
> all 'modify' events and see whether the author and/or year was changed
> - and that's what I'm using extraData for.
>
> So, basically I'm just using the 'old' field of extraData to see what
> has changed in the 'modify' event, and so I can deal with things
> properly. In the future I'd like to make things even more efficient by
> only processing the event at all if one of the BibTeX relevant fields
> (such as title, creators, date, journalTitle etc - and not including
> things like attachments, related items, tags etc) has changed.

Haven't decided on this yet, but it sounds like it'd be sufficient for
the 'modify' event to include just the fields that changed (with their
values) rather than the entire previous object?

I haven't tested this in a while, but I suspect serializing the entire
object to 'old' is actually slowing some parts of Zotero down quite a
bit. There shouldn't be much of a performance penalty to store just the
previous values of the fields that changed, though.

Zotero extensions also then wouldn't need to compare the two objects
themselves to see what changed.

[Robin Wilson]

That sounds like a very sensible solution - it would save me doing the comparisons manually. If the extraData could just provide a hash/dictionary with keys for anything that has changed then it would be really simple to check if certain keys are present, meaning that the fields data had changed.

I'll probably wait to implement this in my code until the new version of the extraData code is available, otherwise I'd just have to write some of the comparison stuff myself (and probably far less robustly than the Zotero code would be!).

Cheers,

Robin

[Dan Stillman]

On 1/16/13 6:09 PM, Robin Wilson wrote:

On Monday, 14 January 2013 09:38:40 UTC, Dan Stillman wrote:

Haven't decided on this yet, but it sounds like it'd be sufficient for
the 'modify' event to include just the fields that changed (with their
values) rather than the entire previous object?

I haven't tested this in a while, but I suspect serializing the entire
object to 'old' is actually slowing some parts of Zotero down quite a
bit. There shouldn't be much of a performance penalty to store just the
previous values of the fields that changed, though.

Zotero extensions also then wouldn't need to compare the two objects
themselves to see what changed.

That sounds like a very sensible solution - it would save me doing the comparisons manually. If the extraData could just provide a hash/dictionary with keys for anything that has changed then it would be really simple to check if certain keys are present, meaning that the fields data had changed.

OK, I was able to get this in for you in time for 3.0.12. See https://github.com/zotero/zotero/commit/1a0849e48 for details. It's available now in the latest 3.0 branch dev XPI.

It'd be great if you could test this out to make sure it does what you need and others could test this out to make sure it doesn't break Zotero in unexpected ways. (Any item additions/modifications/deletions could in theory be affected, though I tested it fairly thoroughly.)

I could pretty easily alter this to include both old and new versions of modified values, which I imagine would make some uses a bit tidier, but it shouldn't really be necessary given that the current data is always available in the existing object. Let me know what you think.

- Dan