Merge into sync-ui : sync-ui : Code : Tomdroid

Status:	Merged
Merged at revision:	239
Proposed branch:	lp:~salgado/tomdroid/sync-ui
Merge into:	lp:~tomdroid-dev/tomdroid/sync-ui
Diff against target:	289 lines (+149/-9) 7 files modified .classpath (+1/-0) AndroidManifest.xml (+3/-0) src/org/tomdroid/Note.java (+17/-1) src/org/tomdroid/NoteManager.java (+15/-6) src/org/tomdroid/NoteProvider.java (+4/-2) tests/org/tomdroid/NoteManagerTest.java (+74/-0) tests/org/tomdroid/NoteTest.java (+35/-0)
To merge this branch:	bzr merge lp:~salgado/tomdroid/sync-ui
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Rodja (community)	2010-08-25	Approve on 2010-09-26
Olivier Bilodeau	2010-08-12	Pending
Review via email: mp+32509@code.launchpad.net

lp:~salgado/tomdroid/sync-ui updated on 2010-08-16

237. By Guilherme Salgado on 2010-08-16: Get rid of lots of unnecessary boilerplate created by Eclipse when I created a separate test project
238. By Guilherme Salgado on 2010-08-16: Fix a Null pointer error when loading a note JSON with no tags.

Revision history for this message

Olivier Bilodeau (plaxx) wrote on 2010-08-25:

#

Rodja, please review this and integrate it in sync-ui if you don't see any problems with it.

Revision history for this message

Rodja (trappe) wrote on 2010-08-25:

#

It is great to get rid of the templates but...

1. The tests are not passing if there are already notes in the database (see two-way-branch for an setUp/tearDown example).

2. Rather then introducing a new member to flag a note as "template" I would suggest to seach for the template tag each time isNotebookTemplate() is called. Performance is not an issue, but later refactorings will be harder with each new member variable.

3. Similar to the second remark, I would sooner see a row for all the tags changing the database when ever we feel about supporting a new tag.

Revision history for this message

Guilherme Salgado (salgado) wrote on 2010-08-25:

#

On Wed, 2010-08-25 at 20:26 +0000, Rodja wrote:
> It is great to get rid of the templates but...
>
> 1. The tests are not passing if there are already notes in the
> database (see two-way-branch for an setUp/tearDown example).
>

So, IIUC, you suggest that I just reset the database as part of
setUp/tearDown? That sounds fine to me, but the sync-ui branch doesn't
have LocalStorage (which you use in the two-way-sync branch to reset the
DB), so I'd have to do it manually for now. Something like:

activity.getContentResolver().delete(Tomdroid.CONTENT_URI, null, null);
Preferences.putLong(Preferences.Key.LATEST_SYNC_REVISION, 0);

Is that ok with you?

> 2. Rather then introducing a new member to flag a note as "template" I
> would suggest to seach for the template tag each time
> isNotebookTemplate() is called. Performance is not an issue, but later
> refactorings will be harder with each new member variable.

It's not because of performance that I made it a flag -- it's because I
think we should avoid spending processor cycles unnecessarily, to not
drain people's batteries.

My thinking was that if you have a long list of notes and you're
switching back and forth between any note and the notes list, tomdroid
would be doing a good job at draining your battery. To be honest I have
no idea how much power it'd spend while doing that, but it's easily
avoidable so I thought it was worth it. I'd be OK with dropping the new
flag as you suggest, if you feel it's not worth.

>
> 3. Similar to the second remark, I would sooner see a row for all the
> tags changing the database when ever we feel about supporting a new
> tag.

Do you mean you think we shouldn't be changing the DB schema often? I
don't think that's a problem (thanks to the framework's ability of
detecting when schema upgrades are needed and doing the migration
semi-automatically for us), specially when it's as early in the
lifecycle of the project as we are now.

--
Guilherme Salgado <https://launchpad.net/~salgado>

On Wed, 2010-08-25 at 20:26 +0000, Rodja wrote:
> It is great to get rid of the templates but...
> 
> 1. The tests are not passing if there are already notes in the
> database (see two-way-branch for an setUp/tearDown example).
>

So, IIUC, you suggest that I just reset the database as part of
setUp/tearDown?  That sounds fine to me, but the sync-ui branch doesn't
have LocalStorage (which you use in the two-way-sync branch to reset the
DB), so I'd have to do it manually for now.  Something like:

