Mailing List Archive: Catalyst: Dev

Plugin or controller?

Index | Next | Previous | View Threaded

mdietrich at cpan

Jun 1, 2009, 6:47 AM

Post #1 of 12 (1585 views)
Permalink

Plugin or controller?

Hi,

Matt S Trout redirected me to this list to discuss a "new" plugin for
Catalyst. But I guess I need to reach back a bit.

I surfed on CPAN yesterday and found the module
Catalyst::Controller::RateLimit. One of my first thouhgts was, this
module should has been written as a plugin, not as a controller
because it extends an application with functionality which is neither
business logic nor controller actions.

I then contacted the author and stated my thoughts. Andrey Kostenko ( http://search.cpan.org/~gugu
) answered that he first wrote it as a plugin, but the list
suggested to find another namespace. So he has chosen the Controller
namespace. He suggested to rewrite the module so that it is based on
Algorithm::TokenBucket instead of Algorithm::FloodControl and file it
under the Plugin namespace.

To my module proposing Matt answered:

> It's normal for Catalyst plugins to be rewritten to become controllers
> - trying to go the other way round strongly suggests a design
> mistake to me.

In my opinion, the module Catalyst::Controller::RateLimit should be
Catalyst::Plugin::RateLimit. But may be I'm wrong. What does the
list think about this?

Thanks,
matt

--
rainboxx Matthias Dietrich
Freier Software Engineer

rainboxx | Tel.: +49 (0) 151 / 50 60 78 64
Tölzer Str. 19 | Mail: matt[at]rainboxx.de
70372 Stuttgart | WWW : http://www.rainboxx.de

XING: https://www.xing.com/profile/Matthias_Dietrich18
GULP: http://www.gulp.de/profil/rainboxx.html

Attachments:

PGP.sig (0.19 KB)

bobtfish at bobtfish

Jun 1, 2009, 8:42 AM

Post #2 of 12 (1517 views)
Permalink

Re: Plugin or controller? [In reply to]

Matthias Dietrich wrote:
> In my opinion, the module Catalyst::Controller::RateLimit should be
> Catalyst::Plugin::RateLimit. But may be I'm wrong. What does the list
> think about this?

You haven't in any way explained why you think it _needs_ to be a plugin.

Therefore, it doesn't need to be, and so shouldn't be. ;)

Cheers
t0m

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

mdietrich at cpan

Jun 1, 2009, 8:50 AM

Post #3 of 12 (1515 views)
Permalink

Re: Plugin or controller? [In reply to]

Hi,

> You haven't in any way explained why you think it _needs_ to be a
> plugin.
>
> Therefore, it doesn't need to be, and so shouldn't be. ;)

if you argue that way, nothing needs to be anything (because opinions
are different) ;).

The module provides a thin wrapper around Algorithm::FloodControl and
implements one function that can be called within a controller action
to decide, what to do. It's like the Authorization plugin. And
because it only implements this helper function, the controller
namespace is a bit confusing.

matt

--
rainboxx Matthias Dietrich
Freier Software Engineer

rainboxx | Tel.: +49 (0) 151 / 50 60 78 64
Tölzer Str. 19 | Mail: matt[at]rainboxx.de
70372 Stuttgart | WWW : http://www.rainboxx.de

XING: https://www.xing.com/profile/Matthias_Dietrich18
GULP: http://www.gulp.de/profil/rainboxx.html

Attachments:

PGP.sig (0.19 KB)

jshirley at gmail

Jun 1, 2009, 9:13 AM

Post #4 of 12 (1517 views)
Permalink

Re: Plugin or controller? [In reply to]

On Mon, Jun 1, 2009 at 8:50 AM, Matthias Dietrich <mdietrich[at]cpan.org>wrote:

> Hi,
>
> You haven't in any way explained why you think it _needs_ to be a plugin.
>>
>> Therefore, it doesn't need to be, and so shouldn't be. ;)
>>
>
> if you argue that way, nothing needs to be anything (because opinions are
> different) ;).
>
> The module provides a thin wrapper around Algorithm::FloodControl and
> implements one function that can be called within a controller action to
> decide, what to do. It's like the Authorization plugin. And because it
> only implements this helper function, the controller namespace is a bit
> confusing.
>
>
> matt
>

I'm not sure why $self->flood_control is more confusing than
$c->flood_control. But I'd also probably stuff this in a Model...
$c->model('FloodControl')->register_attempt( $c->controller, $c->user->login
|| $c->req->address) looks more meaningful to me... again, just opinions.

A controller should determine, via configuration, how it can be flooded. If
your application needs such things, I would say that something upstream from
Catalyst itself is a better solution (but applying this at a root chain
would accomplish the same thing).

