[djabberd] kane, r795: r6532@coke: josboum | 2008-08-05 16:30...

[Jos I. Boumans]

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");