activity.getContentResolver().delete(Tomdroid.CONTENT_URI, null, null);
  Preferences.putLong(Preferences.Key.LATEST_SYNC_REVISION, 0);

Is that ok with you?

> 2. Rather then introducing a new member to flag a note as "template" I
> would suggest to seach for the template tag each time
> isNotebookTemplate() is called. Performance is not an issue, but later
> refactorings will be harder with each new member variable.

It's not because of performance that I made it a flag -- it's because I
think we should avoid spending processor cycles unnecessarily, to not
drain people's batteries.

My thinking was that if you have a long list of notes and you're
switching back and forth between any note and the notes list, tomdroid
would be doing a good job at draining your battery. To be honest I have
no idea how much power it'd spend while doing that, but it's easily
avoidable so I thought it was worth it.  I'd be OK with dropping the new
flag as you suggest, if you feel it's not worth.

> 
> 3. Similar to the second remark, I would sooner see a row for all the
> tags changing the database when ever we feel about supporting a new
> tag.

Do you mean you think we shouldn't be changing the DB schema often?  I
don't think that's a problem (thanks to the framework's ability of
detecting when schema upgrades are needed and doing the migration
semi-automatically for us), specially when it's as early in the
lifecycle of the project as we are now.

-- 
Guilherme Salgado <https://launchpad.net/~salgado>

Revision history for this message

Rodja (trappe) wrote on 2010-08-28:

#

> On Wed, 2010-08-25 at 20:26 +0000, Rodja wrote:
> > It is great to get rid of the templates but...
> >
> > 1. The tests are not passing if there are already notes in the
> > database (see two-way-branch for an setUp/tearDown example).
> >
>
> So, IIUC, you suggest that I just reset the database as part of
> setUp/tearDown? That sounds fine to me, but the sync-ui branch doesn't
> have LocalStorage (which you use in the two-way-sync branch to reset the
> DB), so I'd have to do it manually for now. Something like:
>
> activity.getContentResolver().delete(Tomdroid.CONTENT_URI, null, null);
> Preferences.putLong(Preferences.Key.LATEST_SYNC_REVISION, 0);
>
> Is that ok with you?

Yes.

> > 2. Rather then introducing a new member to flag a note as "template" I
> > would suggest to seach for the template tag each time
> > isNotebookTemplate() is called. Performance is not an issue, but later
> > refactorings will be harder with each new member variable.
>
> It's not because of performance that I made it a flag -- it's because I
> think we should avoid spending processor cycles unnecessarily, to not
> drain people's batteries.
>
> My thinking was that if you have a long list of notes and you're
> switching back and forth between any note and the notes list, tomdroid
> would be doing a good job at draining your battery.

Is it? Have you tested? I would expect Android to keep the notes list Activity running while showing a Note; and calling onResume when switching back. But guessing is never good when talking about optimizations.

> To be honest I have
> no idea how much power it'd spend while doing that, but it's easily
> avoidable so I thought it was worth it. I'd be OK with dropping the new
> flag as you suggest, if you feel it's not worth.

Power consumtion is not easily measureable. But when looking on the battery usage graph from my Motorola Droid the big consumers are wifi, display, Maps and phone. I prefer having a clean code base which is easier to change in the future than "pollute" it with optimizations.

> > 3. Similar to the second remark, I would sooner see a row for all the
> > tags changing the database when ever we feel about supporting a new
> > tag.
>
> Do you mean you think we shouldn't be changing the DB schema often? I
> don't think that's a problem (thanks to the framework's ability of
> detecting when schema upgrades are needed and doing the migration
> semi-automatically for us), specially when it's as early in the
> lifecycle of the project as we are now.

True. I'm not against changing the data base scheme in general. But sooner or later we will introduce a column to store the tags of a note. Should we remove the system:template tag befor putting the others into that column?

I've no insights about Tomboy server API decisions of having "open-on-startup" and "pinned" as extra flags and "system:template" inside the tag list, but would recommend to stick as close as possible with the already provided scheme to avoid complicated data renderings and difficulties for other Tomboy developers.

> On Wed, 2010-08-25 at 20:26 +0000, Rodja wrote:
> > It is great to get rid of the templates but...
> >
> > 1. The tests are not passing if there are already notes in the
> > database (see two-way-branch for an setUp/tearDown example).
> >
> 
> So, IIUC, you suggest that I just reset the database as part of
> setUp/tearDown?  That sounds fine to me, but the sync-ui branch doesn't
> have LocalStorage (which you use in the two-way-sync branch to reset the
> DB), so I'd have to do it manually for now.  Something like:
> 
>   activity.getContentResolver().delete(Tomdroid.CONTENT_URI, null, null);
>   Preferences.putLong(Preferences.Key.LATEST_SYNC_REVISION, 0);
> 
> Is that ok with you?

