Merge lp:~plaxx/tomdroid/storage-redesign into lp:~benoit.garret/tomdroid/storage-redesign

[Olivier Bilodeau]

[Olivier Bilodeau (plaxx)]

first of a series of small fixes to the storage-redesign branch

- its now using a thread to load a note
- loading from the web works again! even though no one cares ;)

_BUT_ the note's title appears at the top of the note

Anyway, more is to come, this was the critique of revno 148. I propose a merge because I think smaller is more manageable. If you don't have time for review, don't worry I'll stack additional changes to my branch later this week and I'll update the review.

[Benoit Garret (benoit.garret)]

The main point of having an updateContent method was to make sure the xml content and the displayable content are always in sync. The caching thing was added as an afterthought and I agree it is not the best solution. What did you dislike in it: the fact that xml parsing happens in the Note object or the update mechanism itself? If it's the former, I'm not against delegating the xml parsing to another class, like NoteBuilder.

In fact, I don't like the idea of having two different methods to get and set what are in fact two representations of the same content. It would be far too easy to set one and not the other and wonder why the content that gets synced isn't the one that is displayed.

Here's what I'd also like to do (not related to my previous point):

1/ Remove the load note from web feature. It might seem backwards to remove features, but we both know that this one is quite useless and that no one will miss it. Moreover, it adds a fair bit of ugly code to handle a code path that will get removed sooner than later.

2/ Create a NoteManager to abstract the complexity of dealing with the content provider, with the following interfaces:
public Note getNote(UUID guid, Handler callback) // gets a note from the content provider
public Note getNote(URL url, Handler callback) // gets a note from a remote url
public void putNote(Note note) // puts a note in the content provider
public ListAdapter getListAdapter()
public ArrayList getTitles() // gets the titles of the notes present in the db, used in ViewNote.buildLinkifyPattern()
public TransformFilter getTitleToUrl() // returns the transform filter used for the note titles

Point 2 amounts to get rid of references to the content provider everywhere except in the NoteManager. This would clean the code a lot and avoid having connections to the content provider, which is far from optimal.

Your thoughts on this?

[Benoit Garret (benoit.garret)]

I merged the revisions 158, 159 and 160.

[Benoit Garret (benoit.garret)]

I pushed some updates in my branch:
Rev 161 and 162 respectively delete the load note from web feature and create a NoteManager.
Rev 163 is rev. 161 of your branch excepted that the NoteBuilder is used in Note instead of ViewNote.

Please let me know what is your opinion on these changes, I'm ready to rework them if they're not up to your standards.

[Olivier Bilodeau (plaxx)]

On Sun, Oct 18, 2009 at 9:19 AM, Benoit Garret <
<email address hidden>> wrote:

> The main point of having an updateContent method was to make sure the xml
> content and the displayable content are always in sync. The caching thing
> was added as an afterthought and I agree it is not the best solution. What
> did you dislike in it: the fact that xml parsing happens in the Note object
> or the update mechanism itself? If it's the former, I'm not against
> delegating the xml parsing to another class, like NoteBuilder.
>
>
Yes, it was the former.

> In fact, I don't like the idea of having two different methods to get and
> set what are in fact two representations of the same content. It would be
> far too easy to set one and not the other and wonder why the content that
> gets synced isn't the one that is displayed.
>

Totally agree with you on this one.

>
> Here's what I'd also like to do (not related to my previous point):
>
> 1/ Remove the load note from web feature. It might seem backwards to remove
> features, but we both know that this one is quite useless and that no one
> will miss it. Moreover, it adds a fair bit of ugly code to handle a code
> path that will get removed sooner than later.
>

Yes, and I saw that you did remove it in a later message. Although, I would
have preferred a new branch or a patch (but I'm being an ass here). I'll
merge or cherry-pick, I agree with you: its time to get rid of that

>
> 2/ Create a NoteManager to abstract the complexity of dealing with the
> content provider, with the following interfaces:
> public Note getNote(UUID guid, Handler callback) // gets a note from the
> content provider
> public Note getNote(URL url, Handler callback) // gets a note from a remote
> url
> public void putNote(Note note) // puts a note in the content provider
> public ListAdapter getListAdapter()
> public ArrayList getTitles() // gets the titles of the notes present in the
> db, used in ViewNote.buildLinkifyPattern()
> public TransformFilter getTitleToUrl() // returns the transform filter used
> for the note titles
>
> Point 2 amounts to get rid of references to the content provider everywhere
> except in the NoteManager. This would clean the code a lot and avoid having
> connections to the content provider, which is far from optimal.
>

I don't mind, actually, I like it, its cleaner. Just being curious: how is
linking to the content provider inefficient?

[Benoit Garret (benoit.garret)]

> > Point 2 amounts to get rid of references to the content provider everywhere
> > except in the NoteManager. This would clean the code a lot and avoid having
> > connections to the content provider, which is far from optimal.
> >
>
> I don't mind, actually, I like it, its cleaner. Just being curious: how is
> linking to the content provider inefficient?

Oops, I forgot the word multiple here. We won't get rid of the connection to the provider. I meant that having a single point for dealing with the content provider enables us to quickly check how much cursors are we using, etc.

Are you okay with setting your branch as merged?

[Olivier Bilodeau (plaxx)]

I merged your branch already but I haven't pushed yet (need to fix
conflicts).

