Proposal:QueryObjectRefactor

From MovableType

Contents


QueryObjectRefactor: Get query methods and mod_perl logic out of the core

Summary

Instead of "if $ENV{MOD_PERL}" being mentioned in a number of places, most things related to HTTP request processing would be in a query object. We would then have conditional logic in primarily one place to tell us which object to load. An outcome of the project would be a new CPAN module which provides the performance of some native Apache APIs, with a CGI.pm compatible interface.


Background / Benefits

  • Lazy Loading. The query object will be loaded on-demand with the first call the query() method, instead absolutely every time, as it is now.
  • Better encapsulation: The query object should start to be referenced as $app->query() everywhere, not $app->{query}
  • mod_perl2 support. A wrapper already exists for mod_perl2, so once the refactor is done, it will easier and cleaner to reach mod_perl2 support.
  • A cleaner core. This refactor will set the stage to move infrastructure details out of the core MT app, making the value-added parts of the application simpler and easier to maintain. In all, 11 methods are recommended for deprecation.
  • More QA. As a CPAN module, the new CGI::Apache::Wrapper will receive plenty of automated testing through the cpan-testers network, and be a product usable for many perl projects. It will have more eyeballs reviewing it besides just the MT community.
  • Known interface. While not Perl hackers are fans of CGI.pm, many are familiar with the interface.


Requirements

  1. All calls to query object go through ->query()
  2. Minimum references to $ENV{MOD_PERL} will remain the core
  3. produce and release CGI::Apache::Wrapper to CPAN

Effect on the platform/existing features/code

See background / benefits section above.

In the short term, MT becomes a bit more complex while it maintains the legacy APIs for compatibility. In the long term, the legacy APIs can be removed.

Effect on existing, upgrading users

Because the current API can continue to work until it is safe to remove it, there are no upgrade concerns.

Effect on future new users

This is a win for new folks looking at MT's API, as they will find a cleaner, simpler core.

Project Plan

Other volunteers are sought to implement the plan.

Implementation Notes

The query() method would be implemented like the one in CGI::Application for forward compatibility, only loading the query object when the query() method is called for the first time. This method in turn calls "cgiapp_get_query". It is in this method that the decision of which query object to choose should happen.

 sub cgiapp_get_query { 
  my $c = shift;
  my $class = ($ENV{MOD_PERL}) ? 'CGI::Wrapper::Apache' : 'CGI';
  eval "require $class";
  return $class->new; 
} 
                                                                                                                             

( It's important to keep this routine named the same for future CGI::App compatibility. ) Note that 'CGI::Apache2::Wrapper' module already exists, and be used as a model for the Apache1 wrapper being written. In the short term, the existing APIs of MT can be preserved. For example $app->get_header() might become refactored to be:

   sub get_header { shift->query->http(@_) }       
                                                                                                                                                                                                     Methods like this can be deprecated in favor of calling the query object methods directly, and then eventually removed.  

Details:

MT::Auth::BasicAuth
   remote_user becomes a wrapper for $app->query->remote_user
   and is scheduled for deprecation.
MT::App
   send_http_headers
       should use $app->query->header
       It has MT-specific bits and should not be deprecated.
   init_request
       This currently sets up the query object. This code
       will be simplified by calling the new wrapper here.
       This code can moved so that it is executed on the first
       call to $app->query.
   init_query
       This customizes the query object a bit in a non-standard way.
       It should be called  from cgiapp_get_query(), but not
       included int the wrapper.
   request_content
       This method has no equivalent in CGI.pm for a good reason: it is not
       related to either GET or POST specs, but storing the full content of an
       HTTP request.  The method is referred to in one place in the MT code
       base, by MT::AtomServer, which accepts a raw XML payload.  It would
       remain untouched for now.
   get_header
       This would be refactored to be $app->query->http()
   set_header
       Remains for now, but the special case for mod_perl can be removed.
   request_method
       maps to $app->query->request_method
       Should be deprecated.
   upload_info
       Will call $app->query->upload and $app->query->uploadInfo.
       The special case for mod_perl can be removed.
       Could be deprecated in favor of calling the above methods directly.
   bake_cookie
       Special case for mod_perl can be removed.
       Not deprecated because of MT-specific logic.
   cookies
       Special case for mod_perl can be removed.
       Deprecated in favor of calling $app->query->cookie (which has a slightly different API)
   run
       Redirects can now handled uniformly, without a special case for mod_perl
   param
       Should be deprecated in favor of direct calls to $app->query->param
       Note: With Apache2, the params are read-only
   delete_param
       becomes a wrapper for $app->query->delete
       Note: In mod_perl2 you can't delete query params, and
       CGI::Apache2::Wrapper accordingly does not provide a delete() method.
   param_hash
       - Should call $app->query->Vars and be deprecated
         in favor of calling that directly.
   query_string
       Should call $app->query->query_string and be deprecated
   app_path
       Should call $app->query->url
       The mod_perl special case can be eliminated.
   script
       Should call $app->query->script_name
       The mod_perl special case can be eliminated.
       No deprecation for now.
   path_info
       Should call $app->query->path_info
       mod_perl code is moved to the new compatibility module
       Can be deprecated in favor of a direct call if you don't mind losing the caching you are doing.
   is_secure
       Becomes a wrapper for $app->query->https
       Can be deprecated in favor of direct calls.
   remote_ip
       Calls $app->query->remote_ip(), eliminating conditional mod_perl logic
       This needs to be implemented in CGI.pm as well as the new mod_perl wrapper:
       http://rt.cpan.org/Ticket/Display.html?id=38884
       MT offers a bit of project-specific value by altering behavior based on
       $app->config->TransparentProxyIPs
   cookie_val
       Becomes a wrapper for " $app->query->cookie(-name => $foo); "
       Can be deprecated in favor of direct calls.

Other notes:

The code base should be purged of calls to "$app->{query}" and use "$app->query" for proper encapsulation, and to allow the lazy-loading to work. If the raw access method continues to need to be supported, using something like 'Data::Lazy' will still allow the query() method to be lazy-loaded.


Resources

Project Team

Below is a list of community members participating in the project. All users are linked to their wiki user page and project leaders are marked with an asterisk.

If you are interested in participating but still not sure, add yourself to the bottom of the list. You can italicize your name if you are interested but undecided.