543888 - Places APIs design "skeleton"

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[David Dahl :ddahl]

Building on the PlacesQuery jsm ( bug 522572 ) - design a skeleton "mock object" that describes APIs for querying and creating place objects.

Comment 1

[David Dahl :ddahl]

from irc:

...
<mak> it is a waste of time, that's why i want you to design an API without thinking to current impl.
<mak> and LATER wrap it around current api
<ddahl> exactly what I wanted
<mak> so, i think i love the Places.entries.bookmarks, Places.entries.visits, Places.entries.filter, Places.entries.bookmarks.filter... why didn't you do that?
<ddahl> i wanted to think about this with no limitations
<ddahl> 1. i want to make the usage short and sweet
<ddahl> 2. you can name it anything you want: let places = new PlacesQuery()
<ddahl> places.find({}, callback)
<ddahl> thats SWEET
<ddahl> places.bookmark.create({}, callback)
<mak> Places.entries.bookmarks.create is sweet too, and even easier, you just include the module and already have the mafic Places object in scope
<ddahl> places.find({phrase: "foo", between: [new Date(), new Date()]})
<mak> Places.entries.filter({phrase:"foo"}) (both bookmarks and history)
<ddahl> places.find({phrase: "foo", between: [new Date(), new Date()], where: "everywhere"})
<ddahl> yeah
<ddahl> not specifying where is unified!
<mak> do you think your current impl is good enough?
<ddahl> nice
<ddahl> no
<mak> i think you can do better, because i see you have much better ideas
<ddahl> it needs more properties to sea4rch via
<mak> put them down, follow a scheme, win
<ddahl> yeah. I will, when I have time to go back to places work. url-classifier is kicking my ass
<mak> putting down an empty skeleton won't take a lot of time...just a commented object skeleton, and no impl

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

or at least analyze use cases like:
Places.entries.bookmark.add
Places.entries.bookmark.rename
Places.entries.bookmark.move
Places.entries.visit.add
Places.entries.visit.filter
so we can have an overview of what we will get and what objects are needed.

Comment 3

[David Dahl :ddahl]

we need to also figure out what kind of objects are returned and I think this API needs to require Paging! (user configurable of course)

using django as a "template", I would like the querying top look like this:

let myPlaces = Places.entries.filter({phrase: "foo.com", since: new Date(), limit: 100}, callback);

myPlaces is a generator that returns up to 100 records to the callback function each time next() is called or is iterated over.

myPlaces.next(); // returns a page of records to the callback

myPlaces.previous(); // reduces the offset and returns the previous page of 100

as far as rename is concerned, I think an even more general approach is better:

Places.entries.bookmark.get(123).save({title: "foo bar", tags: ["foo", "bar"]})

 ".get" gets a single object via 1 or more params. an exception is thrown if more than one record is returned

".filter" always returns an array-like object with extra properties like ".count" for total number of records found, which is lazily executed and cached

I'll do some more thinking and trawling of django's source as well as other projects before I really put something down.

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to comment #3)
> let myPlaces = Places.entries.filter({phrase: "foo.com", since: new Date(),
> limit: 100}, callback);
> 
> myPlaces is a generator that returns up to 100 records to the callback function
> each time next() is called or is iterated over.

sounds fine

> Places.entries.bookmark.get(123).save({title: "foo bar", tags: ["foo", "bar"]})
> 
>  ".get" gets a single object via 1 or more params. an exception is thrown if
> more than one record is returned

save or update? save sounds like being able to add even something that does not exist. btw minor thing.

> ".filter" always returns an array-like object with extra properties like
> ".count" for total number of records found, which is lazily executed and cached

are these objects live-updated or static?

Comment 5

[David Dahl :ddahl]

(In reply to comment #4)
> > Places.entries.bookmark.get(123).save({title: "foo bar", tags: ["foo", "bar"]})
> > 
> >  ".get" gets a single object via 1 or more params. an exception is thrown if
> > more than one record is returned
> 
> save or update? save sounds like being able to add even something that does not
> exist. btw minor thing.

I think what is implied here is that you have the place object's primary key so a save() would do an update, otherwise save() would do an insert.

> 
> > ".filter" always returns an array-like object with extra properties like
> > ".count" for total number of records found, which is lazily executed and cached
> 
> are these objects live-updated or static?

THAT is a GOOD question:)

perhaps the first WIP iteration would be static, informing how best to do a live-updatable version, which would be best of course.

Comment 6

[David Dahl :ddahl]

Attachment: WIP 0.0.0.0.1 api textfile from etherpad

Comment 7

[David Dahl :ddahl]

Attachment: WIP 0.0.0.0.2 api textfile from etherpad

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

The work we did on this was a nice start and i'll try to follow this approach. We will be able to discuss it further, it will most likely be more toward JetPack, while Places will just provide a simpler querying module (Bug 522572).
Discussion should be moved to the JetPack Bug 545700, then.