Yes.

> > 2. Rather then introducing a new member to flag a note as "template" I
> > would suggest to seach for the template tag each time
> > isNotebookTemplate() is called. Performance is not an issue, but later
> > refactorings will be harder with each new member variable.
> 
> It's not because of performance that I made it a flag -- it's because I
> think we should avoid spending processor cycles unnecessarily, to not
> drain people's batteries.
> 
> My thinking was that if you have a long list of notes and you're
> switching back and forth between any note and the notes list, tomdroid
> would be doing a good job at draining your battery.

Is it? Have you tested? I would expect Android to keep the notes list Activity running while showing a Note; and calling onResume when switching back. But guessing is never good when talking about optimizations.

> To be honest I have 
> no idea how much power it'd spend while doing that, but it's easily
> avoidable so I thought it was worth it.  I'd be OK with dropping the new
> flag as you suggest, if you feel it's not worth.

Power consumtion is not easily measureable. But when looking on the battery usage graph from my Motorola Droid the big consumers are wifi, display, Maps and phone. I prefer having a clean code base which is easier to change in the future than "pollute" it with optimizations.

> > 3. Similar to the second remark, I would sooner see a row for all the
> > tags changing the database when ever we feel about supporting a new
> > tag.
> 
> Do you mean you think we shouldn't be changing the DB schema often?  I
> don't think that's a problem (thanks to the framework's ability of
> detecting when schema upgrades are needed and doing the migration
> semi-automatically for us), specially when it's as early in the
> lifecycle of the project as we are now.

True. I'm not against changing the data base scheme in general. But sooner or later we will introduce a column to store the tags of a note. Should we remove the system:template tag befor putting the others into that column?

I've no insights about Tomboy server API decisions of having "open-on-startup" and "pinned" as extra flags and "system:template" inside the tag list, but would recommend to stick as close as possible with the already provided scheme to avoid complicated data renderings and difficulties for other Tomboy developers.

Revision history for this message

Guilherme Salgado (salgado) wrote on 2010-08-31:

#

On Sat, 2010-08-28 at 08:10 +0000, Rodja wrote:
> > On Wed, 2010-08-25 at 20:26 +0000, Rodja wrote:
> > > It is great to get rid of the templates but...
> > >
> > > 1. The tests are not passing if there are already notes in the
> > > database (see two-way-branch for an setUp/tearDown example).
> > >
> >
> > So, IIUC, you suggest that I just reset the database as part of
> > setUp/tearDown? That sounds fine to me, but the sync-ui branch doesn't
> > have LocalStorage (which you use in the two-way-sync branch to reset the
> > DB), so I'd have to do it manually for now. Something like:
> >
> > activity.getContentResolver().delete(Tomdroid.CONTENT_URI, null, null);
> > Preferences.putLong(Preferences.Key.LATEST_SYNC_REVISION, 0);
> >
> > Is that ok with you?
>
> Yes.
>

Cool, I've done that.

> > > 2. Rather then introducing a new member to flag a note as "template" I
> > > would suggest to seach for the template tag each time
> > > isNotebookTemplate() is called. Performance is not an issue, but later
> > > refactorings will be harder with each new member variable.
> >

One issue with the solution you suggest is that, in the current
implementation, we only have access to the tags (which is in the JSON
representation sent by the server) when creating a new Note row in the
database, so I'd have to store all the tags in the DB somehow. Probably
in a custom format as the web sync gives me JSON and the file sync gives
XML.

Does that sound ok to you? Do you have any suggestions as to which
format to use for the tags in the DB? Ideally I think it should be in a
separate table (linked to the Note table) with one column for the key
and another for the value.

> > It's not because of performance that I made it a flag -- it's because I
> > think we should avoid spending processor cycles unnecessarily, to not
> > drain people's batteries.
> >
> > My thinking was that if you have a long list of notes and you're
> > switching back and forth between any note and the notes list, tomdroid
> > would be doing a good job at draining your battery.
>
> Is it? Have you tested? I would expect Android to keep the notes list
> Activity running while showing a Note; and calling onResume when
> switching back. But guessing is never good when talking about
> optimizations.

You're most likely right here. I didn't know this could happen as I
don't know much about Android and this is the first time I write some
code for it.

