[drupal-devel] [feature] Enable override and extension of core module functions

Sun Sep 11 16:49:17 UTC 2005

Issue status update for 
http://drupal.org/node/29428
Post a follow up: 
http://drupal.org/project/comments/add/29428

 Project:      Drupal
 Version:      cvs
 Component:    module system
 Category:     feature requests
 Priority:     normal
 Assigned to:  Anonymous
 Reported by:  nedjo
 Updated by:   robertDouglass
 Status:       patch (code needs review)

The only way I see to resolve the potential namespace conflict that
you're introducing with this patch would be to do what the theme system
does now; decide on a per-function basis upfront which function version
is going to be used, and it would have to do it by loading all of the
modules and making a map of where the various potential overrides
exist, and then make an interface so that the admin could choose which
one is to be used. In other words, you'd be doing exactly what Java or
PHP5 does when you code this:

object = new SomeClass();
object.callMethod();

Here's a better suggestion; why don't we require that any functionality
that needs to be patched or overridden exist as a class?

robertDouglass

Previous comments:
------------------------------------------------------------------------

Mon, 22 Aug 2005 22:47:09 +0000 : nedjo

Attachment: http://drupal.org/files/issues/module-extension-and-override.patch (1.38 KB)

A frequently requested functionality is the ability to override or
extend core module functions.

This small patch is an initial take on how to enable function extension
and overriding.  It takes the existing module_invoke() function and adds
tests for override and extension functions, in the forms
modulename_originalfunction_override() and
modulename_originalfunction_extension().  So, for example, an override
function for the core taxonomy_node_form() function as defined by a
module named testmodule would be called
testmodule_taxonomy_node_form_override(), and would be run instead of
taxonomy_node_form().  And testmodule_taxonomy_node_form_extension()
would be run every time taxonomy_node_form() was called, hence
"extending" it.

Of course, this approach would only be useful if we converted most - or
all! - of our current module function calls to use module_invoke()...

------------------------------------------------------------------------

Tue, 23 Aug 2005 11:35:06 +0000 : stefan nagtegaal

I'm not sure I get this..

Am I right when I think we can override (and change) any function in
drupal with this patch, which would give us the possibility to:
- override forms;
- override functions which generates the node links, so they (finally)
could be hidden instead of always displayed;
- any other advantages which i'm missing atm? (probably a lot, but
can't think of any right now, right here)

Stefan

------------------------------------------------------------------------

Wed, 24 Aug 2005 00:15:09 +0000 : drumm

-1

This will just encourage hacks and create weird bugs as the code
interactions become more complex.

The example form will be themeable when the new form API is in place.

------------------------------------------------------------------------

Wed, 24 Aug 2005 00:37:49 +0000 : nedjo

Thanks for the comments.

I'm throwing this out not because I think the solution I've suggested
is full or even sound, but to try to stimulate a better solution.

(The example I gave was random and not designed as one particularly
needing overriding or extending.)

Basically, the problem is: because we don't use PHP's class object
functionality or some substitute, we don't have a way of overriding or
extending object methods -- in Drupal, core functions -- beyond the
limited functionality explicitly exposed through hooks.  This problem
has fairly significant impacts.

Consider the taxonomy system and contributed modules.  We've seen
various approaches to enhancing taxonomy display, e.g., taxonomy_menu. 
But, because they can't (readily) add to the existing (core) taxonomy
system, they tend to bypass and replace it, e.g., by creating new urls
and displays.  So we get a variety of competing approaches, each
implementing its own limited set of functionality, often replicating
code.  As site admins, we have to choose between one or another
implementation, rather than being able to seamlessly combine them all.

Ideally, we'd to be able to add just the specific enhancement we want.

Is there another, better way to emulate object method overrides and
extension?  Or should we simply accept this limitation and work on
improving extendability through hooks and theming?

------------------------------------------------------------------------

Wed, 24 Aug 2005 07:26:07 +0000 : chx

Attachment: http://drupal.org/files/issues/extend.patch (1.42 KB)

I think this method would be a better way to extend pretty much
anything. Based on $_GET['q'] and the arguments from theme() you can
determine which theme function called you and can add anything. This
won't allow override but would allow extension. If we want override
then simply replace both hooks with one at the end and use $function()
syntax to enable references.

------------------------------------------------------------------------

Wed, 24 Aug 2005 07:27:09 +0000 : chx

Attachment: http://drupal.org/files/issues/extend_0.patch (1.48 KB)

Corrected.

------------------------------------------------------------------------

Mon, 05 Sep 2005 15:24:32 +0000 : Jose A Reyero