There are other minor items I want to change before merging your storage
branch in main. I'll work on these and once ready, I'll resubmit this merge
proposal.

--
Olivier

On Nov 13, 2009 12:39 AM, "Benoit Garret" <email address hidden>
wrote:

> > Point 2 amounts to get rid of references to the content provider
everywhere > > except in the No...
Oops, I forgot the word multiple here. We won't get rid of the connection to
the provider. I meant that having a single point for dealing with the
content provider enables us to quickly check how much cursors are we using,
etc.

Are you okay with setting your branch as merged?

-- https://code.launchpad.net/~plaxx/tomdroid/storage-redesign/+merge/12858You
are the owner of lp...

[Benoit Garret (benoit.garret)]

> I merged your branch already but I haven't pushed yet (need to fix
> conflicts).
>
> There are other minor items I want to change before merging your storage
> branch in main. I'll work on these and once ready, I'll resubmit this merge
> proposal.

Awesome! Just let me know when you're ready.

[Olivier Bilodeau (plaxx)]

I really like your NoteManager, it does remove a lot of Cursor complexity in the ui classes. Nice!

Also, the removal of "load from web" is very welcome.

I still have some other items I need to look at but its getting closer to merge.

[Olivier Bilodeau (plaxx)]

I would consider this ready for your storage-redesign branch.

If you think it's fine, accept it and propose a merge from your storage-redesign into main/.

review: Approve

[lp:~plaxx/tomdroid/storage-redesign]

169. By Olivier Bilodeau on 2010-01-15

Misc cleanup
- NoteManager's constructor no longer public (to prevent bypassing getInstance())
- Added comments
- Moved stuff around (static first)

[Olivier Bilodeau (plaxx)]

Just did a little commit with some minor cleanup.

Also, there's another thing with NoteManager. It needs initialization.. for a singleton class I don't think it's good. Especially when it's an activity class that is passed to it and that this activity is solely used to do managed cursor queries.

For the past half-hour I'm looking at other ways to do it to avoid side effects mentioned in the managedQuery page and I didn't find anything..

http://developer.android.com/reference/android/app/Activity.html#managedQuery%28android.net.Uri,%20java.lang.String[],%20java.lang.String,%20java.lang.String[],%20java.lang.String%29

Our manager cannot rely on a Cursor that can die at any minute because the activity associated to it (ui/Tomdroid) was the oldest memory eater out there and got killed.

Are you aware of any global mechanism that can allow any class to get it's way to query()? Otherwise, I'm afraid I think the best to avoid bugs in the long run is to either:
a) have an instance of NoteManager per activity (and send activity in the constructor) and drop the singleton code
b) all of NoteManager's methods should require an Activity in the parameter list so the proper cursor can be created

What do you prefer?

[Benoit Garret (benoit.garret)]

Hey, thanks for all the work. I won't have time today and tomorrow, I'll take a look at this on Sunday. Feel free to change anything you want until then. In the meantime, have a good weekend!

[lp:~plaxx/tomdroid/storage-redesign]

170. By Olivier Bilodeau on 2010-01-16

Removed the init phase of the NoteManager singleton to avoid problems with managed cursors.
Now every call to NoteManager provides an activity reference to use the cursor in the current active activity's context.

[Benoit Garret (benoit.garret)]

> Also, there's another thing with NoteManager. It needs initialization.. for a
> singleton class I don't think it's good. Especially when it's an activity
> class that is passed to it and that this activity is solely used to do managed
> cursor queries.
>
> For the past half-hour I'm looking at other ways to do it to avoid side
> effects mentioned in the managedQuery page and I didn't find anything..
>
> http://developer.android.com/reference/android/app/Activity.html#managedQuery%
> 28android.net.Uri,%20java.lang.String[],%20java.lang.String,%20java.lang.Strin
> g[],%20java.lang.String%29
>
> Our manager cannot rely on a Cursor that can die at any minute because the
> activity associated to it (ui/Tomdroid) was the oldest memory eater out there
> and got killed.
>
> Are you aware of any global mechanism that can allow any class to get it's way
> to query()? Otherwise, I'm afraid I think the best to avoid bugs in the long
> run is to either:
> a) have an instance of NoteManager per activity (and send activity in the
> constructor) and drop the singleton code
> b) all of NoteManager's methods should require an Activity in the parameter
> list so the proper cursor can be created
>
> What do you prefer?

It's perfectly possible to use query directly but the cursor won't be managed (and I guess we don't really want that).

I'd say solution b is the way to go, adding a parameter is not that cumbersome and we keep the advantages of the singleton.

Still reviewing the rest...

[Benoit Garret (benoit.garret)]

I'm done, everything you made seems sensible. I found a few outdated comments and useless methods but I made a commit in my branch. I also made the NoteManager methods static because they became stateless when you added the Activity as a parameter.

review: Approve

=== modified file 'src/org/tomdroid/Note.java'
--- src/org/tomdroid/Note.java	2009-10-31 15:07:30 +0000
+++ src/org/tomdroid/Note.java	2010-01-16 21:03:11 +0000
@@ -27,7 +27,7 @@
 import org.joda.time.DateTime;
 import org.joda.time.format.DateTimeFormatter;
 import org.joda.time.format.ISODateTimeFormat;
-import org.tomdroid.util.NoteBuilder;
+import org.tomdroid.util.NoteContentBuilder;
 
 import android.os.Handler;
 import android.text.SpannableStringBuilder;
@@ -114,8 +114,11 @@
 		this.guid = UUID.fromString(guid);
 	}
 	