>
> > To be honest I have
> > no idea how much power it'd spend while doing that, but it's easily
> > avoidable so I thought it was worth it. I'd be OK with dropping the new
> > flag as you suggest, if you feel it's not worth.
>
> Power consumtion is not easily measureable. But when looking on the
> battery usage graph from my Motorola Droid the big consumers are wifi,
> display, Maps and phone. I prefer having a clean code base which is
> easier to change in the future than "pollute" it with optimizations.

Fair enough.

On Sat, 2010-08-28 at 08:10 +0000, Rodja wrote:
> > On Wed, 2010-08-25 at 20:26 +0000, Rodja wrote:
> > > It is great to get rid of the templates but...
> > >
> > > 1. The tests are not passing if there are already notes in the
> > > database (see two-way-branch for an setUp/tearDown example).
> > >
> > 
> > So, IIUC, you suggest that I just reset the database as part of
> > setUp/tearDown?  That sounds fine to me, but the sync-ui branch doesn't
> > have LocalStorage (which you use in the two-way-sync branch to reset the
> > DB), so I'd have to do it manually for now.  Something like:
> > 
> >   activity.getContentResolver().delete(Tomdroid.CONTENT_URI, null, null);
> >   Preferences.putLong(Preferences.Key.LATEST_SYNC_REVISION, 0);
> > 
> > Is that ok with you?
> 
> Yes.
>

Cool, I've done that.

> > > 2. Rather then introducing a new member to flag a note as "template" I
> > > would suggest to seach for the template tag each time
> > > isNotebookTemplate() is called. Performance is not an issue, but later
> > > refactorings will be harder with each new member variable.
> >

One issue with the solution you suggest is that, in the current
implementation, we only have access to the tags (which is in the JSON
representation sent by the server) when creating a new Note row in the
database, so I'd have to store all the tags in the DB somehow.  Probably
in a custom format as the web sync gives me JSON and the file sync gives
XML.

Does that sound ok to you?  Do you have any suggestions as to which
format to use for the tags in the DB?  Ideally I think it should be in a
separate table (linked to the Note table) with one column for the key
and another for the value.

> > It's not because of performance that I made it a flag -- it's because I
> > think we should avoid spending processor cycles unnecessarily, to not
> > drain people's batteries.
> > 
> > My thinking was that if you have a long list of notes and you're
> > switching back and forth between any note and the notes list, tomdroid
> > would be doing a good job at draining your battery.
> 
> Is it? Have you tested? I would expect Android to keep the notes list
> Activity running while showing a Note; and calling onResume when
> switching back. But guessing is never good when talking about
> optimizations.

You're most likely right here.  I didn't know this could happen as I
don't know much about Android and this is the first time I write some
code for it.

> 
> > To be honest I have 
> > no idea how much power it'd spend while doing that, but it's easily
> > avoidable so I thought it was worth it.  I'd be OK with dropping the new
> > flag as you suggest, if you feel it's not worth.
> 
> Power consumtion is not easily measureable. But when looking on the
> battery usage graph from my Motorola Droid the big consumers are wifi,
> display, Maps and phone. I prefer having a clean code base which is
> easier to change in the future than "pollute" it with optimizations.

Fair enough.

lp:~salgado/tomdroid/sync-ui updated on 2010-08-31

239. By Guilherme Salgado on 2010-08-31: A couple tweaks suggested by reviewer
240. By Guilherme Salgado on 2010-08-31: merge lp:~tomdroid-developers/sync-ui

Revision history for this message

Olivier Bilodeau (plaxx) wrote on 2010-09-03:

#

Hi guys,

I'm a bit concerned when hearing about the XML vs JSON tags.. In the end, aren't they both strings (ex: system:template or system:notebook:bookname)? Wait, as I write this (noticing the <bookname> related to the notebook addin) I see tons of potential problems with a rigid SQL approach..

- A multi to multi SQL approach would be ugly as hell (one entry per <bookname>) but then what about nested xml in the tag or attributes in the xml node.. where would they end up?
- A list of enabled/disabled tags represented in a row of a note as a packed binary would not handle additional data either.. and would be more coupled with Tomdroid's version..

What do we do here? <buzzword>Schema-less database?</buzzword> I'll let you guys think about that. Do not hesitate to bring this to the -dev list if you feel it deserves to.

