Bug 430013: Make extensions load their modules like Bugzilla::Extension::Foo::Bar, where Bar.pm is in extensions/Foo/lib/.

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> (module owner) a=mkanat

8 files changed, 270 insertions, 21 deletions

diff --git a/Bugzilla/Extension.pm b/Bugzilla/Extension.pm
index 1046e09ae..08d5c86c3 100644
--- a/Bugzilla/Extension.pm
+++ b/Bugzilla/Extension.pm
@@ -26,7 +26,8 @@ use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Install::Util qw(extension_code_files);
 
-use File::Basename qw(basename);
+use File::Basename;
+use File::Spec;
 
 ####################
 # Subclass Methods #
@@ -45,18 +46,42 @@ sub new {
 
 sub load {
     my ($class, $extension_file, $config_file) = @_;
-    require $config_file if $config_file;
-
     my $package;
+
     # This is needed during checksetup.pl, because Extension packages can 
     # only be loaded once (they return "1" the second time they're loaded,
-    # instead of their name). If an extension has only an Extension.pm,
-    # and no Config.pm, the Extension.pm gets loaded by 
-    # Bugzilla::Install::Requirements before this load() method is ever
-    # called.
+    # instead of their name). During checksetup.pl, extensions are loaded
+    # once by Bugzilla::Install::Requirements, and then later again via
+    # Bugzilla->extensions (because of hooks).
     my $map = Bugzilla->request_cache->{extension_requirement_package_map};
+
+    if ($config_file) {
+        if ($map and defined $map->{$config_file}) {
+            $package = $map->{$config_file};
+        }
+        else {
+            my $name = require $config_file;
+            if ($name =~ /^\d+$/) {
+                ThrowCodeError('extension_must_return_name',
+                               { extension => $config_file, 
+                                 returned  => $name });
+            }
+            $package = "${class}::$name";
+        }
+
+        # This allows people to override modify_inc in Config.pm, if they
+        # want to.
+        if ($package->can('modify_inc')) {
+            $package->modify_inc($config_file);
+        }
+        else {
+            modify_inc($package, $config_file);
+        }
+    }
+
     if ($map and defined $map->{$extension_file}) {
         $package = $map->{$extension_file};
+        $package->modify_inc($extension_file) if !$config_file;
     }
     else {
         my $name = require $extension_file;
@@ -65,6 +90,7 @@ sub load {
                            { extension => $extension_file, returned => $name });
         }
         $package = "${class}::$name";
+        $package->modify_inc($extension_file) if !$config_file;
     }
 
     if (!eval { $package->NAME }) {
@@ -94,6 +120,45 @@ sub load_all {
     return \@packages;
 }
 
+# Modifies @INC so that extensions can use modules like
+# "use Bugzilla::Extension::Foo::Bar", when Bar.pm is in the lib/
+# directory of the extension.
+sub modify_inc {
+    my ($class, $file) = @_;
+    my $lib_dir = File::Spec->catdir(dirname($file), 'lib');
+    # Allow Config.pm to override my_inc, if it wants to.
+    if ($class->can('my_inc')) {
+        unshift(@INC, sub { $class->my_inc($lib_dir, @_); });
+    }
+    else {
+        unshift(@INC, sub { my_inc($class, $lib_dir, @_); });
+    }
+}
+
+# This is what gets put into @INC by modify_inc.
+sub my_inc {
+    my ($class, $lib_dir, undef, $file) = @_;
+    my @class_parts = split('::', $class);
+    my ($vol, $dir, $file_name) = File::Spec->splitpath($file);
+    my @dir_parts = File::Spec->splitdir($dir);
+    # Validate that this is a sub-package of Bugzilla::Extension::Foo ($class).
+    for (my $i = 0; $i < scalar(@class_parts); $i++) {
+        return if !@dir_parts;
+        if (File::Spec->case_tolerant) {
+            return if lc($class_parts[$i]) ne lc($dir_parts[0]);
+        }
+        else {
+            return if $class_parts[$i] ne $dir_parts[0];
+        }
+        shift(@dir_parts);
+    }
+    # For Bugzilla::Extension::Foo::Bar, this would look something like
+    # extensions/Example/lib/Bar.pm
+    my $resolved_path = File::Spec->catfile($lib_dir, @dir_parts, $file_name);
+    open(my $fh, '<', $resolved_path);
+    return $fh;
+}
+
 ####################
 # Instance Methods #
 ####################
@@ -298,6 +363,30 @@ your extension is a single file named C<Foo.pm>.
 If any of this is confusing, just look at the code of the Example extension.
 It uses this method to specify requirements.
 
+=head2 Libraries
+
+Extensions often want to have their own Perl modules. Your extension
+can load any Perl module in its F<lib/> directory. (So, if your extension is 
+F<extensions/Foo/>, then your Perl modules go into F<extensions/Foo/lib/>.)
+
+However, the C<package> name of your libraries will not work quite
+like normal Perl modules do. F<extensions/Foo/lib/Bar.pm> is
+loaded as C<Bugzilla::Extension::Foo::Bar>. Or, to say it another way,
+C<use Bugzilla::Extension::Foo::Bar;> loads F<extensions/Foo/lib/Bar.pm>,
+which should have C<package Bugzilla::Extension::Foo::Bar;> as its package
+name.
+
+This allows any place in Bugzilla to load your modules, which is important
+for some hooks. It even allows other extensions to load your modules. It
+even allows you to install your modules into the global Perl install
+as F<Bugzilla/Extension/Foo/Bar.pm>, if you'd like, which helps allow CPAN
+distribution of Bugzilla extensions.
+
+B<Note:> If you want to C<use> or C<require> a module that's in 
+F<extensions/Foo/lib/> at the top level of your F<Extension.pm>,
+you must have a F<Config.pm> (see above) with at least the C<NAME>
+constant defined in it.
+
 =head2 Disabling Your Extension
 
 If you want your extension to be totally ignored by Bugzilla (it will
diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm
index 26e3a30e5..d29d4cf86 100644
--- a/Bugzilla/Hook.pm
+++ b/Bugzilla/Hook.pm
@@ -22,15 +22,10 @@
 
 package Bugzilla::Hook;
 use strict;
-use Bugzilla::Constants;
 
 sub process {
     my ($name, $args) = @_;
     foreach my $extension (@{ Bugzilla->extensions }) {
-        local @INC = @INC;
-        my $ext_dir = bz_locations()->{'extensionsdir'};
-        my $ext_name = $extension->NAME;
-        unshift(@INC, "$ext_dir/$ext_name/lib");
         if ($extension->can($name)) {
             $extension->$name($args);
         }
diff --git a/extensions/Example/Extension.pm b/extensions/Example/Extension.pm
index 8e3a385d6..290867e0d 100644
--- a/extensions/Example/Extension.pm
+++ b/extensions/Example/Extension.pm
@@ -24,10 +24,11 @@ package Bugzilla::Extension::Example;
 use strict;
 use base qw(Bugzilla::Extension);
 
-use Bugzilla::Util qw(
-    diff_arrays
-    html_quote
-);
+use Bugzilla::Util qw(diff_arrays html_quote);
+
+# This is extensions/Example/lib/Util.pm. I can load this here in my
+# Extension.pm only because I have a Config.pm.
+use Bugzilla::Extension::Example::Util;
 
 use Data::Dumper;
 
@@ -57,7 +58,7 @@ sub auth_login_methods {
     my ($self, $params) = @_;
     my $modules = $params->{modules};
     if (exists $modules->{Example}) {
-        $modules->{Example} = 'extensions/Example/lib/AuthLogin.pm';
+        $modules->{Example} = 'Bugzilla/Extension/Example/Auth/Login.pm';
     }
 }
 
@@ -65,7 +66,7 @@ sub auth_verify_methods {
     my ($self, $params) = @_;
     my $modules = $params->{modules};
     if (exists $modules->{Example}) {
-        $modules->{Example} = 'extensions/Example/lib/AuthVerify.pm';
+        $modules->{Example} = 'Bugzilla/Extension/Example/Auth/Verify.pm';
     }
 }
 
@@ -195,14 +196,14 @@ sub config {
     my ($self, $params) = @_;
 
     my $config = $params->{config};
-    $config->{Example} = "extensions::Example::lib::ConfigExample";
+    $config->{Example} = "Bugzilla::Extension::Example::Config";
 }
 
 sub config_add_panels {
     my ($self, $params) = @_;
     
     my $modules = $params->{panel_modules};
-    $modules->{Example} = "extensions::Example::lib::ConfigExample";
+    $modules->{Example} = "Bugzilla::Extension::Example::Config";
 }
 
 sub config_modify_panels {
@@ -417,7 +418,7 @@ sub webservice {
     my ($self, $params) = @_;
 
     my $dispatch = $params->{dispatch};
-    $dispatch->{Example} = "extensions::Example::lib::WSExample";
+    $dispatch->{Example} = "Bugzilla::Extension::Example::WebService";
 }
 
 sub webservice_error_codes {
diff --git a/extensions/Example/lib/Auth/Login.pm b/extensions/Example/lib/Auth/Login.pm
new file mode 100644
index 000000000..9f4f37dc3
--- /dev/null
+++ b/extensions/Example/lib/Auth/Login.pm
@@ -0,0 +1,32 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Example Plugin.
+#
+# The Initial Developer of the Original Code is Canonical Ltd.
+# Portions created by Canonical are Copyright (C) 2008 Canonical Ltd.
+# All Rights Reserved.
+#
+# Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
+
+package Bugzilla::Extension::Example::Auth::Login;
+use strict;
+use base qw(Bugzilla::Auth::Login);
+use constant user_can_create_account => 0;
+use Bugzilla::Constants;
+
+# Always returns no data.
+sub get_login_info {
+    return { failure => AUTH_NODATA };
+}
+
+1;
diff --git a/extensions/Example/lib/Auth/Verify.pm b/extensions/Example/lib/Auth/Verify.pm
new file mode 100644
index 000000000..0141a0d6a
--- /dev/null
+++ b/extensions/Example/lib/Auth/Verify.pm
@@ -0,0 +1,31 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Example Plugin.
+#
+# The Initial Developer of the Original Code is Canonical Ltd.
+# Portions created by Canonical are Copyright (C) 2008 Canonical Ltd.
+# All Rights Reserved.
+#
+# Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
+
+package Bugzilla::Extension::Example::Auth::Verify;
+use strict;
+use base qw(Bugzilla::Auth::Verify);
+use Bugzilla::Constants;
+
+# A verifier that always fails.
+sub check_credentials {
+    return { failure => AUTH_NO_SUCH_USER };
+}
+
+1;
diff --git a/extensions/Example/lib/Config.pm b/extensions/Example/lib/Config.pm
new file mode 100644
index 000000000..a126e82df
--- /dev/null
+++ b/extensions/Example/lib/Config.pm
@@ -0,0 +1,41 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Example Plugin.
+#
+# The Initial Developer of the Original Code is Canonical Ltd.
+# Portions created by Canonical Ltd. are Copyright (C) 2008
+# Canonical Ltd. All Rights Reserved.
+#
+# Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
+#                 Bradley Baetz <bbaetz@acm.org>
+
+package Bugzilla::Extension::Example::Config;
+use strict;
+use warnings;
+
+use Bugzilla::Config::Common;
+
+sub get_param_list {
+    my ($class) = @_;
+
+    my @param_list = (
+    {
+        name => 'example_string',
+        type => 't',
+        default => 'EXAMPLE',
+    },
+    );
+    return @param_list;
+}
+
+1;
diff --git a/extensions/Example/lib/Util.pm b/extensions/Example/lib/Util.pm
new file mode 100644
index 000000000..596f048e9
--- /dev/null
+++ b/extensions/Example/lib/Util.pm
@@ -0,0 +1,28 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Bug Tracking System.
+#
+# The Initial Developer of the Original Code is Everything Solved, Inc.
+# Portions created by Everything Solved, Inc. are Copyright (C) 2009 
+# Everything Solved, Inc. All Rights Reserved.
+#
+# Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
+
+package Bugzilla::Extension::Example::Util;
+use strict;
+use warnings;
+
+# This file exists only to demonstrate how to use and name your
+# modules in an extension.
+
+1;
diff --git a/extensions/Example/lib/WebService.pm b/extensions/Example/lib/WebService.pm
new file mode 100644
index 000000000..8563ec7f0
--- /dev/null
+++ b/extensions/Example/lib/WebService.pm
@@ -0,0 +1,32 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Bug Tracking System.
+#
+# The Initial Developer of the Original Code is Everything Solved, Inc.
+# Portions created by Everything Solved, Inc. are Copyright (C) 2007 
+# Everything Solved, Inc. All Rights Reserved.
+#
+# Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
+
+package Bugzilla::Extension::Example::WebService;
+use strict;
+use warnings;
+use base qw(Bugzilla::WebService);
+use Bugzilla::Error;
+
+# This can be called as Example.hello() from the WebService.
+sub hello { return 'Hello!'; }
+
+sub throw_an_error { ThrowUserError('example_my_error') }
+
+1;