+	// TODO: should this handler passed around evolve into an observer pattern?
 	public SpannableStringBuilder getNoteContent(Handler handler) {
-		noteContent = new NoteBuilder().setCaller(handler).setInputSource(xmlContent).build();
+		
+		// TODO not sure this is the right place to do this
+		noteContent = new NoteContentBuilder().setCaller(handler).setInputSource(xmlContent).build();
 		return noteContent;
 	}
 	
@@ -126,7 +129,7 @@
 	public void setXmlContent(String xmlContent) {
 		this.xmlContent = xmlContent;
 	}
-	
+
 	@Override
 	public String toString() {
 		// format date time according to XML standard

=== modified file 'src/org/tomdroid/NoteManager.java'
--- src/org/tomdroid/NoteManager.java	2009-10-25 17:00:11 +0000
+++ src/org/tomdroid/NoteManager.java	2010-01-16 21:03:11 +0000
@@ -1,3 +1,25 @@
+/*
+ * Tomdroid
+ * Tomboy on Android
+ * http://www.launchpad.net/tomdroid
+ * 
+ * Copyright 2009, 2010 Olivier Bilodeau <[e-mail]>
+ * 
+ * This file is part of Tomdroid.
+ * 
+ * Tomdroid is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ * 
+ * Tomdroid is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Public License
+ * along with Tomdroid.  If not, see <http://www.gnu.org/licenses/>.
+ */
 package org.tomdroid;
 
 import org.tomdroid.ui.Tomdroid;
@@ -23,9 +45,12 @@
 	private static final String TAG = "NoteManager";
 	
 	private static NoteManager instance = null;
-	// the main activity this manager is linked to
-	private static Activity activity = null;
+
+	// instance properties
+	private Cursor notesCursor;
 	
+	// singleton pattern
+	// TODO verify this singleton with current java best practices
 	public static NoteManager getInstance()
 	{
 		if(instance == null) {
@@ -39,22 +64,10 @@
 		return instance;
 	}
 	
-	// instance properties
-	private Cursor notesCursor;
-	
-	public NoteManager() throws Exception
-	{
-		if(activity == null)
-			throw new Exception("init() has not been called.");
-	}
-	
-	public static void init(Activity a)
-	{
-		activity = a;
-	}
+	private NoteManager() {}
 	
 	// gets a note from the content provider
-	public Note getNote(Uri uri) {
+	public Note getNote(Activity activity, Uri uri) {
 		
 		Note note = null;
 		
@@ -77,7 +90,7 @@
 	}
 	
 	// puts a note in the content provider
-	public void putNote(Note note) {
+	public void putNote(Activity activity, Note note) {
 		
 		// verify if the note is already in the content provider
 		
@@ -120,7 +133,7 @@
 		}
 	}
 	
-	public ListAdapter getListAdapter() {
+	public ListAdapter getListAdapter(Activity activity) {
 		
 		// get a cursor representing all notes from the NoteProvider
 		Uri notes = Tomdroid.CONTENT_URI;
@@ -133,13 +146,13 @@
 	}
 	
 	// gets the titles of the notes present in the db, used in ViewNote.buildLinkifyPattern()
-	public Cursor getTitles() {
+	public Cursor getTitles(Activity activity) {
 		
 		// get a cursor containing the notes titles
 		return activity.managedQuery(Tomdroid.CONTENT_URI, TITLE_PROJECTION, null, null, null);
 	}
 	
-	public int getNoteId(String title) {
+	public int getNoteId(Activity activity, String title) {
 		
 		int id = 0;
 		

=== modified file 'src/org/tomdroid/ui/Tomdroid.java'
--- src/org/tomdroid/ui/Tomdroid.java	2009-10-25 17:00:11 +0000
+++ src/org/tomdroid/ui/Tomdroid.java	2010-01-16 21:03:11 +0000
@@ -3,7 +3,7 @@
  * Tomboy on Android
  * http://www.launchpad.net/tomdroid
  * 
- * Copyright 2008, 2009 Olivier Bilodeau <[e-mail]>
+ * Copyright 2008, 2009, 2010 Olivier Bilodeau <[e-mail]>
  * 
  * This file is part of Tomdroid.
  * 
@@ -61,7 +61,7 @@
 	// TODO hardcoded for now
 	public static final String NOTES_PATH = "/sdcard/tomdroid/";
 	// Logging should be disabled for release builds
-	public static final boolean LOGGING_ENABLED = false;
+	public static final boolean LOGGING_ENABLED = true;
 
 	// Logging info
 	private static final String TAG = "Tomdroid";
@@ -82,7 +82,6 @@
     public void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
         
-        NoteManager.init(this);
         setContentView(R.layout.main);
         
         // did we already show the warning and got destroyed by android's activity killer?
@@ -101,7 +100,7 @@
 				.show();
         }
         
-        adapter = NoteManager.getInstance().getListAdapter();
+        adapter = NoteManager.getInstance().getListAdapter(this);
 		setListAdapter(adapter);
 
         // set the view shown when the list is empty
@@ -122,20 +121,20 @@
 	public boolean onOptionsItemSelected(MenuItem item) {
         switch (item.getItemId()) {
 	        case R.id.menuSyncWithSD:
-	            
-	            // start loading local notes
-	            if (LOGGING_ENABLED) Log.v(TAG, "Loading local notes");
-	            
-	        	try {
-	        		File notesRoot = new File(Tomdroid.NOTES_PATH);
-	        		
-	        		if (!notesRoot.exists()) {
-	        			throw new FileNotFoundException("Tomdroid notes folder doesn't exist. It is configured to be at: "+Tomdroid.NOTES_PATH);
-	        		}
-	        		
-	        		AsyncNoteLoaderAndParser asyncLoader = new AsyncNoteLoaderAndParser(notesRoot);
+
+	        	// start loading local notes
+                if (LOGGING_ENABLED) Log.v(TAG, "Loading local notes");
+
+            	try {
+            		File notesRoot = new File(Tomdroid.NOTES_PATH);
+        		
+            		if (!notesRoot.exists()) {
+        			throw new FileNotFoundException("Tomdroid notes folder doesn't exist. It is configured to be at: "+Tomdroid.NOTES_PATH);
+            		}
+        		
+	        		AsyncNoteLoaderAndParser asyncLoader = new AsyncNoteLoaderAndParser(this, notesRoot);
 	        		asyncLoader.readAndParseNotes();
-	        		
+        		
 	    		} catch (FileNotFoundException e) {
 	    			//TODO put strings in an external resource
 	    			listEmptyView.setText(R.string.strListEmptyNoNotes);
@@ -149,8 +148,8 @@
 	    				.show();
 	    			e.printStackTrace();
 	    		}
-	        	
-	        	return true;
+        	
+	    		return true;
 	        
 	        case R.id.menuAbout:
 				showAboutDialog();

=== modified file 'src/org/tomdroid/ui/ViewNote.java'
--- src/org/tomdroid/ui/ViewNote.java	2009-10-31 15:07:30 +0000
+++ src/org/tomdroid/ui/ViewNote.java	2010-01-16 21:03:11 +0000
@@ -3,7 +3,7 @@
  * Tomboy on Android
  * http://www.launchpad.net/tomdroid
  * 
- * Copyright 2008, 2009 Olivier Bilodeau <[e-mail]>
+ * Copyright 2008, 2009, 2010 Olivier Bilodeau <[e-mail]>
  * 
  * This file is part of Tomdroid.
  * 
@@ -28,10 +28,13 @@
 import org.tomdroid.Note;
 import org.tomdroid.NoteManager;
 import org.tomdroid.R;
-import org.tomdroid.util.NoteBuilder;
+import org.tomdroid.util.NoteContentBuilder;
 
 import android.app.Activity;
+import android.app.AlertDialog;
+import android.content.DialogInterface;
 import android.content.Intent;
+import android.content.DialogInterface.OnClickListener;
 import android.database.Cursor;
 import android.net.Uri;
 import android.os.Bundle;
@@ -75,7 +78,8 @@
 			// TODO validate the good action?
 			// intent.getAction()
 			
-			note = NoteManager.getInstance().getNote(uri);
+			// TODO verify that getNote is doing the proper validation
+			note = NoteManager.getInstance().getNote(this, uri);
 			
 			if(note != null) {
 				
@@ -85,7 +89,34 @@
 				
 				// TODO send an error to the user
 				if (Tomdroid.LOGGING_ENABLED) Log.d(TAG, "The note "+uri+" doesn't exist");
+				
+				// TODO put error string in a translatable resource
+				new AlertDialog.Builder(this)
+					.setMessage("The requested note could not be found. If you see this error by " +
+							    "mistake and you are able to replicate it, please file a bug!")
+					.setTitle("Error")
+					.setNeutralButton("Ok", new OnClickListener() {
+						public void onClick(DialogInterface dialog, int which) {
+							dialog.dismiss();
+							finish();
+						}})
+					.show();
 			}
+		} else {
+			
+			if (Tomdroid.LOGGING_ENABLED) Log.d(TAG, "The Intent's data was null.");
+			
+			// TODO put error string in a translatable resource
+			new AlertDialog.Builder(this)
+			.setMessage("The requested note could not be found. If you see this error by " +
+					    "mistake and you are able to replicate it, please file a bug!")
+			.setTitle("Error")
+			.setNeutralButton("Ok", new OnClickListener() {
+				public void onClick(DialogInterface dialog, int which) {
+					dialog.dismiss();
+					finish();
+				}})
+			.show();
 		}
 	}
 	
@@ -126,8 +157,25 @@
 		@Override
 		public void handleMessage(Message msg) {
 			
-			if(msg.what == NoteBuilder.PARSE_OK)
+			//parsed ok - show
+			if(msg.what == NoteContentBuilder.PARSE_OK) {
 				showNote();
+
+			//parsed not ok - error
+			} else if(msg.what == NoteContentBuilder.PARSE_ERROR) {
+				
+				// TODO put this String in a translatable resource
+				new AlertDialog.Builder(ViewNote.this)
+					.setMessage("The requested note could not be parsed. If you see this error by " +
+								"mistake and you are able to replicate it, please file a bug!")
+					.setTitle("Error")
+					.setNeutralButton("Ok", new OnClickListener() {
+						public void onClick(DialogInterface dialog, int which) {
+							dialog.dismiss();
+							finish();
+						}})
+					.show();
+        	}
 		}
 	};
 	
@@ -139,12 +187,13 @@
 	public Pattern buildNoteLinkifyPattern()  {
 		
 		StringBuilder sb = new StringBuilder();
-		Cursor cursor = NoteManager.getInstance().getTitles();
+		Cursor cursor = NoteManager.getInstance().getTitles(this);
 		
 		// cursor must not be null and must return more than 0 entry 
 		if (!(cursor == null || cursor.getCount() == 0)) {
 			
 			String title;
+			
 			cursor.moveToFirst();
 			
 			do {
@@ -176,8 +225,7 @@
 
 		public String transformUrl(Matcher m, String str) {
 
-			// FIXME if this activity is called from another app and Tomdroid was never launched, getting here will probably make it crash
-			int id = NoteManager.getInstance().getNoteId(str);
+			int id = NoteManager.getInstance().getNoteId(ViewNote.this, str);
 			
 			// return something like content://org.tomdroid.notes/notes/3
 			return Tomdroid.CONTENT_URI.toString()+"/"+id;

=== modified file 'src/org/tomdroid/util/AsyncNoteLoaderAndParser.java'
--- src/org/tomdroid/util/AsyncNoteLoaderAndParser.java	2009-10-25 17:00:11 +0000
+++ src/org/tomdroid/util/AsyncNoteLoaderAndParser.java	2010-01-16 21:03:11 +0000
@@ -3,7 +3,7 @@
  * Tomboy on Android
  * http://www.launchpad.net/tomdroid
  * 
- * Copyright 2009 Olivier Bilodeau <[e-mail]>
+ * Copyright 2009, 2010 Olivier Bilodeau <[e-mail]>
  * 
  * This file is part of Tomdroid.
  * 
@@ -25,36 +25,48 @@
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.FilenameFilter;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.io.Reader;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import javax.xml.parsers.ParserConfigurationException;
 import javax.xml.parsers.SAXParser;
 import javax.xml.parsers.SAXParserFactory;
 
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+import org.xml.sax.XMLReader;
+
 import org.tomdroid.Note;
 import org.tomdroid.NoteManager;
 import org.tomdroid.ui.Tomdroid;
 import org.tomdroid.xml.NoteHandler;
-import org.xml.sax.InputSource;
-import org.xml.sax.SAXException;
-import org.xml.sax.XMLReader;
 
+import android.app.Activity;
 import android.util.Log;
 
 public class AsyncNoteLoaderAndParser {
 	private final ExecutorService pool;
 	private final static int poolSize = 1;
+	private Activity activity;
 	private File path;
 	
+	// regexp for <note-content..>...</note-content>
+	private static Pattern note_content = Pattern.compile(".*(<note-content.*>.*<\\/note-content>).*", Pattern.CASE_INSENSITIVE+Pattern.DOTALL);
+	
 	// logging related
 	private final static String TAG = "AsyncNoteLoaderAndParser";
 	
-	public AsyncNoteLoaderAndParser(File path) {
+	public AsyncNoteLoaderAndParser(Activity a, File path) {
+		this.activity = a;
 		this.path = path;
+		
 		pool = Executors.newFixedThreadPool(poolSize);
 	}
 
@@ -86,6 +98,7 @@
 		// the note to be loaded and parsed
 		private Note note = new Note();
 		private File file;
+		final char[] buffer = new char[0x10000];
 		
 		public Worker(File f) {
 			file = f;
@@ -96,7 +109,7 @@
 			note.setFileName(file.getAbsolutePath());
 			// the note guid is not stored in the xml but in the filename
 			note.setGuid(file.getName().replace(".note", ""));
-			
+
 			try {
 				// Parsing
 		    	// XML 
@@ -111,13 +124,12 @@
 		        NoteHandler xmlHandler = new NoteHandler(note);
 		        xr.setContentHandler(xmlHandler);
 
-		        
-		        // Create the proper input source based on if its a local note or a web note
-	        	FileInputStream fin = new FileInputStream(file);
-				BufferedReader in = new BufferedReader(new InputStreamReader(fin), 8192);
-				InputSource is = new InputSource(in);
-		        
-				if (Tomdroid.LOGGING_ENABLED) Log.v(TAG, "parsing note");
+		        // Create the proper input source
+		        FileInputStream fin = new FileInputStream(file);
+		        BufferedReader in = new BufferedReader(new InputStreamReader(fin), 8192);
+		        InputSource is = new InputSource(in);
+		        
+				if (Tomdroid.LOGGING_ENABLED) Log.d(TAG, "parsing note");
 				xr.parse(is);
 			
 			// TODO wrap and throw a new exception here
@@ -128,8 +140,38 @@
 			} catch (IOException e) {
 				e.printStackTrace();
 			}
-			
-			NoteManager.getInstance().putNote(note);
+
+			// extract the <note-content..>...</note-content>
+			if (Tomdroid.LOGGING_ENABLED) Log.d(TAG, "retrieving what is inside of note-content");
+			
+			// FIXME here we are re-reading the whole note just to grab note-content out, there is probably a best way to do this
+			StringBuilder out = new StringBuilder();
+			try {
+				int read;
+				Reader reader = new InputStreamReader(new FileInputStream(file), "UTF-8");
+				//Reader reader = new InputStreamReader(fin, "UTF-8");
+				do {
+				  read = reader.read(buffer, 0, buffer.length);
+				  if (read>0) {
+				    out.append(buffer, 0, read);
+				  }
+				}
+				while (read>=0);
+				
+				Matcher m = note_content.matcher(out.toString());
+				if (m.find()) {
+					note.setXmlContent(m.group());
+				} else {
+					if (Tomdroid.LOGGING_ENABLED) Log.w(TAG, "Something went wrong trying to grab the note-content out of a note");
+				}
+
+			} catch (IOException e) {
+				// TODO handle properly
+				e.printStackTrace();
+				if (Tomdroid.LOGGING_ENABLED) Log.w(TAG, "Something went wrong trying to read the note");
+			}
+			
+			NoteManager.getInstance().putNote(AsyncNoteLoaderAndParser.this.activity, note);
 		}
 	}
 }

=== renamed file 'src/org/tomdroid/util/NoteBuilder.java' => 'src/org/tomdroid/util/NoteContentBuilder.java'
--- src/org/tomdroid/util/NoteBuilder.java	2009-10-31 15:07:30 +0000
+++ src/org/tomdroid/util/NoteContentBuilder.java	2010-01-16 21:03:11 +0000
@@ -3,7 +3,7 @@
  * Tomboy on Android
  * http://www.launchpad.net/tomdroid
  * 
- * Copyright 2008, 2009 Olivier Bilodeau <[e-mail]>
+ * Copyright 2008, 2009, 2010 Olivier Bilodeau <[e-mail]>
  * 
  * This file is part of Tomdroid.
  * 
@@ -30,14 +30,13 @@
 import org.tomdroid.ui.Tomdroid;
 import org.tomdroid.xml.NoteContentHandler;
 import org.xml.sax.InputSource;
-import org.xml.sax.XMLReader;
 
 import android.os.Handler;
 import android.os.Message;
 import android.text.SpannableStringBuilder;
 import android.util.Log;
 
-public class NoteBuilder implements Runnable {
+public class NoteContentBuilder implements Runnable {
 	
 	public static final int PARSE_OK = 0;
 	public static final int PARSE_ERROR = 1;
@@ -54,24 +53,17 @@
 	private Thread runner;
 	private Handler parentHandler;
 	
-	public NoteBuilder () {}
+	public NoteContentBuilder () {}
 	
-	public NoteBuilder setCaller(Handler parent) {
+	public NoteContentBuilder setCaller(Handler parent) {
 		
 		parentHandler = parent;
 		return this;
 	}
 	
-	public NoteBuilder setInputSource(String nc) {
+	public NoteContentBuilder setInputSource(String nc) {
 		
-		//FIXME: I would pay a beer to get rid of that ugliness; I can't believe we cannot parse a partial XML tree using SAX
-		// Create a valid xml document
-		String xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n";
-		xml += "<note-content xmlns:link=\"http://beatniksoftware.com/tomboy/link\" xmlns:size=\"http://beatniksoftware.com/tomboy/size\" xmlns=\"http://beatniksoftware.com/tomboy\">";
-		xml += nc;
-		xml += "</note-content>";
-
-		noteContentIs = new InputSource(new StringReader(xml));
+		noteContentIs = new InputSource(new StringReader(nc));
 		return this;
 	}
 	
@@ -91,17 +83,14 @@
 	    	// XML 
 	    	// Get a SAXParser from the SAXPArserFactory
 	        SAXParserFactory spf = SAXParserFactory.newInstance();
+
+	        // trashing the namespaces but keep prefixes (since we don't have the xml header)
+	        spf.setFeature("http://xml.org/sax/features/namespaces", false);
+	        spf.setFeature("http://xml.org/sax/features/namespace-prefixes", true);
 	        SAXParser sp = spf.newSAXParser();
-	
-	        // Get the XMLReader of the SAXParser we created
-	        XMLReader xr = sp.getXMLReader();
-	        
-	        // Create a new ContentHandler, send it this note to fill and apply it to the XML-Reader
-	        NoteContentHandler xmlHandler = new NoteContentHandler(noteContent);
-	        xr.setContentHandler(xmlHandler);
-	        
+
 			if (Tomdroid.LOGGING_ENABLED) Log.v(TAG, "parsing note");
-			xr.parse(noteContentIs);
+	        sp.parse(noteContentIs, new NoteContentHandler(noteContent));
 		} catch (Exception e) {
 			e.printStackTrace();
 			// TODO handle error in a more granular way

=== removed file 'src/org/tomdroid/util/XmlUtils.java'
--- src/org/tomdroid/util/XmlUtils.java	2009-09-01 14:27:46 +0000
+++ src/org/tomdroid/util/XmlUtils.java	1970-01-01 00:00:00 +0000
@@ -1,36 +0,0 @@
-package org.tomdroid.util;
-
-public class XmlUtils {
-	
-	/**
-	 * Useful to replace the characters forbidden in xml by their escaped counterparts
-	 * Ex: &amp; -> &amp;amp;
-	 * 
-	 * @param input the string to escape
-	 * @return the escaped string
-	 */
-	public static String escape(String input) {
-		return input
-			.replace("&", "&amp;")
-			.replace("<", "&lt;")
-			.replace(">", "&gt;")
-			.replace("\"", "&quot;")
-			.replace("\'", "&apos;");
-	}
-	
-	/**
-	 * Useful to replace the escaped characters their unescaped counterparts
-	 * Ex: &amp;amp; -> &amp;
-	 * 
-	 * @param input the string to unescape
-	 * @return the unescaped string
-	 */
-	public static String unescape(String input) {
-		return input
-			.replace("&amp;", "&")
-			.replace("&lt;", "<")
-			.replace("&gt;", ">")
-			.replace("&quot;", "\"")
-			.replace("&apos;", "\'");
-	}
-}

=== modified file 'src/org/tomdroid/xml/NoteContentHandler.java'
--- src/org/tomdroid/xml/NoteContentHandler.java	2009-09-01 14:27:46 +0000
+++ src/org/tomdroid/xml/NoteContentHandler.java	2010-01-16 21:03:11 +0000
@@ -3,7 +3,7 @@
  * Tomboy on Android
  * http://www.launchpad.net/tomdroid
  * 
- * Copyright 2008 Olivier Bilodeau <[e-mail]>
+ * Copyright 2008, 2009, 2010 Olivier Bilodeau <[e-mail]>
  * 
  * This file is part of Tomdroid.
  * 
@@ -64,11 +64,9 @@
 	private final static String STRIKETHROUGH = "strikethrough";
 	private final static String HIGHLIGHT = "highlight";
 	private final static String MONOSPACE = "monospace";
-	// Sizes are using a namespace identifier size. Ex: <size:small></size:small>
-	private final static String NS_SIZE = "http://beatniksoftware.com/tomboy/size";
-	private final static String SMALL = "small";
-	private final static String LARGE = "large";
-	private final static String HUGE = "huge";
+	private final static String SMALL = "size:small";
+	private final static String LARGE = "size:large";
+	private final static String HUGE = "size:huge";
 	// Bullet list-related
 	private final static String LIST = "list";
 	private final static String LIST_ITEM = "list-item";
@@ -84,7 +82,7 @@
 	@Override
 	public void startElement(String uri, String localName, String name,	Attributes attributes) throws SAXException {
 		
-		if (localName.equals(NOTE_CONTENT)) {
+		if (name.equals(NOTE_CONTENT)) {
 
 			// we are under the note-content tag
 			// we will append all its nested tags so I create a string builder to do that
@@ -94,28 +92,25 @@
 		// if we are in note-content, keep and convert formatting tags
 		// TODO is XML CaSe SeNsItIve? if not change equals to equalsIgnoreCase and apply to endElement()
 		if (inNoteContentTag) {
-			if (localName.equals(BOLD)) {
+			if (name.equals(BOLD)) {
 				inBoldTag = true;
-			} else if (localName.equals(ITALIC)) {
+			} else if (name.equals(ITALIC)) {
 				inItalicTag = true;
-			} else if (localName.equals(STRIKETHROUGH)) {
+			} else if (name.equals(STRIKETHROUGH)) {
 				inStrikeTag = true;
-			} else if (localName.equals(HIGHLIGHT)) {
+			} else if (name.equals(HIGHLIGHT)) {
 				inHighlighTag = true;
-			} else if (localName.equals(MONOSPACE)) {
+			} else if (name.equals(MONOSPACE)) {
 				inMonospaceTag = true;
-			} else if (uri.equals(NS_SIZE)) {
-				// now check for the different possible sizes
-				if (localName.equals(SMALL)) {
-					inSizeSmallTag = true;
-				} else if (localName.equals(LARGE)) {
-					inSizeLargeTag = true;
-				} else if (localName.equals(HUGE)) {
-					inSizeHugeTag = true;
-				}
-			} else if (localName.equals(LIST)) {
+			} else if (name.equals(SMALL)) {
+				inSizeSmallTag = true;
+			} else if (name.equals(LARGE)) {
+				inSizeLargeTag = true;
+			} else if (name.equals(HUGE)) {
+				inSizeHugeTag = true;
+			} else if (name.equals(LIST)) {
 				inListLevel++;
-			} else if (localName.equals(LIST_ITEM)) {
+			} else if (name.equals(LIST_ITEM)) {
 				inListItem = true;
 			}
 		}
@@ -126,34 +121,31 @@
 	public void endElement(String uri, String localName, String name)
 			throws SAXException {
 
-		if (localName.equals(NOTE_CONTENT)) {
+		if (name.equals(NOTE_CONTENT)) {
 			inNoteContentTag = false;
 		}
 		
 		// if we are in note-content, keep and convert formatting tags
 		if (inNoteContentTag) {
-			if (localName.equals(BOLD)) {
+			if (name.equals(BOLD)) {
 				inBoldTag = false;
-			} else if (localName.equals(ITALIC)) {
+			} else if (name.equals(ITALIC)) {
 				inItalicTag = false;
-			} else if (localName.equals(STRIKETHROUGH)) {
+			} else if (name.equals(STRIKETHROUGH)) {
 				inStrikeTag = false;
-			} else if (localName.equals(HIGHLIGHT)) {
+			} else if (name.equals(HIGHLIGHT)) {
 				inHighlighTag = false;
-			} else if (localName.equals(MONOSPACE)) {
+			} else if (name.equals(MONOSPACE)) {
 				inMonospaceTag = false;
-			} else if (uri.equals(NS_SIZE)) {
-				// now check for the different possible sizes
-				if (localName.equals(SMALL)) {
-					inSizeSmallTag = false;
-				} else if (localName.equals(LARGE)) {
-					inSizeLargeTag = false;
-				} else if (localName.equals(HUGE)) {
-					inSizeHugeTag = false;
-				} 
-			} else if (localName.equals(LIST)) {
+			} else if (name.equals(SMALL)) {
+				inSizeSmallTag = false;
+			} else if (name.equals(LARGE)) {
+				inSizeLargeTag = false;
+			} else if (name.equals(HUGE)) {
+				inSizeHugeTag = false;
+			} else if (name.equals(LIST)) {
 				inListLevel--;
-			} else if (localName.equals(LIST_ITEM)) {
+			} else if (name.equals(LIST_ITEM)) {
 				inListItem = false;
 			}
 		}

=== modified file 'src/org/tomdroid/xml/NoteHandler.java'
--- src/org/tomdroid/xml/NoteHandler.java	2009-09-01 14:27:46 +0000
+++ src/org/tomdroid/xml/NoteHandler.java	2010-01-16 21:03:11 +0000
@@ -3,7 +3,7 @@
  * Tomboy on Android
  * http://www.launchpad.net/tomdroid
  * 
- * Copyright 2008 Olivier Bilodeau <[e-mail]>
+ * Copyright 2008, 2009, 2010 Olivier Bilodeau <[e-mail]>
  * 
  * This file is part of Tomdroid.
  * 
@@ -23,7 +23,6 @@
 package org.tomdroid.xml;
 
 import org.tomdroid.Note;
-import org.tomdroid.util.XmlUtils;
 import org.xml.sax.Attributes;
 import org.xml.sax.SAXException;
 import org.xml.sax.helpers.DefaultHandler;
@@ -38,22 +37,11 @@
 	// position keepers
 	private boolean inTitleTag = false;
 	private boolean inLastChangeDateTag = false;
-	private boolean inNoteContentTag = false;
 	
 	// -- Tomboy's notes XML tags names --
 	// Metadata related
 	private final static String TITLE = "title";
 	private final static String LAST_CHANGE_DATE = "last-change-date"; 
-	// Style related
-	private final static String NOTE_CONTENT = "note-content";
-	
-	private final static String NS_SIZE = "http://beatniksoftware.com/tomboy/size";
-	private final static String PREFIX_SIZE = "size";
-	private final static String NS_LINK = "http://beatniksoftware.com/tomboy/link";
-	private final static String PREFIX_LINK = "link";
-	
-	// accumulate notecontent is this var since it spans multiple xml tags
-	private StringBuilder xmlContent = new StringBuilder();
 	
 	// link to model 
 	private Note note;
@@ -65,72 +53,30 @@
 	@Override
 	public void startElement(String uri, String localName, String name,	Attributes attributes) throws SAXException {
 		
-		// TODO validate top-level tag for tomboy notes and throw exception if its the wrong version number (maybe offer to parse also?)
-		
-		if (localName.equals(NOTE_CONTENT)) {
+		// TODO validate top-level tag for tomboy notes and throw exception if its the wrong version number (maybe offer to parse also?)		
 
-			// we are under the note-content tag
-			// we will append all its nested tags so I create a string builder to do that
-			inNoteContentTag = true;
-		} else if (localName.equals(TITLE)) {
+		if (localName.equals(TITLE)) {
 			inTitleTag = true;
 		} else if (localName.equals(LAST_CHANGE_DATE)) {
 			inLastChangeDateTag = true;
 		}
 
-		// if we are in note-content, recreate the xml
-		// we're not adding the note-content tags to the xml content
-		if (inNoteContentTag && !localName.equals(NOTE_CONTENT)) {
-			
-			String tag = "<";
-			
-			if (uri != null) {
-				if (uri.equals(NS_LINK)) {
-					tag += PREFIX_LINK+":";
-				} else if (uri.equals(NS_SIZE)) {
-					tag += PREFIX_SIZE+":";
-				}
-			}
-			
-			tag += localName+">";
-			xmlContent.append(tag);
-		}
 	}
 
 	@Override
 	public void endElement(String uri, String localName, String name)
 			throws SAXException {
 
-		if (localName.equals(NOTE_CONTENT)) {
-			inNoteContentTag = false;
-		} else if (localName.equals(TITLE)) {
+		if (localName.equals(TITLE)) {
 			inTitleTag = false;
 		} else if (localName.equals(LAST_CHANGE_DATE)) {
 			inLastChangeDateTag = false;
 		}
-		
-		// if we are in note-content, recreate the xml
-		if (inNoteContentTag) {
-			
-			String tag = "</";
-			
-			if (uri != null) {
-				if (uri.equals(NS_LINK)) {
-					tag += PREFIX_LINK+":";
-				} else if (uri.equals(NS_SIZE)) {
-					tag += PREFIX_SIZE+":";
-				}
-			}
-			
-			tag += localName+">";
-			xmlContent.append(tag);
-		}
 	}
 
 	@Override
 	public void endDocument() throws SAXException {
 		super.endDocument();
-		note.setXmlContent(xmlContent.toString());
 	}
 
 	// FIXME we'll have to think about how we handle the title soon.. IMHO there's a problem with duplicating the data from the <title> tag and also putting it straight into the note.. this will have to be reported to tomboy 
@@ -148,10 +94,5 @@
 //			DateTimeFormatter fmt = ISODateTimeFormat.dateTime();
 //			note.setLastChangeDate(fmt.parseDateTime(currentString));
 		}
-
-		if (inNoteContentTag) {
-			// while we are in note-content, append
-			xmlContent.append(XmlUtils.escape(currentString));
-		}
 	}
 }