952310 - Create database schema for HomeProvider

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Data Providers, defect)

Description

[:Margaret Leibovic]

Currently database use is disabled in HomeListsContentProvider until we can come up with a v1 schema that we're happy with.

There's some discussion of schema in bug 941318, let's continue that discussion here.

Comment 1

[:Margaret Leibovic]

As part of this, we should make the current HomeListItems columns more generic. E.g., instead of "title" and "url", we should have "primary_text" and "secondary_text" (or something like that).

Comment 2

[:Margaret Leibovic]

There's a few different moving parts here, but we can use this bug to make sure we're representing the same schema in JS and Java.

Comment 3

[:Margaret Leibovic]

Also building this on top of my patch for bug 949039, which should be ready to land soon. We'll want the fake items to match the same schema anyway.

Comment 4

[:Margaret Leibovic]

Attachment: (Part 1) Rename HomeListsProvider -> HomeProvider

This is built on top of patches in the dependent bugs. If this is too annoying to review right now, you can also wait until those patches land (hopefully soon!).

Comment 5

[:Margaret Leibovic]

Comment on attachment 8364785 [details] [diff] [review]
(Part 1) Rename HomeListsProvider -> HomeProvider

Review of attachment 8364785 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/tests/robocop.ini
@@ +37,5 @@
>  skip-if = processor == "x86"
>  [testInputUrlBar]
>  [testJarReader]
>  [testLinkContextMenu]
> +# [testHomeListsProvider] # see bug 952310

I decided to punt on updating testHomeListsProvider for now, since I want to figure out how to test querying the content provider from Java, while populating the database from JS... but maybe that's too ambitious and we should just insert test data from Java as well. I can address this in a later patch in this bug.

Comment 6

[:Margaret Leibovic]

Attachment: (Part 2) Unify JS and Java representations of HomeProvider schema

This patch has some more renaming, but it's main objective is to get things into the same state everywhere.

Once again, this might be annoying to review before the dependent patches land :/

Comment 7

[Wesley Johnston (:wesj)]

Comment on attachment 8364788 [details] [diff] [review]
(Part 2) Unify JS and Java representations of HomeProvider schema

Review of attachment 8364788 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/db/HomeProvider.java
@@ +116,3 @@
>                      item.getString("url"),
> +                    item.getString("primary_text"),
> +                    item.getString("secondary_text")

I'm sorta coming back around to calling these title/description, to make it clear that one should be short and one can be long. Sorta matches how Android settings look...

Meh. I also hate getting distracted on naming.

Comment 8

[:Margaret Leibovic]

(In reply to Wesley Johnston (:wesj) from comment #7)
> Comment on attachment 8364788 [details] [diff] [review]
> (Part 2) Unify JS and Java representations of HomeProvider schema
> 
> Review of attachment 8364788 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/db/HomeProvider.java
> @@ +116,3 @@
> >                      item.getString("url"),
> > +                    item.getString("primary_text"),
> > +                    item.getString("secondary_text")
> 
> I'm sorta coming back around to calling these title/description, to make it
> clear that one should be short and one can be long. Sorta matches how
> Android settings look...
> 
> Meh. I also hate getting distracted on naming.

I don't think it's a distraction, since this is an API that will be exposed to developers.

Calling these things title/description to match Android conventions sounds like a reasonable proposal. I think we just need to make sure we don't call them title/url, like we currently do, since right now our UI always assumes list items correspond to pages. (However, we should still have a url field that corresponds to an action, just not a user-visible thing.)

Comment 9

[:Margaret Leibovic]

Attachment: (Part 2) Schema update

This updates the schema to match what we talked about this morning.

I also verified locally that testHomeProvider still passes.

Comment 10

[Lucas Rocha (:lucasr)]

Comment on attachment 8364785 [details] [diff] [review]
(Part 1) Rename HomeListsProvider -> HomeProvider

Review of attachment 8364785 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

Comment 11

[Lucas Rocha (:lucasr)]

Comment on attachment 8366129 [details] [diff] [review]
(Part 2) Schema update

Review of attachment 8366129 [details] [diff] [review]:
-----------------------------------------------------------------

Cool.

Comment 12

[:Margaret Leibovic]

https://hg.mozilla.org/integration/fx-team/rev/f88683073c1e
https://hg.mozilla.org/integration/fx-team/rev/629b52808ae1

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f88683073c1e
https://hg.mozilla.org/mozilla-central/rev/629b52808ae1