Skip to content

Commit

Permalink
Removed the usage of AUTOLOAD in the Dancer2::Plugin code.
Browse files Browse the repository at this point in the history
The context: in the new plugin infrastructure, the plugin code can use DSL keywords, but the keywords need to be the one of the App that where this plugin is running. This can be known only very late in the execution. This association of the keywords with the right app is what I'll call the "mapping". To be able to defer the building of this mapping long enough, previous implementation was using an AUTOLOAD: when a DSL keyword was called, the AUTOLOAD code would get the App that we are currently running under, and map the call to the right keyword implementation.

The issue: using AUTOLOAD is considered dangerous, kind of ugly and error prone, and it has nasty side effect (some of them already woked around in D2::Plugin itself)

The proposed fix: at import time, each time an App loads Dancer2::Plugin, and late enough so that the App is fleshed out, we store the various DSL keywords implementation of this App in a mapping hash. And we inflate the plugin namespace with keywords definition that will choose the right implementation from the mapping hash at run time. There is imho very little magic, which makes this fix a preferred solution compared to AUTOLOAD.
  • Loading branch information
dams authored and veryrusty committed Mar 3, 2016
1 parent e31fd54 commit aa86913
Showing 1 changed file with 37 additions and 16 deletions.
53 changes: 37 additions & 16 deletions lib/Dancer2/Plugin.pm
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ sub _exporter_app {
};
}

# deprecated backwards compat: on_plugin_import()
local $CUR_PLUGIN = $plugin;
$_->($plugin) for @{ $plugin->_DANCER2_IMPORT_TIME_SUBS() };

Expand Down Expand Up @@ -351,9 +350,9 @@ sub _exporter_plugin {
my \$_ClassHooks = [];
sub ClassHooks { \$_ClassHooks }
# deprecated backwards compat
# FIXME should this throw a deprecation notice? probably yes...
sub register_plugin {1}
# this is important as it'll do the keywords mapping between the
# plugin and the app
sub register_plugin { Dancer2::Plugin::register_plugin(@_) }
sub register {
my ( \$keyword, \$sub ) = \@_;
Expand Down Expand Up @@ -439,18 +438,6 @@ END
Dancer2::Core::Hook->new( name => shift, code => shift ) );
}
our \$AUTOLOAD;
sub AUTOLOAD {
my ( \$self, \@args ) = \@_;
my \$method = \$AUTOLOAD;
\$method =~ s/.*:://;
my \$cb = \$self->app->name->can(\$method)
or croak("Can't locate method '\$method'");
#Carp::carp "Using DSL in plugins is deprecated (\$method).";
\$cb->(\@args);
}
}
END

Expand All @@ -460,6 +447,40 @@ END
qw/ plugin_keywords plugin_hooks /;
}


{
# This has to be called at the end of every plugin package, in order to map the
# keywords of the associated app to the plugin, so that these keywords can be
# called from within the plugin code.
my %mapping;
sub register_plugin {

my $plugin_module = caller(1);

my $_DANCER2_IMPORT_TIME_SUBS = $plugin_module->_DANCER2_IMPORT_TIME_SUBS;
unshift(@$_DANCER2_IMPORT_TIME_SUBS, sub {
my $plugin_instance = shift;
my $app_name = $plugin_instance->app->name;
my @keywords = keys %{$app_name->dsl->dsl_keywords};

no strict 'refs';
foreach my $keyword ( @keywords ) {
my $coderef = $app_name->can($keyword);
$mapping{$app_name}{$keyword} = $coderef;
# if not yet defined, inject the keyword in the plugin
# namespace, but make sure the code will always get the
# coderef from the right associated app, because one plugin
# can be used by multiple apps
$plugin_module->can($keyword)
or *{"${plugin_module}::$keyword"} = sub {
my $app_name = shift()->app->name;
$mapping{$app_name}{$keyword}->(@_);
};
}
});
}
}

sub _exporter_expand_sub {
my( $plugin, $name, $args, $global ) = @_;
my $class = $args->{class};
Expand Down

0 comments on commit aa86913

Please sign in to comment.