Hi,

On 06 Aug 2008, at 20:02, Martin Atkins wrote:

Brad Fitzpatrick wrote:
Yeah, that works, or some general accessor on vhost.
Perhaps during configuration parsing, we keep a map from classname to instantiated object, and then clients can say $vhost->by_class ("DJabberd::RosterStorage") and then the vhost iterates over all its instantiated objects looking for something that UNIVERSAL::isa ("DJabberd::RosterStorage"), finding first, say, "DJabberd::RosterStorage::SQLite" and caching that in some vhost member hashref to make lookup faster later.

Thanks for the suggestion; I've implemented it in Vhost.pm now as demonstrated by the diff below the sig. I've also added a new test file that checks the full code path, to make sure it works as expected, and doesn't break anything else.

If you like the approach, I'll send it upstream and update the demo app to use
this functionality instead.

Did you mean this to be specifically for RosterStorage?

I ask because for other types it's not uncommon for the same class to be instantiated several times in the same VHost. Consider DJabberd::Component::External, for example.

Since the roster-related hooks have a decline method on $cb I assume that it's intended to be possible for there to be several RosterStorage plugins loaded simultaneously too. I guess you could imagine overlaying a writable sqlite rosterstorage with a read-only LDAP one that declines all writes to allow them to pass through.

The code is implemented so it doesn't care how many plugins there are of a certain type; it simply returns a list of objects matching your criteria. The client can then iterate over these objects, and call methods/inspect them to pick out a specific one,
if so desired.

It'd be possible to add a 'limit => X' or to return only the first one in scalar context for example, but since whatever object that would be depends on the order the plugins were read in (ie, the config file), that might not even be desired feature.

Any suggestions are welcome of course.

Cheers,

--

  Jos Boumans                   http://www.linkedin.com/in/josboumans

  How do I prove I'm not crazy to people who are?

=== lib/DJabberd/VHost.pm
==================================================================
--- lib/DJabberd/VHost.pm       (revision 6911)
+++ lib/DJabberd/VHost.pm       (local)
@@ -40,6 +40,9 @@

disco_kids => {}, # $jid_str -> "Description" - children of this vhost for service discovery
         plugin_types    => {},  # ref($plugin instance) -> 1
+
+ plugin_objects => {}, # __all => [ $p1, $p2, $p3, ... ], # all objects + # class => [ $p2, $p3 ], # all objs that ISA class
     };

     croak("Missing/invalid vhost name") unless
@@ -313,7 +316,14 @@
 sub add_plugin {
     my ($self, $plugin) = @_;
     $logger->info("Adding plugin: $plugin");
+
+    # store that we have one of these types loaded
+    # XXX can we increment this, to show how many are loaded
+    # of a certain type? --kane
     $self->{plugin_types}{ref $plugin} = 1;
+
+    # store the object for later retrieval
+    push @{ $self->{plugin_objects}{__all} ||= [] }, $plugin;
     $plugin->register($self);
 }

@@ -328,11 +338,39 @@
     return scalar @{ $self->{hooks}{$phase} || [] } ? 1 : 0;
 }

+# bool yes/no answer
 sub has_plugin_of_type {
     my ($self, $class) = @_;
     return $self->{plugin_types}{$class};
 }

+# all plugin objects that ISA the class you ask for
+sub find_plugin_object_of_type {
+    my ($self, $class) = @_;
+
+    # if you didn't provide a class, you will get all objects
+    $class ||= '__all';
+
+    # did we do this look up before? return what we
+    # found last time.
+    my $aref = $self->{plugin_objects}{$class};
+    return @$aref if ref $aref;
+
+    # haven't looked at this before, iterate over all objects,
+    # find which one are of the class you want
+    for my $obj (@{ $self->{plugin_objects}{__all} || [] }) {
+
+        # it's the right class, store it now
+        push @{ $self->{plugin_objects}{$class} ||= [] }, $obj
+            if UNIVERSAL::isa( $obj, $class );
+    }
+
+    # return what we found. if that's nothing, store an empty list
+    # so we dont have to iterate through all objects next time the
+    # same requests comes
+    return @{ $self->{plugin_objects}{$class} ||= [] };
+}
+
 sub register_hook {
     my ($self, $phase, $subref) = @_;
Carp::croak("Can't register hook on a non-VHost") unless UNIVERSAL::isa($self, "DJabberd::VHost");



Reply via email to