On the performance / potential power-saving topic:
Until we have benchmark evidence showing it's not efficient I would opt for simpler code and less db maintenance. I don't plan to do auto db upgrade for 0.4 but just drop the db because no notes can be created locally yet. Someone could work on that if required though.

However, related to your concern Guilherme, to improve things we should save and restore the list of notes from the main activity whenever we can in the lifecycle. This would improve a case or two where it is dropped. Actually I should write that down in doc/dev/TODO...

Thanks for the work guys!

Revision history for this message

Rodja (trappe) wrote on 2010-09-04:

#

I'm by no means a database guy, so i've no clue how to solve the tag storing problem. But a quick Google search (http://lmgtfy.com/?q=storing+tags+in+relational+database) reveals tons of howtos and insights.

I suggest to use the simplest possible approach for a start (this may be storing all tags comma seperated in a single column) and optimizing it in a seperate branch. This merge is about removing the templates from the main list, not getting a high performance tag storage.

Revision history for this message

Guilherme Salgado (salgado) wrote on 2010-09-17:

#

On Fri, 2010-09-03 at 04:29 +0000, Olivier Bilodeau wrote:
> Hi guys,
>
> I'm a bit concerned when hearing about the XML vs JSON tags.. In the
> end, aren't they both strings (ex: system:template or
> system:notebook:bookname)? Wait, as I write this (noticing the

Yes, they're the same thing; I don't know what I had in mind when I
wrote that.

> <bookname> related to the notebook addin) I see tons of potential
> problems with a rigid SQL approach..

Well, that could be extrapolated to the rest of tomdroid, no? I mean,
is a relational database the right thing to store tomboy notes?

>
> - A multi to multi SQL approach would be ugly as hell (one entry per
> <bookname>) but then what about nested xml in the tag or attributes in
> the xml node.. where would they end up?
> - A list of enabled/disabled tags represented in a row of a note as a
> packed binary would not handle additional data either.. and would be
> more coupled with Tomdroid's version..
>
> What do we do here? <buzzword>Schema-less database?</buzzword> I'll
> let you guys think about that. Do not hesitate to bring this to the
> -dev list if you feel it deserves to.

We could use CouchDB (http://www.couch.io/android) for 2.1 and 2.2, but
not for previous versions. Do you know of any alternatives?

> On the performance / potential power-saving topic:
> Until we have benchmark evidence showing it's not efficient I would
> opt for simpler code and less db maintenance. I don't plan to do auto
> db upgrade for 0.4 but just drop the db because no notes can be
> created locally yet. Someone could work on that if required though.

IMHO, that's not necessary as long as notes are read-only in tomdroid.

I'd really like to keep working on this and do some experiments, but I
won't be able to do that before November as I'm recovering from a
surgery and have my left arm in a sling, so unless anybody else wants to
move this forward, I think it won't be included in 0.4

--
Guilherme Salgado <https://launchpad.net/~salgado>

On Fri, 2010-09-03 at 04:29 +0000, Olivier Bilodeau wrote:
> Hi guys,
> 
> I'm a bit concerned when hearing about the XML vs JSON tags.. In the
> end, aren't they both strings (ex: system:template or
> system:notebook:bookname)? Wait, as I write this (noticing the

Yes, they're the same thing; I don't know what I had in mind when I
wrote that.

>  <bookname> related to the notebook addin) I see tons of potential
> problems with a rigid SQL approach..

Well, that could be extrapolated to the rest of tomdroid, no?  I mean,
is a relational database the right thing to store tomboy notes?

> 
> - A multi to multi SQL approach would be ugly as hell (one entry per
> <bookname>) but then what about nested xml in the tag or attributes in
> the xml node.. where would they end up?
> - A list of enabled/disabled tags represented in a row of a note as a
> packed binary would not handle additional data either.. and would be
> more coupled with Tomdroid's version..
> 
> What do we do here? <buzzword>Schema-less database?</buzzword> I'll
> let you guys think about that. Do not hesitate to bring this to the
> -dev list if you feel it deserves to.

