Closed
Bug 449031
Opened 16 years ago
Closed 16 years ago
Add meta data API to memory/storage
Categories
(Calendar :: Provider: Local Storage, enhancement)
Calendar
Provider: Local Storage
Tracking
(Not tracked)
RESOLVED
FIXED
0.9
People
(Reporter: dbo, Assigned: dbo)
Details
Attachments
(1 file, 1 obsolete file)
28.31 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
For the purpose of synchronizing remote calendars with the local cache, it's sensible to have meta data stored with items.
Flags: wanted-calendar0.9+
Flags: in-testsuite?
Assignee | ||
Comment 1•16 years ago
|
||
patch including unit tests
Assignee: nobody → daniel.boelzle
Status: NEW → ASSIGNED
Attachment #332211 -
Flags: review?(philipp)
Comment 2•16 years ago
|
||
Daniel, For whatever reason, your code doesn't store anything in the database, even though the message "calStorageCalendar: stored meta data: id = ..." is displayed and no exception occurs...
Assignee | ||
Comment 3•16 years ago
|
||
I cannot confirm this: modifying the unit test to leave some items and meta data in works well for me. BTW: I am a bit confused, because a quick glance shows me you're using a memory calendar in the caldav opt patch to store meta data.
Comment 4•16 years ago
|
||
(In reply to comment #3) > I cannot confirm this: modifying the unit test to leave some items and meta > data in works well for me. BTW: I am a bit confused, because a quick glance > shows me you're using a memory calendar in the caldav opt patch to store meta > data. > Hi Daniel, When you adapted my code, you replaced the references to "mDBTwo" with "mDB" for the insert and delete statements but left the other queries untouched. Replacing "mDB" back with "mDBTwo" solves the issue. Regarding the memory calendar, where do you see this? If you look closely, the memory calendar is setup only when the calendar is its own superCalendar (meaning its not cached), otherwise, the property "mTargetCalendar" gets the value of the calendar passed as parameter to replayChangesOn.
Assignee | ||
Comment 5•16 years ago
|
||
Wolfgang, you're right, there's a misalignment with mDb and mDbTwo. However, I am wondering why my unit tests actually pass... seems to be different from "real" application scenario :/ I think using mDb is correct here, since mDbTwo is to be used when querying over secondary tables iterating events/todos.
Attachment #332211 -
Attachment is obsolete: true
Attachment #332252 -
Flags: review?(philipp)
Attachment #332211 -
Flags: review?(philipp)
Comment 6•16 years ago
|
||
Comment on attachment 332252 [details] [diff] [review] patch - v2 >+ * >+ * @attention Attention? I might be wrong, but I think the javadoc identifier you are looking for is @note. >+ this.mInsertMetaData.execute(); >+ this.mInsertMetaData.reset(); >+// LOG("calStorageCalendar: stored meta data: id = " + id + "; value = " + value); >+ }, >+ >+ deleteMetaData: function stor_deleteMetaData(id) { >+ this.mDeleteMetaData(id); >+// LOG("calStorageCalendar: deleted meta data: " + id); Remove commented code here and elsewhere. Also I just noticed mDeleteMetaData() as a function? We usually don't prefix functions with m and since this function does nothing else but to call the other, why not use it right away? >+ try { >+ if (query.step()) { >+ value = query.row.value; >+ } >+ } finally { >+ query.reset(); >+ } Not quite sure, but doesn't this cause a warning like "try without catch" ? >+function testMetaData() { >+ function testMetaData_(cal) { Some comments as to what exactly you are testing would be nice, but not required. r=philipp
Attachment #332252 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 7•16 years ago
|
||
(In reply to comment #6) > (From update of attachment 332252 [details] [diff] [review]) > >+ * > >+ * @attention > Attention? I might be wrong, but I think the javadoc identifier you are looking > for is @note. If we use javadoc... changed that to @note. > >+ this.mInsertMetaData.execute(); > >+ this.mInsertMetaData.reset(); > >+// LOG("calStorageCalendar: stored meta data: id = " + id + "; value = " + value); > >+ }, > >+ > >+ deleteMetaData: function stor_deleteMetaData(id) { > >+ this.mDeleteMetaData(id); > >+// LOG("calStorageCalendar: deleted meta data: " + id); > Remove commented code here and elsewhere. Also I just noticed removed LOGs mDeleteMetaData() > as a function? We usually don't prefix functions with m and since this function > does nothing else but to call the other, why not use it right away? you could call storage statement wrappers as functions if there's one parameter > >+ try { > >+ if (query.step()) { > >+ value = query.row.value; > >+ } > >+ } finally { > >+ query.reset(); > >+ } > Not quite sure, but doesn't this cause a warning like "try without catch" ? no, it's valid Checked in on HEAD and MOZILLA_1_8_BRANCH => FIXED.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 0.9
You need to log in
before you can comment on or make changes to this bug.
Description
•