Re: Fwd: Idea for a Gofer transport for translating SQL - primarily intended for testing
I suggest that we move this discussion to dbi-dev.
On 11/09/11 18:14, Tim Bunce wrote:
On Fri, Sep 09, 2011 at 03:50:53PM +0100, Andrew Ford wrote:
Currently DBD::Gofer::dr:connect() contains code to set up the transport:
my $transport_class = delete $go_attr{go_transport}
or return $drh->set_err($DBI::stderr, "No transport argument in
'$orig_dsn'");
$transport_class = "DBD::Gofer::Transport::$transport_class"
unless $transport_class =~ /::/;
_load_class($transport_class)
or return $drh->set_err($DBI::stderr, "Can't load $transport_class:
$@");
my $go_transport = eval { $transport_class->new(\%go_attr) }
or return $drh->set_err($DBI::stderr, "Can't instanciate
$transport_class: $@");
I would imagine that we could have a "middleware" keyword that was a
comma-separated list of names that were assumed to be in the
DBDx::GoferMiddleware namespace - these would be installed in
sequence with a similar block of code that follows on from the
snippet above, looking something like:
my $middleware_classes = delete $go_attr{go_middleware};
foreach my $middleware_class (split /,/, $middleware_classes) {
$middleware_class = "DBDx::GoferMiddleware::$middleware_class"
unless $middleware_class =~ /::/;
_load_class($middleware_class)
or return $drh->set_err($DBI::stderr, "Can't load
$middleware_class: $@");
$go_transport->middleware( sub { return
$middleware_class->HANDLER(shift, $go_transport->middleware) } );
My code was wrong, what I meant was:
my $next_middleware = $go_transport->middleware;
$go_transport->middleware( sub { return
$middleware_class->handler(shift, $next_middleware) } );
as in the original code the reference in the closure to $go_transport
would refer to its value when the closure was defined, but invoking the
middleware accessor on that reference would return the middleware value
as it is when the closure is invoked - not when it was defined, whereas
calculating $next_middleware when the closure is defined means that it
will still point to the value of the previous middleware (as it was when
the closure was defined) when it is later used as the closure is invoked.
Each middleware handler would take a request object and a reference
to the next handler in the chain.
Calling $go_transport->middleware within the sub isn't right if we go
with a chain of closures, which I'd prefer. I'd expect that last line
to be something like:
$go_transport->middleware( $middleware_class->new_handler(
$go_transport->middleware ) );
where new_handler would look something like:
sub new_handler {
my ($class, $existing_handler) = @_;
my $new_handler = sub {
my ($self, $request) = @_;
...do something with $request...
my $response = $existing_handler->($self, $request);
...do something with $response...
return $response;
} );
return $new_handler;
}
So at setup-time a chain of closures is created.
The difference between the two approaches is whether the middleware
class provides a new_handler method that builds the closure, or whether
the middleware class provides a handler method that is included in a
closure that the Gofer class constructs. The latter approach would
involve an extra subroutine call per middleware layer at runtime (as you
would end up with a closure calling the middleware handler that calls
the next closure...). Your approach is probably cleaner, but it does
put the onus on middleware writers to build the closures properly
(although that can be mitigated against with good documentation that
provides example code for writing new_hander).
The DSNs would then look like:
dbi:Gofer:transport=null;middleware=adaptor;transform=MyMapper;dsn=dbi:XXX...
I don't see a need for both middleware= and transform=. Once the
middleware concept is in Gofer then middleware=MyMapper would suffice.
(Unless I'm missing something.)
I wasn't clear here - I meant that in my original idea I would have a
middleware component called "adaptor" that took a parameter called
"MyMapper" (this was the name used in my original example - but it could
have been anything). Thinking about it further I got to the idea that
it should be something like (using the same component/parameter names):
dbi:Gofer:transport=null;middleware=adaptor(transform=MyMapper);dsn=dbi:XXX...
(which is the syntax that you suggest in your response, below)
Then thinking about my stored-procedure-to-simple-SQL adaptor, I
realized that that is then application-specific and wouldn't be
something to publish to CPAN, so the example just becomes:
dbi:Gofer:transport=null;middleware=MyApp::MyMapper;dsn=dbi:XXX
As I would probably want to use a
Re: Fwd: Idea for a Gofer transport for translating SQL - primarily intended for testing
On Fri, Sep 09, 2011 at 03:50:53PM +0100, Andrew Ford wrote:
> Currently DBD::Gofer::dr:connect() contains code to set up the transport:
>
>my $transport_class = delete $go_attr{go_transport}
>or return $drh->set_err($DBI::stderr, "No transport argument
> in '$orig_dsn'");
>$transport_class = "DBD::Gofer::Transport::$transport_class"
>unless $transport_class =~ /::/;
>_load_class($transport_class)
>or return $drh->set_err($DBI::stderr, "Can't load
> $transport_class: $@");
>my $go_transport = eval { $transport_class->new(\%go_attr) }
>or return $drh->set_err($DBI::stderr, "Can't instanciate
> $transport_class: $@");
>
> I would imagine that we could have a "middleware" keyword that was a
> comma-separated list of names that were assumed to be in the
> DBDx::GoferMiddleware namespace - these would be installed in
> sequence with a similar block of code that follows on from the
> snippet above, looking something like:
>
>my $middleware_classes = delete $go_attr{go_middleware};
>foreach my $middleware_class (split /,/, $middleware_classes) {
>$middleware_class = "DBDx::GoferMiddleware::$middleware_class"
>unless $middleware_class =~ /::/;
>_load_class($middleware_class)
>or return $drh->set_err($DBI::stderr, "Can't load
> $middleware_class: $@");
>$go_transport->middleware( sub { return
> $middleware_class->HANDLER(shift, $go_transport->middleware) } );
>}
>
> Each middleware handler would take a request object and a reference
> to the next handler in the chain.
Calling $go_transport->middleware within the sub isn't right if we go
with a chain of closures, which I'd prefer. I'd expect that last line
to be something like:
$go_transport->middleware( $middleware_class->new_handler(
$go_transport->middleware ) );
where new_handler would look something like:
sub new_handler {
my ($class, $existing_handler) = @_;
my $new_handler = sub {
my ($self, $request) = @_;
...do something with $request...
my $response = $existing_handler->($self, $request);
...do something with $response...
return $response;
} );
return $new_handler;
}
So at setup-time a chain of closures is created.
> The DSNs would then look like:
>
>
> dbi:Gofer:transport=null;middleware=adaptor;transform=MyMapper;dsn=dbi:XXX...
I don't see a need for both middleware= and transform=. Once the
middleware concept is in Gofer then middleware=MyMapper would suffice.
(Unless I'm missing something.)
> There are a few holes in this that I can think of:
>
>* We might want to instantiate a middleware object for each
> component and use that in installing the middleware handler
I don't see a need. Got a use case?
>* I am not sure how we could specify a handler name other than the
> default for individual middleware components
I don't see a need. Got a use case?
>* Also not sure how attributes for each middleware component could
> be specified such that the component for which the attribute is
> intended could be specified - the example DSN above assumes that
> the attribute (transform in the example DSN) is just dumped into
> the general "go_" attribute hash)
Some smarter parsing should make this style possible:
dbi:Gofer:transport=null;middleware=Foo(a=1,b=2),Bar(a=2,b=1);dsn=dbi:XXX...
Anything more fancy could be done by writing a new module with whatever
logic is needed.
>* The go_middleware attribute could be set in the DBI connect
> attribute hash to a list of subroutine references (code would need
> to differentiate between a comma-separated list value and a list
> reference value)
Umm, firstly having a go_middleware attribute that's different to the
$go_transport->middleware attribute is going to get confusing.
Let's keep 'middleware' as the name for the concept but rename the
go_middleware attribute to go_via. That's shorter and more descriptive:
dbi:Gofer:transport=null;via=MyMapper;dsn=dbi:XXX...
And rename $go_transport->middleware to $go_transport->sender.
That's also shorter and actually more descriptive since it is just a
sender (the fact that the sender code ref may reference layers of
middleware is a separate issue).
Let's ignore the semantics of go_via being a reference for now.
Doesn't seem like a priority.
>* Should the middleware components be listed innermost first (as
> implemented in the pseudo-code above) or outermost first?
Innermost first would match the implementation, i.e., when read left to
right each is wrapping the previous (where 'previous' for the first one
is the transport itself).
>* We might want to add some convenience methods to the request class
> to build respons
