909454 - Allow filters for mozContacts.getAll

Status: RESOLVED INVALID

(Core Graveyard :: DOM: Contacts, defect, P3)

Description

[Kevin Grandon :kgrandon]

Currently filters are only used for find() calls. We would really like to see filters for mozContacts.getAll for some performance investigation. This bug to see if it's possible and how much work that it would take to implement. For example, we are trying to make a call like:

navigator.mozContacts.getAll({
  sortBy: 'givenName',
  sortOrder: 'descending',
  filterBy: ['familyName'],
  filterValue: 'Z',
  filterOp: 'startsWith'
});

Comment 1

[Kevin Grandon :kgrandon]

Gregor - if you have any tips about where to get started with this it would be helpful. I might have someone on our team try to tackle this if you are busy. Thanks!

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Kevin and I were looking at the code it appears this might work if we change nsIDOMContactManager.idl so that getAll() uses nsIContactFindOptions.

This should allow the filter options to pass to getAll(), which will be applied during the non-cached query.

Kevin just verified that this works in a modified firefox nightly and the simulator.

One potential issue would be if we get too many cached results that never get cleaned up.

Thoughts?

Comment 3

[Gregor Wagner [:gwagner]]

We should just limit the number of cache-entries and overwrite old values. Otherwise it should just work.

Comment 4

[Reuben Morais [:reuben]]

So you want find() with cursors? That should be simple, yes. Caching results keyed on the find options is going to be pretty much useless, though.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Reuben Morais [:reuben] from comment #4)
> So you want find() with cursors? That should be simple, yes. Caching results
> keyed on the find options is going to be pretty much useless, though.

Well, our thought was to all filtering options in getAll() instead of adding cursors to find().  The caching was only mentioned since getAll() does caching.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Sorry, to *add* filtering options to getAll().

Comment 7

[Reuben Morais [:reuben]]

(In reply to Ben Kelly [:bkelly] from comment #5)
> Well, our thought was to all filtering options in getAll() instead of adding
> cursors to find().  The caching was only mentioned since getAll() does
> caching.

Without mentioning the obvious problem of appropriate naming, I don't like the idea of getAll having two very different (speed and memory usage) behaviors depending on what options are passed.

Comment 8

[Kevin Grandon :kgrandon]

I think that this bug is no longer valid with the transition plan. Please re-open if you think we should still do this.