We could use CouchDB (http://www.couch.io/android) for 2.1 and 2.2, but
not for previous versions.  Do you know of any alternatives?

> On the performance / potential power-saving topic:
> Until we have benchmark evidence showing it's not efficient I would
> opt for simpler code and less db maintenance. I don't plan to do auto
> db upgrade for 0.4 but just drop the db because no notes can be
> created locally yet. Someone could work on that if required though.

IMHO, that's not necessary as long as notes are read-only in tomdroid.

I'd really like to keep working on this and do some experiments, but I
won't be able to do that before November as I'm recovering from a
surgery and have my left arm in a sling, so unless anybody else wants to
move this forward, I think it won't be included in 0.4

-- 
Guilherme Salgado <https://launchpad.net/~salgado>

lp:~salgado/tomdroid/sync-ui updated on 2010-09-17

241. By Guilherme Salgado on 2010-09-17: Store all tags in the DB rather than storing just a flag telling whether or not a note is a notebook template
242. By Guilherme Salgado on 2010-09-17: Add a comment explaining why the test will fail in some cases

Revision history for this message

Guilherme Salgado (salgado) wrote on 2010-09-18:

#

Actually, it was fairly easy to do what Rodja suggested, so maybe this can still be included in 0.4

Revision history for this message

Rodja (trappe) wrote on 2010-09-26:

#

Works fine and is defenitly a good thing to have in the 0.4 release featuring 'web sync'.

review: Approve

Tomdroid

Merge lp:~salgado/tomdroid/sync-ui into lp:~tomdroid-dev/tomdroid/sync-ui

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file '.classpath'
 --- .classpath	2010-08-31 21:00:35 +0000
 +++ .classpath	2010-09-17 21:12:56 +0000
@@ -6,4 +6,5 @@
  	<classpathentry kind="lib" path="lib/signpost-core-1.2.1.1.jar"/>
  	<classpathentry kind="lib" path="lib/signpost-commonshttp4-1.2.1.1.jar"/>
  	<classpathentry kind="output" path="bin"/>
++  <classpathentry kind="src" path="tests"/>
  </classpath>
 === modified file 'AndroidManifest.xml'
 --- AndroidManifest.xml	2010-05-24 07:27:14 +0000
 +++ AndroidManifest.xml	2010-09-17 21:12:56 +0000
@@ -48,7 +48,10 @@
      	</activity>
++      <uses-library android:name="android.test.runner" />
      </application>
  	 <uses-permission android:name="android.permission.INTERNET" />
++<instrumentation android:name="android.test.InstrumentationTestRunner" android:targetPackage="org.tomdroid"></instrumentation>
++
  </manifest>
 === modified file 'src/org/tomdroid/Note.java'
 --- src/org/tomdroid/Note.java	2010-08-30 16:33:02 +0000
 +++ src/org/tomdroid/Note.java	2010-09-17 21:12:56 +0000
@@ -28,6 +28,7 @@
  import java.util.regex.Pattern;
  import org.json.JSONObject;
++import org.json.JSONArray;
  import org.tomdroid.util.NoteContentBuilder;
  import org.tomdroid.util.XmlUtils;
@@ -46,6 +47,7 @@
  	public static final String MODIFIED_DATE = "modified_date";
  	public static final String URL = "url";
  	public static final String FILE = "file";
++	public static final String TAGS = "tags";
  	public static final String NOTE_CONTENT = "content";
  	// Logging info
@@ -65,6 +67,7 @@
  	private String url;
  	private String fileName;
  	private String title;
++	private String tags;
  	private Time lastChangeDate;
  	private int dbId;
  	private UUID guid;
@@ -85,8 +88,21 @@
  		setGuid(json.optString("guid"));
  		setLastChangeDate(json.optString("last-change-date"));
  		setXmlContent(json.optString("note-content"));
++		JSONArray jtags = json.optJSONArray("tags");
++		String tag;
++		tags = new String();
++		if (jtags != null) {
++			for (int i = 0; i < jtags.length(); i++ ) {
++				tag = jtags.optString(i);
++				tags += tag + ",";
++			}
++		}
+ 	}
++	public String getTags() {
++		return tags;
++	}
++
  	public String getUrl() {
  		return url;
+ 	}
@@ -150,7 +166,7 @@
  	public void setGuid(String guid) {
  		this.guid = UUID.fromString(guid);
+ 	}
--
++
  	// TODO: should this handler passed around evolve into an observer pattern?
  	public SpannableStringBuilder getNoteContent(Handler handler) {
 === modified file 'src/org/tomdroid/NoteManager.java'
 --- src/org/tomdroid/NoteManager.java	2010-03-21 18:38:49 +0000
 +++ src/org/tomdroid/NoteManager.java	2010-09-17 21:12:56 +0000
@@ -96,6 +96,7 @@
  		// Notice that we store the date in UTC because sqlite doesn't handle RFC3339 timezone information
  		values.put(Note.MODIFIED_DATE, note.getLastChangeDate().format3339(false));
  		values.put(Note.NOTE_CONTENT, note.getXmlContent());
++		values.put(Note.TAGS, note.getTags());
  		if (managedCursor.getCount() == 0) {
@@ -127,18 +128,26 @@
  			return false;
+ 	}
++	public static Cursor getAllNotes(Activity activity, Boolean includeNotebookTemplates) {
++		// get a cursor representing all notes from the NoteProvider
++		Uri notes = Tomdroid.CONTENT_URI;
++		String where = null;
++		if (!includeNotebookTemplates) {
++			where = Note.TAGS + " NOT LIKE '%" + "system:template" + "%'";
++		}
++		return activity.managedQuery(notes, LIST_PROJECTION, where, null, null);
++	}
++
++
  	public static ListAdapter getListAdapter(Activity activity) {
--
--		// get a cursor representing all notes from the NoteProvider
--		Uri notes = Tomdroid.CONTENT_URI;
--		Cursor notesCursor = activity.managedQuery(notes, LIST_PROJECTION, null, null, null);
++		Cursor notesCursor = getAllNotes(activity, false);
  		// set up an adapter binding the TITLE field of the cursor to the list item
  		String[] from = new String[] { Note.TITLE };
  		int[] to = new int[] { R.id.note_title };
  		return new SimpleCursorAdapter(activity, R.layout.main_list_item, notesCursor, from, to);
+ 	}
--
++
  	// gets the titles of the notes present in the db, used in ViewNote.buildLinkifyPattern()
  	public static Cursor getTitles(Activity activity) {
@@ -149,7 +158,7 @@
  	// gets the ids of the notes present in the db, used in SyncService.deleteNotes()
  	public static Cursor getGuids(Activity activity) {
--		// get a cursor containing the notes titles
++		// get a cursor containing the notes guids
  		return activity.managedQuery(Tomdroid.CONTENT_URI, GUID_PROJECTION, null, null, null);
+ 	}
 === modified file 'src/org/tomdroid/NoteProvider.java'
 --- src/org/tomdroid/NoteProvider.java	2010-08-30 16:33:02 +0000
 +++ src/org/tomdroid/NoteProvider.java	2010-09-17 21:12:56 +0000
@@ -69,7 +69,7 @@
  	// --
  	private static final String DATABASE_NAME = "tomdroid-notes.db";
  	private static final String DB_TABLE_NOTES = "notes";
--	private static final int DB_VERSION = 2;
++	private static final int DB_VERSION = 3;
  	private static final String DEFAULT_SORT_ORDER = Note.MODIFIED_DATE + " DESC";
      private static HashMap<String, String> notesProjectionMap;
@@ -100,7 +100,8 @@
                      + Note.TITLE + " TEXT,"
                      + Note.FILE + " TEXT,"
                      + Note.NOTE_CONTENT + " TEXT,"
--                    + Note.MODIFIED_DATE + " STRING"
++                    + Note.MODIFIED_DATE + " STRING,"
++                    + Note.TAGS + " STRING"
                      + ");");
+         }
@@ -297,6 +298,7 @@
          notesProjectionMap.put(Note.TITLE, Note.TITLE);
          notesProjectionMap.put(Note.FILE, Note.FILE);
          notesProjectionMap.put(Note.NOTE_CONTENT, Note.NOTE_CONTENT);
