Feedback on my Perl wrapper library (written using Moose) for a REST API

Question

I wrote a wrapper library for a REST API (https://semantics3.com) 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.

The full repo is on github: https://github.com/Semantics3/semantics3-perl/

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();
    }
}

Thanks!

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