Closed Bug 399535 Opened 17 years ago Closed 9 years ago

Beef up mozStorage documentation

Categories

(Developer Documentation Graveyard :: Mozilla Platform, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sheppy, Assigned: sdwilsh)

References

Details

(Whiteboard: u=mozdev p=0 c=Platform)

We need to clean up and beef up the mozStorage how-to articles we have at present as well as implement reference documents for all storage-related interfaces. I've marked this as dependent upon several bugs that were previously marked as doc needed so that I can unmark them in favor of this bug since the scope of the work goes beyond just updating some stuff in a minor way.
If I'm correct the interfaces are: mozIStorageConnection * mozIStorageAggregateFunction mozIStorageFunction mozIStorageProgressHandler * mozIStorageStatement * mozIStorageValueArray * They are all documented in reference form at http://developer.mozilla.org/en/docs/mozIStorageConnection created by Arehman4. Those marked with * have a reference page of their own also created by Arehman4 (mozIStorageFunction has an empty page). The real problem as I see it is *finding* the stuff. I only found out about lastInsertRowID by looking at the IDL definition. Then I started putting reference material in the Storage article, only to find out it was redundant. I'm adding some wiki links now. The how-to pages are a different story, the Storage article really needs work. Storage how-to article: - documented lastInsertRowID (with JS example). - replace reference material with links to reference articles - reorganized headings, cleaned up intro, etc Storage reference articles: - added "See also" section: link all reference articles What else is do you think is needed?
We also need all the JS magic stuff: http://mxr.mozilla.org/seamonkey/source/storage/public/mozIStorageStatementWrapper.idl It's essentially complete undocumented. I guess sunbird uses it, and there are some basic tests of it's functionality here: http://mxr.mozilla.org/seamonkey/source/storage/test/unit/test_storage_statement_wrapper.js If I ever get time (darn you school!), I want to revamp the existing storage docs.
This documentation is really still a work in progress. There's plenty of room for improvement but it's better than two months ago when there was essentially none. :)
hey, I'm not complaining - you just asked what else was needed :p
Renaming for clarity.
Summary: Write mozStorage documentation → Beef up mozStorage documentation
We finished these docs up; they're pretty good now, and we have examples finally too.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
This document is still out of date: https://developer.mozilla.org/En/Storage I've been meaning to get to it, but that's been on the todo list for probably a year now :/
In what way is it out of date?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
It talks about invariants that don't exist anymore, etc. This is all stuff I should fix though since I know the code well. We can assign this to me.
Assignee: nobody → sdwilsh
Status: REOPENED → ASSIGNED
OK - I've gone through some of the main Storage page. I've down all the way down to https://developer.mozilla.org/en/Storage#Synchronously. I hope to get the rest done tomorrow, then start tackling the remaining documents. It'd be great if someone could look over what I've already done and provide comments.
OK - all done with https://developer.mozilla.org/En/Storage I tried to make this into more of an overview document, and I'm going to put more details into interfaces and other pages. I removed a bunch of stuff that was either wrong, or I felt was too much detail. I also beefed up the code samples quite a bit, and turned on language formatting in all code samples.
Storage is shaping up to be among the cleanest code and most well documented too! To that end, I provide the following suggestions while also already being very happy with where things are: == Storage * "Statements", "Creating a Statement" The sync IO warning after the description of executeSimpleSQL is almost a non sequitur. Suggest having the executeSimpleSQL say something more like "executeSimpleSQL synchronously executes the provided SQL on the calling thread, returning no results. If you are on the default thread, this will block the user interface. Unless you are doing something that you know will return quickly, you are strongly advised to create and execute an asynchronous statement." Also, it seems bad to use "CREATE TABLE" as an example when there's a (non-deprecated) createTable method. "CREATE VIRTUAL TABLE" is at least a real thing to use it for, but that seems confusing for an example. * "Executing a Statement", "Asynchronously" "A C++ example is omitted here because it would be verbose" seems like a red documentation flag. Presumably an mxr link (possibly indirect off of an identifier) to some C++ code is feasible? You might want to point out that, currently, exceptions in async callbacks get silently eaten (bug 465831) and advise that any meaningful code be wrapped in a try/catch/dump idiom. future work-ish: If you trace the mozIStorageStatementCallback link to mozIStorageRow's getResultByName, it says it returns an nsIVariant. But nsIVariant does not have a devmo page. No one on the way to nsIVariant explains what getResultByName is really returning either, so I think at least a simple nsIVariant explanation should be provided. * "Executing a Statement", "Synchronously": Suggest making the JS and C++ cases completely distinct. As it is, the introduction of step and executeStep in the same sentence with no real explanation of the difference is confusing. This might also help clear up some confusion raised by the "You can still use this helper object" statement, since the lack of a step/executeStep explanation makes that also a bit of a non sequitur. The "SQLite is not a typed database" warning block should probably link to a more detailed page on the matter, as it's not the kind of concept fully explained in two sentences. * "How To Corrupt a Database"' The title is amusing but should be offset by an into explanation about how corrupting a database is bad. For example: "SQLite is very good about maintaining database integrity, but it is not magic; there are things that you can do that will lead to database corruption. You can read more about how it works here (link), but here is a quick list of things you must not do:" == MozIStorageService * backupDatabaseFile I think more explanation of the rationale for the function its and suggested usage would be beneficial. Also, the description that "This method ensures that the backup file being created has a unique name" seems confusing given that the caller apparently provides a non-optional filename. As a user of the storage service, I would be interested in why I should be backing up my database (automatically because it is likely to get corrupted? because the user asked me to? other?), and how to do it properly without filling up the user's hard disk (especially if the unique name logic causes it to not over-write the previous file, etc.) oop, just kept reading, and it's apparently a CYA measure when we are compelled to nuke the database so the program can keep working. So perhaps at least say that. * openSharedDatabase Saying "Read the summary of this interface" without a hyperlink seems mean. (Also mean is me reading a wiki document and not simply making the change.) == MozIStorageConnection * setProgressHandler This dude and/or mozIStorageProgressHandler should probably make mention of the fact that storage's implementation of progress handler does not deal with threads. So if you are using the recommended strategy of using async statements, you need to proxy any UI-touching events back to the main thread.
(In reply to comment #14) > The sync IO warning after the description of executeSimpleSQL is almost a non > sequitur. Suggest having the executeSimpleSQL say something more like > "executeSimpleSQL synchronously executes the provided SQL on the calling > thread, returning no results. If you are on the default thread, this will > block the user interface. Unless you are doing something that you know will > return quickly, you are strongly advised to create and execute an asynchronous > statement." While it is true that the warning is pretty obvious, I'm really trying to discourage the use of it. That's why I'm using the big warning class that includes the big icon. > Also, it seems bad to use "CREATE TABLE" as an example when there's a > (non-deprecated) createTable method. "CREATE VIRTUAL TABLE" is at least a real > thing to use it for, but that seems confusing for an example. Switched to CREATE TEMP TABLE. Virtual tables would add some complexity to this document that I don't really want to do. > You might want to point out that, currently, exceptions in async callbacks get > silently eaten (bug 465831) and advise that any meaningful code be wrapped in a > try/catch/dump idiom. Maybe we can add this to mozIStorageStatement documentation. Don't want it in the overview because I don't want it to be too detail oriented. > future work-ish: If you trace the mozIStorageStatementCallback link to > mozIStorageRow's getResultByName, it says it returns an nsIVariant. But > nsIVariant does not have a devmo page. No one on the way to nsIVariant > explains what getResultByName is really returning either, so I think at least a > simple nsIVariant explanation should be provided. Filed bug 485944 to get it documented. That shouldn't block this though. > Suggest making the JS and C++ cases completely distinct. As it is, the > introduction of step and executeStep in the same sentence with no real > explanation of the difference is confusing. This might also help clear up some > confusion raised by the "You can still use this helper object" statement, since > the lack of a step/executeStep explanation makes that also a bit of a non > sequitur. The more I think about it, the more I think step doesn't bring anything extra to the table and that maybe we should deprecate it. It's been removed from the overview page. > The "SQLite is not a typed database" warning block should probably link to a > more detailed page on the matter, as it's not the kind of concept fully > explained in two sentences. Using links inside of the note template seems to fail... > Saying "Read the summary of this interface" without a hyperlink seems mean. > (Also mean is me reading a wiki document and not simply making the change.) I'm not really sure if that note has any value, other than the thread safety to be honest...
Just rewrote https://developer.mozilla.org/En/MozIStorageStatementParams since it wasn't useful in the first place.
OK, I think for real now I'm done with https://developer.mozilla.org/En/MozIStorageStatement. I refactored it a bunch today and added an additional subsection and added more links back to appropriate sections of the overview document.
https://developer.mozilla.org/En/MozIStorageValueArray is now complete. Bug 487110 has been filed to get the noscript methods added to it (has dependency on mdc bug).
Finished https://developer.mozilla.org/En/MozIStorageFunction. We now have good docs with good sample code on custom functions. Yay!
And completely new documentation (page didn't exist before) for https://developer.mozilla.org/En/MozIStorageAggregateFunction. I had to implement standard deviation for the sample code, so let's hope I got the algorithm right (and that my code would actually compile!)
Component: Documentation Requests → Documentation
Component: Documentation → General
Product: Mozilla Developer Network → Developer Documentation
Component: General → Mozilla Platform
Whiteboard: u=mozdev p=0
Whiteboard: u=mozdev p=0 → u=mozdev p=0 c=Platform
Taking into account the update done by Chris on the Web facing part of this (See: https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API ). I'm marking this FIXED.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.