++        notesProjectionMap.put(Note.TAGS, Note.TAGS);
          notesProjectionMap.put(Note.MODIFIED_DATE, Note.MODIFIED_DATE);
+     }
+ }
 === added directory 'tests'
 === added directory 'tests/org'
 === added directory 'tests/org/tomdroid'
 === added file 'tests/org/tomdroid/NoteManagerTest.java'
 --- tests/org/tomdroid/NoteManagerTest.java	1970-01-01 00:00:00 +0000
 +++ tests/org/tomdroid/NoteManagerTest.java	2010-09-17 21:12:56 +0000
@@ -0,0 +1,74 @@
++package org.tomdroid;
++
++import org.json.JSONObject;
++import org.tomdroid.Note;
++import org.tomdroid.NoteManager;
++import org.tomdroid.ui.Tomdroid;
++import org.tomdroid.util.Preferences;
++
++import android.app.Activity;
++import android.content.Intent;
++import android.database.Cursor;
++import android.test.ActivityUnitTestCase;
++
++public class NoteManagerTest extends ActivityUnitTestCase<Tomdroid> {
++
++	public NoteManagerTest() {
++		super(Tomdroid.class);
++	}
++
++	public void testGetAllNotes() throws Exception {
++		Activity activity = getActivity();
++		putNotes(activity);
++		Cursor cursor;
++		// Get all notes excluding the notebook template ones.
++		cursor = NoteManager.getAllNotes(activity, false);
++		assertEquals(1, cursor.getCount());
++
++		// Get all notes, including notebook templates this time.
++		cursor = NoteManager.getAllNotes(activity, true);
++		assertEquals(2, cursor.getCount());
++	}
++
++	private void putNotes(Activity a) throws Exception {
++		// Add a regular note to the content manager.
++		JSONObject note = new JSONObject(
++				"{'title': 'foo', 'note-content': 'bar', " +
++				"'guid': '002e91a2-2e34-4e2d-bf88-21def49a7704', " +
++				"'last-change-date': '2009-04-19T21:29:23.2197340-07:00', " +
++				"'tags': ['tag1', 'tag2']}");
++		Note n = new Note(note);
++		NoteManager.putNote(a, n);
++
++		// Add a notebook template to the content manager.
++		JSONObject template = new JSONObject(
++				"{'title': 'foo', 'note-content': 'bar', " +
++				"'guid': '992e91a2-2e34-4e2d-bf88-21def49a7712', " +
++				"'last-change-date': '2009-04-19T21:29:23.2197340-07:00', " +
++				"'tags': ['system:template', 'tag2']}");
++		Note t = new Note(template);
++		NoteManager.putNote(a, t);
++	}
++
++	@Override
++	public void setUp() throws Exception {
++		super.setUp();
++		// XXX: For some reason this will raise an
++		// "Unable to add window -- token null is not for an application"
++		// error when you run the test after wiping user data from the emulator.
++		// The error is actually raised when we try to display the AlertDialog that
++		// is shown the first time the user runs tomdroid.
++		startActivity(new Intent(), null, null);
++		// XXX: Soon we'll be able to replace the two lines below with LocalStorage.resetDatabase().
++		getActivity().getContentResolver().delete(Tomdroid.CONTENT_URI, null, null);
++		Preferences.putLong(Preferences.Key.LATEST_SYNC_REVISION, 0);
++	}
++
++	@Override
++	public void tearDown() throws Exception {
++		// XXX: Soon we'll be able to replace the two lines below with LocalStorage.resetDatabase().
++		getActivity().getContentResolver().delete(Tomdroid.CONTENT_URI, null, null);
++		Preferences.putLong(Preferences.Key.LATEST_SYNC_REVISION, 0);
++		super.tearDown();
++	}
++}
 === added file 'tests/org/tomdroid/NoteTest.java'
 --- tests/org/tomdroid/NoteTest.java	1970-01-01 00:00:00 +0000
 +++ tests/org/tomdroid/NoteTest.java	2010-09-17 21:12:56 +0000
