| Subject: | Some bugs with Xapian model configuration |
I'm using Catalyst::Model::Xapian in my project and found some
minor issues about Xapian model configuration.
First issue is that Xapian model can't be configured via external
configuration file handled by ConfigLoader plugin.
For example, I want to change default 'index' database to 'data/index',
so I have next lines in my 'myapp.yml' config file:
---
Model::Xapian:
db: data/index
...
But it doesn't work because configuration data aren't chained into
Catalyst::Model constructor in line 89 and $self->config is widely used
to retrieve configuration (it returns classwide defaults, not instance
configuration). Real configuration data are merged by Catalyst from
both MyApp and Model::Xapian configurations and passed as second
argument to model constructor and can be accessed through $self->{...}
after model instance would be created.
Next issue is that Search::Xapian::Database::new constructor doesn't
expect reference as its first argument. But MyApp->path_to returns
Path::Class::Dir object. So passing it to Xapian database constructor
causes Xapian to croak. It's simple to reproduce - just type next lines
in MyApp::Model::Xapian.
__PACKAGE__->config(
db => MyApp->path_to('data', 'index'),
);
But it's simple to fix - argument to Search::Xapian::Database
constructor might be explicitly strigified in line 98.
Other minor issues are:
- $c->config->{home}.'/index' is better written as $c->path_to('index')
at line 90;
- $class is better named $self in sub search because it is never called
as class method (I'm not sure, but it really can't before new).
I attached diff with my fixes for Catalyst::Model::Xapian module.
I hope it would be useful to fix described issues.
| Subject: | Xapian.pm.diff |
88,90c88,91
< my ( $self, $c ) = @_;
< $self = $self->NEXT::new($c); my %config = (
< db => $c->config->{home}.'/index',
---
> my ( $class, $c, @args ) = @_;
> my $self = $class->NEXT::new($c, @args);
> %$self = (
> db => $c->path_to('index'),
94c95
< %{ $self->config() },
---
> %$self,
97c98
< $self->db(Search::Xapian::Database->new($config{db}));
---
> $self->db(Search::Xapian::Database->new("$self->{db}"));
100,101c101,102
< if ( defined($config{language}) ) {
< my $stemmer=Search::Xapian::Stem->new($config{language});
---
> if ( defined $self->{language} ) {
> my $stemmer=Search::Xapian::Stem->new($self->{language});
113d113
< $self->config(\%config);
129c129
< my ( $class,$q, $page,$page_size) = @_;
---
> my ( $self, $q, $page, $page_size ) = @_;
132,137c132,137
< $page_size ||= $class->config->{page_size};
< $class->db->reopen();
< my $query=$class->qp->parse_query( $q, 23 );
< my $enq = $class->db->enquire ( $query );
< $class->prepare_enq($enq);
< if( $class->config->{order_by_date} ) {
---
> $page_size ||= $self->{page_size};
> $self->db->reopen();
> my $query = $self->qp->parse_query( $q, 23 );
> my $enq = $self->db->enquire ( $query );
> $self->prepare_enq($enq);
> if( $self->{order_by_date} ) {
145c145
< from_to($q,'utf-8','iso-8859-1') if $class->config->{utf8_query};
---
> from_to($q,'utf-8','iso-8859-1') if $self->{utf8_query};
148c148
< search=>$class,query=>$q,query_obj=>$query,querytime=>$time,page=>$page,page_size=>$page_size });
---
> search=>$self,query=>$q,query_obj=>$query,querytime=>$time,page=>$page,page_size=>$page_size });