Hey, I'd like very much both ideas, nedjo's and chx's. One for module
calls and the other for themes, but I think they're basically the same
idea, and for sure it would save me the need for a lot of patches, and
maintaining patched versions of Drupal would be much easier.

But I'm thinking of this more simplified implementation:

Instead of having overrides as module hooks, we could allow for generic
'override' functions, to be placed anywhere in the code. This way I
could have them in a site's config file or even in a specific module
for a site. My idea is not using them in generic modules but instead,
using them as some "patching" mechanism.

I'm talking of having somewhere a function like
override_taxonomy_node_form() or override_theme_table(). Having plain
functions instead of hooks could save some time at run time.

In case of module_invoke, you dont need to have  override and extend.
The overriding one can call the original function (or not) and then add
stuff if needed.

Also for theme functions, you don't need pre and post. It would be
enough to pass the result of the original theme function, to add html
before or after, or maybe the overriding code can take care of calling
the original theme funcion if needed.

Hope my comments are some help and please: keep working on this!

I like patch bingo :-)

------------------------------------------------------------------------

Tue, 06 Sep 2005 03:17:14 +0000 : Jaza

I agree with Jose: being able to override function
modulefoo_functionbar() by writing a function
override_modulefoo_functionbar() is definitely the way to go. And it
wouldn't be too hard to implement either, since we already have
module_invoke() and module_invoke_all() set up to handle all function
calls (we'd just need to enforce the use of these functions more
strongly).

What's more, I think that a system like this would be quite consistent
with the current theme override system that we have (i.e.
phptemplate_table() can override theme_table() etc). Because Drupal
lacks the benefits of a system that uses objects and classes, there is
a definite need for a simple and effective way to extend a module.

------------------------------------------------------------------------

Tue, 06 Sep 2005 06:03:09 +0000 : robertDouglass

-1. This sounds tempting like a pot of honey. I think it is not a great
idea, however. One has to ask how far we want to go in hacking in
expensive object orientation functionality (polymorphism) before we
just decide that using some classes here and there is a better idea. 

For the sake of poor-mans-polymorphism we lose all encapsulation. 

Why not put a copy of the module with the alternate function in your
site's folder in the sites folder? That achieves the same, doesn't it?

Where are the benchmarks that show just how much processing time this
takes when a site has lots of modules installed?

How will this affect the memory profile if both original and overriding
versions of a function have to be loaded into memory?

------------------------------------------------------------------------

Tue, 06 Sep 2005 09:27:42 +0000 : killes at www.drop.org

-1 as long as no benchmarks are provided.

------------------------------------------------------------------------

Tue, 06 Sep 2005 14:09:57 +0000 : Jose A Reyero

robertDouglass:
> One has to ask how far we want to go in hacking in expensive object
orientation functionality (polymorphism) before we just decide that
using some classes here and there is a better idea.

You're right, but actually I'm for OOP too

> For the sake of poor-mans-polymorphism we lose all encapsulation.

Well, I was thinking here only of "cheap patching"

I think that "poor-mans-polymorphism" is already implemented all
through Drupal.

> Why not put a copy of the module with the alternate function in your
site's folder in the sites folder? That achieves the same, doesn't it?

You are talking about a patched module file, which is what we are
trying to avoid.

>Where are the benchmarks that show just how much processing time this
takes when a site has lots of modules installed?
>How will this affect the memory profile if both original and
overriding versions of a function have to be loaded into memory?

If implementing as a new hook, yes, maybe benchmarks are needed. But if
it's only a single funcion check, I dont think we need that

------------------------------------------------------------------------

Tue, 06 Sep 2005 22:58:17 +0000 : nedjo

Attachment: http://drupal.org/files/issues/override_functions.patch (1.04 KB)

Here's a two-line patch implementing Jose's idea of function
override_modulename_functionname().

------------------------------------------------------------------------

Wed, 07 Sep 2005 02:38:47 +0000 : Jaza

> Why not put a copy of the module with the alternate function in your
site's folder in the sites folder? That achieves the same, doesn't it?

What you are suggesting is equivalent to telling theme developers:

/We're removing the theme() function from Drupal core. If you want to
override any theme functions from now on, please put a copy of
theme.inc in your 'sites' folder, and modify that copy./

Surely that analogy speaks for itself: module patching is currently a
maintenance nightmare, and this addition will make the nightmare
significantly more pleasant.

------------------------------------------------------------------------

Sun, 11 Sep 2005 15:33:02 +0000 : Jose A Reyero

+1 for nedjo's patch, which looks quite straight forward and powerful.

------------------------------------------------------------------------

Sun, 11 Sep 2005 16:39:38 +0000 : robertDouglass

Doesn't this latest patch allow for namespace conflicts? I thought
$modulename had to be part of the function name.