@@ -0,0 +1,35 @@
++package org.tomdroid;
++
++import junit.framework.Assert;
++import junit.framework.TestCase;
++
++import org.tomdroid.Note;
++import org.json.JSONException;
++import org.json.JSONObject;
++
++
++public class NoteTest extends TestCase {
++
++	public void testConstructorForNoteWithTags() throws JSONException {
++		JSONObject json = new JSONObject(
++				"{'title': 'foo', 'note-content': 'bar', " +
++				"'guid': '002e91a2-2e34-4e2d-bf88-21def49a7705', " +
++				"'last-change-date': '2009-04-19T21:29:23.2197340-07:00', " +
++				"'tags': ['tag1', 'tag2']}");
++		Note n = new Note(json);
++		Assert.assertEquals("foo", n.getTitle());
++		Assert.assertEquals("002e91a2-2e34-4e2d-bf88-21def49a7705", n.getGuid().toString());
++		Assert.assertEquals("bar", n.getXmlContent());
++		Assert.assertEquals("tag1,tag2,", n.getTags());
++	}
++
++	public void testConstructorForNoteWithNoTags() throws JSONException {
++		JSONObject json = new JSONObject(
++				"{'title': 'foo', 'note-content': 'bar', " +
++				"'guid': '002e91a2-2e34-4e2d-bf88-21def49a7705', " +
++				"'last-change-date': '2009-04-19T21:29:23.2197340-07:00'}");
++		Note n = new Note(json);
++		Assert.assertEquals("foo", n.getTitle());
++		Assert.assertEquals("", n.getTags());
++	}
++}