I wrote a wrapper library for a REST API in Perl using the Moose library. I would like to gather some feedback on it (mainly on the OOP part since I am new to moose), before pushing it out.
Base class:
has 'debug' => (is => 'rw', isa => 'Bool', default => 0);
has 'api_key' => (is => 'ro', isa => 'Str', required => 1);
has 'api_secret' => (is => 'ro', isa => 'Str', required => 1);
has 'api_base' => (is => 'ro', isa => 'Str', lazy_build => 1);
has 'oauth_client' => (is => 'ro', isa => 'Object', lazy_build => 1);
# Helper methods
method _get {
my $path = shift;
my $json_params = shift;
return $self->_make_request('GET',$path,$json_params);
}
method _make_request {
my $req_type = shift;
my $req_path = shift;
my $req_params_json = shift;
my $url = $self->api_base . '/' . $req_path;
#$req->header( Authorization =>
# "Basic " . encode_base64($self->api_key . ':'));
my $resp;
my $e = eval{
$resp = $self->oauth_client->request(
method => $req_type,
url => $url,
params => {q => $req_params_json}
);
};
if($@) {
$self->_req_error_msg("Request could not be made","","");
}
if ($resp->code == 200) {
my $hash = decode_json($resp->content);
return hash_to_object($hash) if $hash->{object};
if (my $data = $hash->{data}) {
return [ map { hash_to_object($_) } @$data ];
}
return $hash;
}
else {
$self->_req_error_msg("Request failed",$resp->status_line,$resp->content);
}
$e = eval { decode_json($resp->content) };
if ($@) {
$self->_req_error_msg("Could not decode HTTP response",$resp->status_line,$resp->content);
};
#warn "$e\n" if $self->debug;
die "Error occured\n";
}
method _req_error_msg {
my $msg =shift;
my $status_line = shift;
my $content = shift;
print STDERR "Message: $msg\n";
print STDERR "Status Line: $status_line\n";
print STDERR "Content: $content\n";
}
sub hash_to_object {
my $hash = shift;
my $class = 'Net::Semantics3::' . ucfirst($hash->{object});
return $class->new($hash);
}
method _build_api_base { 'https://api.semantics3.com/v1' }
method _build_oauth_client {
my $ua = LWP::UserAgent->new;
$ua->agent("Semantics3 Perl Lib/$VERSION");
my $oauth_client = OAuth::Lite::Consumer->new(
ua => $ua,
consumer_key => $self->api_key,
consumer_secret => $self->api_secret,
);
return $oauth_client;
}
Products.pm
extends 'Net::Semantics3';
use constant MAX_LIMIT => 10;
has 'products_query' => (isa => 'HashRef', is => 'rw', default => sub { my %hash; return \%hash; } );
has 'categories_query' => (isa => 'HashRef', is => 'rw', default => sub { my %hash; return \%hash; } );
has 'query_result' => (isa => 'HashRef', is => 'rw', default => sub { my %hash; return \%hash; } );
has 'sem3_endpoint' => (isa => 'Str', is => 'ro', writer => 'private_set_endpoint', default => "products");
Products: {
#stupid hack to enforce method overloading
method field {
my $field_name = shift || die "A Field Name is required";
my $field_value1 = shift;
my $field_value2 = shift;
if(!defined($field_value1) && !defined($field_value2)) {
die "A Field value is required";
}
if(defined($field_value2)) {
if(!defined($self->products_query->{$field_name})) {
$self->products_query->{$field_name} = {};
}
$self->products_query->{$field_name}->{$field_value1} = $field_value2;
} else {
$self->products_query->{$field_name} = $field_value1;
}
}
method get_products {
$self->_run_query("products",$self->products_query);
return $self->query_result();
}
}
1 Answer 1
In Net::Semantics3 _make_request I wasn't too sure about the error handling. It looks like _req_error_msg doesn't die if any part of the request process fails. Also 200 may not be the only valid HTTP status code, it's safer to use the is_success method
if ($resp->is_success) { #handle response...
You could also clean up the eval calls and use Try::Tiny instead, which I think looks a bit neater.
My Moose is a bit rusty, but I believe you can use class names in type definitions (feel free to correct if this isn't the case)
has 'oauth_client' => (is => 'ro', isa => 'OAuth::Lite::Consumer', lazy_build => 1);
As for the general structure OO structure I would probably have preferred separate classes for representing queries, results and the Net::Semantics3 user agent. Almost analogous to HTTP::Request, LWP::UserAgent and HTTP::Response. If I was using your code I'd like to be able to do something like this:
my $product_query = Net::Semantics3::ProductsQuery->new({brand => "Toshiba television"});
my $net_semantics3_ua = Net::Semantics->new();
my $prod_resultset = $net_semantics3_ua->execute_query($product_query);
foreach my $prod($prod_resultset->all){
say $prod->field_name;
}
There's nothing really wrong with the approach you've taken, but if I read the example code in your test.pl script it doesn't feel like the interface is very perlish. I think you could make it a bit more intuitive for programmers using your API