If it is a plugin, you couldn't allow different rates on a per-controller
basis unless you start giving it per-controller configuration... at which
point there is a bad design when a plugin starts being controller specific.
Exempting a controller from rate limiting would also be difficult from a
plugin perspective. From a model perspective, the same holds true...

There isn't anything in this code that should affect core dispatching or the
framework itself, which is what plugins are for (and that was Tom's point,
as a general rule of thumb if you can't justify a plugin it should not be a
plugin).

-J

mdietrich at cpan

Jun 1, 2009, 9:46 AM

Post #5 of 12 (1515 views)
Permalink

Re: Plugin or controller? [In reply to]

Hi,

> I'm not sure why $self->flood_control is more confusing than $c-
> >flood_control.

I meant the module's name.

> A controller should determine, via configuration, how it can be
> flooded. If your application needs such things, I would say that
> something upstream from Catalyst itself is a better solution (but
> applying this at a root chain would accomplish the same thing).
>
> If it is a plugin, you couldn't allow different rates on a per-
> controller basis unless you start giving it per-controller
> configuration... at which point there is a bad design when a plugin
> starts being controller specific. Exempting a controller from rate
> limiting would also be difficult from a plugin perspective. From a
> model perspective, the same holds true...

Why couldn't that allow different rates on a per-controller basis?
Because you define a name for the rates it can be anything. Even, as
far as I understand the module Algorithm::FloodControl, the actual
value for the rate would then be available from within any controller
without using this controller as a base controller wherever I want to
access the rate limit. A rate limiting is not always bound to only
one controller; it depends on how you use the module.

> There isn't anything in this code that should affect core
> dispatching or the framework itself, which is what plugins are for

Hm, I don't see why Plugin::Authorization falls under this but not
Controller::RateLimit.

matt

--
rainboxx Matthias Dietrich
Freier Software Engineer

rainboxx | Tel.: +49 (0) 151 / 50 60 78 64
Tölzer Str. 19 | Mail: matt[at]rainboxx.de
70372 Stuttgart | WWW : http://www.rainboxx.de

XING: https://www.xing.com/profile/Matthias_Dietrich18
GULP: http://www.gulp.de/profil/rainboxx.html

Attachments:

PGP.sig (0.19 KB)

jshirley at gmail

Jun 1, 2009, 9:56 AM

Post #6 of 12 (1517 views)
Permalink

Re: Plugin or controller? [In reply to]

On Mon, Jun 1, 2009 at 9:46 AM, Matthias Dietrich <mdietrich[at]cpan.org>wrote:

Why couldn't that allow different rates on a per-controller basis? Because
> you define a name for the rates it can be anything. Even, as far as I
> understand the module Algorithm::FloodControl, the actual value for the rate
> would then be available from within any controller without using this
> controller as a base controller wherever I want to access the rate limit. A
> rate limiting is not always bound to only one controller; it depends on how
> you use the module.
>
> There isn't anything in this code that should affect core dispatching or
>> the framework itself, which is what plugins are for
>>
>
> Hm, I don't see why Plugin::Authorization falls under this but not
> Controller::RateLimit.
>
>
So, first, there is no such thing as Catalyst::Plugin::Authorization. When
you first mentioned it I assumed you simply meant Authorization::ACL or
Authorization::Roles. The latter should not be a plugin in my opinion, but
a model.

To talk of Authorization::ACL, the answer to why it is a plugin is very
simple. It modifies dispatching and extends Catalyst. Before the request
is sent to the controller, the aggregated ACL rules are checked on each
request and Catalyst's execute method is wrapped. You can't do this in a
controller, and it is site-wide.

That makes it a plugin.

If you are trying to access an application wide object, it's a model. As
such, I again restate that Catalyst::Controller::RateLimit should simply be
either a Catalyst::Model::Adaptor or a customized Model. It shouldn't be a
controller or a model, since it does have a benefit of being application
wide.

-J

bobtfish at bobtfish

Jun 1, 2009, 10:10 AM

Post #7 of 12 (1519 views)
Permalink

Re: Plugin or controller? [In reply to]

Matthias Dietrich wrote:
>> You haven't in any way explained why you think it _needs_ to be a plugin.
>>
>> Therefore, it doesn't need to be, and so shouldn't be. ;)
>
> if you argue that way, nothing needs to be anything (because opinions
> are different) ;).

No, sorry.

This isn't an arbitrary choice. Some things _NEED_ to be plugins (i.e.
need to be composed onto the application's @ISA).

The _only_ things which need that are things which explicitly want to
change the way setup works, and even then quite often you can use an
application class role.

If it doesn't alter basic framework operation and dispatching in such a
way the it _NEEDS_ to be on @ISA for your application class, then it
shouldn't be in the Catalyst::Plugin namespace.

The canonical example of this being plugins which provide $c->form =
what a massive bucket of fail that one is - changing form system part
way through an applications life, or running more than one form system
is more common than you'd think.

