From: Paul L. <pa...@sq...> - 2006-11-14 01:05:51
|
OK, folks, here's to keeping the momentum alive... ready for the pain? I have attached two patches and a tarball of files that show proof of concept of: - Smarty-based template set (very limited set, however - just enough to show it works) - Ajax-ified delete button on (top of) the message view page INTRODUCTION (see below for the discussion points) ################################################## To get this all to work, you must have a CVS snapshot as of at least 2006-11-12, since there are a couple of new commits that I made on 11-11 that are needed. Then, apply the attached patches and untar the attached tarball (untar from the main SM directory). Things that are introduced in the tarball: - JavaScript file with proposed foundation for SquirrelMail Ajax library. Not much to it - don't be scared off. I think with some tweaks and finish-up, it would be nice to put it somewhere where it could become a reference library at least -- normally, I'd say let's put it in the default template set, however, if the default set makes no use of it, it should probably go elsewhere. - Smarty template set. Nothing big here except the use of the Ajax thing for the *top* delete button for the message view screen (see the read_menubar_buttons.tpl template file). - Tentative "API" entry point src/squirrelmail_rpc.php, which currently only accepts "API" calls and not page requests (it still remains to be seen if we WANT/SHOULD/CAN route normal page requests though here), and currently only implements one API point, which is the deletion of one or more messages. - Tentative class that encapsulates a "message". SM already has a "Message" class that is done at a lower level (basically a parsed representation of the mime structure of a message), so I named it "IMAP_Message": it is intended as a higher-level message representation so anyone (plugins, API, etc) can just instantiate it with a mailbox and message ID and then do what they need with that message (retrieve it, delete it, etc). The patches are on two files, and they accomplish the following: src/read_body.php (uses API backend to retrieve and delete messages - needs *SERIOUS* review) include/init.php (simply includes the IMAP_Message class so others can use it) Template files for the API/XML responses will be added to the default template set, but not until we have agreed on basic API architecture: rpc_response_error.tpl and rpc_response_success.tpl -- for now, they are output as echo commands directly inside the squirrelmail_rpc.php file. They are small and inconsequential. Note that to get the new template set working, you have to run config/conf.pl and re-detect template sets, then when you log in and look at your Options->Display Preferences, you shold see a selection for a skin that corresponds to the Smarty template set. To try the Ajax functionality, go view a message, then click the delete button on the top of the page. After a second to contact the server, it should pop up a JavaScript alert that tells you that the message has been deleted and to refresh your messsage list to verify. Implementation of actual refreshing and all other gizmos is left up to the interface designers out there... DISCUSSION POINTS ################# The Smarty thing is irrelevant, but as long as I was sending some template set, I thought I may as well get the Smarty stuff out there. Please don't mistake me for a big Smarty advocate, but it is important to show an example of how to code a Smarty-based skin for SquirrelMail. The Ajax thing is where we get back to the "API" discussion. And, as the rubber meets the road here, it starts to get really ugly really quick. Main things we need to figure out: * I'd like to have us finish off the Ajax JavaScript stuff and put it somewhere for template designers to use. * Implementation of the "API" file This isn't too hard to understand, the main point of contention is if/how/should/can we route normal page requests through here. There are notes in src/squirrelmail_rpc.php about it, but basically if we try it, we have to create page requests that look like: http://example.com/squirrelmail/src/squirrelmail_rpc.php?page=read_body&passed_id=333333... ... and then we need to see if this creates problems with any of our more complex page requests, see how it affects plugins, see if it creates security problems, and decide if it's worth the trouble in the first place (I'm not convinced, especially because it looks like if we keep all our system initialization logic in init.php, there is not much need to over burden the API entry point with page requests that are completely unrelated to API requests). * Implementation of the "IMAP_Message" class: This is really ugly. Marc's going to have to look this over very closely and give his thoughts. We need to give a LOT of thought to whether or not there are problems with things like: how the message attributes are handled, how the mailbox cache is hacked in, how we can better share IMAP connections (I would rather pool and share them from sqimap_login rather than basically keeping a more or less static connection in each IMAP_Message object), etc. There are lots of "FIXME" comments in the proposed files that also have relevant notes. The mailbox caching mechanism is horribly hacked (in the sense that it is known as a global value and manipultaed in numerous places in the code), so pulling that into the IMAP_Message class presents a big challenge, given the completely different sense of scope and state of a OO implementation. It also isn't really documented at all, so my implementation is very very uninformed and probably errant in some fundamental ways. Note also that because it is used as a global all over the place, part of the problem is that the core (at least the un-fixed places) needs to then retrieve it back from the IMAP_Message object... then if it is modified, it needs to be given back to the IMAP_Message, etc. Very ugly. Also, the actual usage of the IMAP_Message class (replacing the message retrieval code and deletion code in read_body.php) works for me, but I'm not sure it is at all correct. People need to look at this before I make any commits to the SM CVS. * There is also a HUGE problem with the SM core in that errors are almost entirely handled *on-the-spot* by spitting out some error to the browser. This is entirely unacceptable (not to mention somewhat naive architecturally) when we are dealing with an API request that cannot return output in this manner. This is pervasive and rooting it out from the core will take some serious effort. Before we embark on that mission, IMO we need to first decide on a better error handling architecture. Do we let the API just start an output buffer and capture errors on its own (NO WAY!)? Do we set a special error handler for the API calls and throw errors from the core that can be caught and returned in the right format for API clients? Do we re-write the core to return error messages (an error class object?) whenever an error happens and make it percolate all the way up through the core functions so the original caller (the API, for instance) gets to decide what to do with the error? I believe we should also create a known set of integer constants for every possible error SM generates. I would normally commit stuff to CVS, but, particularly with the IMAP_Message class implementation, this departs and replaces the SM core code to such a drastic extent that I want to have consensus that we are headed in the right direction first. Marc, you mentioned that you had thoughts on the API design, so you want to pipe up? The biggest and most difficult of this for me was trying to understand how the hell to pick out the message code from read_body.php and pull it into a class format. That code is just sooooo embedded into a global programming scheme that it was very difficult and the implementation is probably qutie flawed. The delete code is still set to call a non-class method -- a global function that was originally written to handle form requests -- IMO we also need to pull this apart and put it in the IMAP_Message class, but here, too, the handling of the message and the mailbox cache was just too much for me to tackle, so as long as it worked for proof-of-concept, I left it alone. Brian mentioned that some of the API functionalities would be very eay because they could just be wrappers on top of code that is already in the core, and while I also thought the same thing, I am no longer that hopeful. Sure, we might actually be able to force the core to respond to a hacked up page request sent by the API code back to the core (?), but I think we want to do this a little better than that. Unless someone sees something I do not, this is going to be a lot of work. - Paul |