As you're considering an alternate implementation of flood control to
the one which already exists, this also clearly makes a good case that
neither of them should be a plugin.

I'd recommend going and re-reading the advice in
Catalyst::Manual::ExtendingCatalyst. If after reading that it still
isn't clear as to why this should not be a plugin, please tell us
how/why/where it should be expanded to be more clear, and I'll do so.

Thanks
t0m

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

mdietrich at cpan

Jun 1, 2009, 10:11 AM

Post #8 of 12 (1516 views)
Permalink

Re: Plugin or controller? [In reply to]

Hi,

> So, first, there is no such thing as
> Catalyst::Plugin::Authorization. When you first mentioned it I
> assumed you simply meant Authorization::ACL or
> Authorization::Roles. The latter should not be a plugin in my
> opinion, but a model.

sorry, I meant Plugin::Authentication - sorry for that! But as I
understand it, not every plugin module on CPAN should be a plugin?
But when this is based on opinions, who decides which is a plugin and
which not?

> If you are trying to access an application wide object, it's a
> model. As such, I again restate that
> Catalyst::Controller::RateLimit should simply be either a
> Catalyst::Model::Adaptor or a customized Model. It shouldn't be a
> controller or a model, since it does have a benefit of being
> application wide.

I guess you meant "It shouldn't be a controller or a plugin", right?

Don't get me wrong, I don't want to create this as a plugin under all
circumstances. I just want to figure out when a module is a plugin,
when a model and when a controller. But I'm getting closer to the
truth ;).

matt

--
rainboxx Matthias Dietrich
Freier Software Engineer

rainboxx | Tel.: +49 (0) 151 / 50 60 78 64
Tölzer Str. 19 | Mail: matt[at]rainboxx.de
70372 Stuttgart | WWW : http://www.rainboxx.de

XING: https://www.xing.com/profile/Matthias_Dietrich18
GULP: http://www.gulp.de/profil/rainboxx.html

Attachments:

PGP.sig (0.19 KB)

jayk at ion0

Jun 1, 2009, 2:42 PM

Post #9 of 12 (1512 views)
Permalink

Re: Plugin or controller? [In reply to]

Hi Matthias,

There are many 'plugins' that really never should have been plugins.
The methods described in the Extending Catalyst doc were not always
public knowledge and quite a lot got written as plugins when they
should have been something else.

Do not make the mistake of thinking of Catalyst Plugins in the same
way you think of WordPress or MovableType or Drupal plugins. They are
not the same thing at all.

Think more along the lines of apache modules. Plugins are things that
affect the basic functionality of Catalyst, not things that just add a
feature here or there. If it effects core functionality, and _has_ to
be a plugin to access the parts of Catalyst and it's dispatch process
to do it's job, then it should be a plugin... if it doesn't _have_ to
be a plugin in order for it to do it's job, it should not be a plugin.

The fact that the rate limiter _could_ be written as a controller
indicates that it should NOT be a plugin. It doesn't necessarily need
to be a Controller, but it doesn't rely on Cat internals / Dispatch
process, so it should not be a plugin.

Also, plugins affect the core of catalyst and their methods are added
to the $c object. The more plugins you have, the more likely you are
to have conflicts and failures / incompatibilities.. It simply does
not make sense to add anything as a plugin unless it absolutely has to
be.

There are numerous 'plugins' that don't need to be plugins, but are
still for various reasons. Most of the Authentication system, for
example, got moved out of the Plugin namespace almost two years ago.
Only the initial setup hook portion remains as a plugin.

Hope that helps,

Jay

On Jun 1, 2009, at 11:11 AM, Matthias Dietrich wrote:

> Hi,
>
>> So, first, there is no such thing as
>> Catalyst::Plugin::Authorization. When you first mentioned it I
>> assumed you simply meant Authorization::ACL or
>> Authorization::Roles. The latter should not be a plugin in my
>> opinion, but a model.
>
> sorry, I meant Plugin::Authentication - sorry for that! But as I
> understand it, not every plugin module on CPAN should be a plugin?
> But when this is based on opinions, who decides which is a plugin
> and which not?
>
>> If you are trying to access an application wide object, it's a
>> model. As such, I again restate that
>> Catalyst::Controller::RateLimit should simply be either a
>> Catalyst::Model::Adaptor or a customized Model. It shouldn't be a
>> controller or a model, since it does have a benefit of being
>> application wide.
>
> I guess you meant "It shouldn't be a controller or a plugin", right?
>
> Don't get me wrong, I don't want to create this as a plugin under
> all circumstances. I just want to figure out when a module is a
> plugin, when a model and when a controller. But I'm getting closer
> to the truth ;).
>
> matt
>
> --
> rainboxx Matthias Dietrich
> Freier Software Engineer
>
> rainboxx | Tel.: +49 (0) 151 / 50 60 78 64
> Tölzer Str. 19 | Mail: matt[at]rainboxx.de
> 70372 Stuttgart | WWW : http://www.rainboxx.de
>
> XING: https://www.xing.com/profile/Matthias_Dietrich18
> GULP: http://www.gulp.de/profil/rainboxx.html
>
>
>
> _______________________________________________
> Catalyst-dev mailing list
> Catalyst-dev[at]lists.scsys.co.uk
> http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

kiki at bsdro

Jun 2, 2009, 2:48 AM

Post #10 of 12 (1504 views)
Permalink

Re: Plugin or controller? [In reply to]

Matthias Dietrich wrote:
> Don't get me wrong, I don't want to create this as a plugin under all
> circumstances. I just want to figure out when a module is a plugin,
> when a model and when a controller. But I'm getting closer to the
> truth ;).
>
> matt
Disclaimer: IANACD (Core Developer) but here's my take.

A plugin changes fundamental functionality. By fundamental I mean mostly
the request dispatch. For example C::P::Static skips the whole dispatch
process, it just serves static files when the path prefix matches. And,
as pointed out before, C::P::Authorization::* also alters the dispatch
if the authz conditions are not matched. A plugin is loaded in the main
app module (lib/MyApp.pm).

A controller base adds functionality. C::C::HTML::FormFu allows you to
extend one or more of your controllers with HTML::FormFu functionality
(displaying, processing and validating forms). C::C::RequestToken
extends your controllers with token generating functionality. A
controller base class is mostly used as a parent/base to the actual
controller class and usually provides new methods to $self.

Finally, a model provides data. Be it from a database, XML feed,
(XML)RPC, etc. it will supply data on demand. A model is accessed
explicitly by: $c->model('MyModel'). A model's data source can (and
sometimes will) be used outside the web framework too (from a command
line script, cron job, etc.)

HTH

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

pagaltzis at gmx

Jun 2, 2009, 3:31 AM

Post #11 of 12 (1507 views)
Permalink

Re: Plugin or controller? [In reply to]

* Matthias Dietrich <mdietrich[at]cpan.org> [2009-06-01 19:15]:
> But as I understand it, not every plugin module on CPAN should
> be a plugin? But when this is based on opinions, who decides
> which is a plugin and which not?

Itâ€™s *NOT* based on opinions.

There are objective technical reasons for whether something
should be a plugin or not.

The benefit of making something a plugin is access to some Cat
internals and to parts of the dispatch process that other
mechanisms for adding features do not offer.

The drawback is that itâ€™s easy for plugins to conflict because
they all become part of the same global namespace.

Most features do not require such access to internals. There is
no point in incurring the problems caused by plugins if there is
nothing to gain from implementing something as a plugin.

If something needs the access to Cat internals that plugins get,
then and ONLY then it should be a plugin.

Most people who put Catalyst extensions on CPAN made them plugins
anyway in the past, because most people, including at the time
the authors of the Cat documentation, didnâ€™t really know better.

Some of this had to be learned by painful experience.

The documentation has since been fixed.

Much of the newer stuff on CPAN is not being written as a plugin.

Thatâ€™s all as it should be.

Regards,
--
Aristotle Pagaltzis // <http://plasmasturm.org/>

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

dbix-class at trout

Jun 8, 2009, 12:02 PM

Post #12 of 12 (1491 views)
Permalink

Re: Plugin or controller? [In reply to]

On Mon, Jun 01, 2009 at 05:50:57PM +0200, Matthias Dietrich wrote:
> Hi,
>
> >You haven't in any way explained why you think it _needs_ to be a
> >plugin.
> >
> >Therefore, it doesn't need to be, and so shouldn't be. ;)
>
> if you argue that way, nothing needs to be anything (because opinions
> are different) ;).
>
> The module provides a thin wrapper around Algorithm::FloodControl and
> implements one function that can be called within a controller action
> to decide, what to do.

In which case it doesn't need to wrap the request cycle so it should be
a controller superclass, not a plugin.

If the logic gets complicated I'd then make it a controller superclass
that delegates to a model, but I'm not sure in this case it's complex
enough to be worth doing that.

> It's like the Authorization plugin.

Authorization::Roles probably shouldn't have been a plugin either.

I always just use methods on the user object ...

> And
> because it only implements this helper function, the controller
> namespace is a bit confusing.

I'd probably implement it as a role rather than a superclass for 5.80.

--
Matt S Trout Catalyst and DBIx::Class consultancy with a clue
Technical Director and a commit bit: http://shadowcat.co.uk/catalyst/
Shadowcat Systems Limited
mst (@) shadowcat.co.uk http://shadowcat.co.uk/blog/matt-s-trout/

_______________________________________________
Catalyst-dev mailing list
Catalyst-dev[at]lists.scsys.co.uk
http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst-dev

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com