merge load() method with constructor

bin/account-manager-client.pl

@@ -149,12 +149,11 @@ sub list_test_accounts {
 }
 
 sub parse_federation_metadata {
-    my $federation_metadata = IdPAccountManager::SAMLMetadata->new();
+    my $federation_metadata;
 
     eval {
-        $federation_metadata->load(
-            federation_metadata_file_path =>
-              $configuration->{federation_metadata_file_path}
+        $federation_metadata = IdPAccountManager::SAMLMetadata->new(
+            file => $configuration->{federation_metadata_file_path}
         );
     };
     die "unable to load federation metadata: $EVAL_ERROR" if $EVAL_ERROR;

lib/IdPAccountManager/SAMLMetadata.pm

@@ -11,36 +11,38 @@ use IdPAccountManager::Tools;
 sub new {
     my ($pkg, %args) = @_;
 
-    my $self = {};
+    die "missing argument 'file'" unless defined $args{file};
+    die "non-existing file $args{file}" unless -f $args{file};
+    die "non-readable file $args{file}" unless -r $args{file};
 
-    bless $self, $pkg;
-
-    return $self;
-}
+    open(my $handle, '<', $args{file})
+        or die "failed to open file $args{file}: $ERRNO";
 
-## Load metadata
-sub load {
-    my ($self, %args) = @_;
+    my $parser = XML::LibXML->new();
+    die "Failed to initialize XML parser" unless $parser;
 
-    die "Missing parameter 'federation_metadata_file_path'"
-        unless $args{federation_metadata_file_path};
+    my $doc;
+    eval { $doc = $parser->parse_fh($handle) };
+    die "Failed to parse file $args{file}: $EVAL_ERROR"
+        if $EVAL_ERROR;
 
-    $self->{federation_metadata_file_path} =
-      $args{federation_metadata_file_path};
+    die "Failed to parse file $args{file}: $EVAL_ERROR"
+        unless $doc;
 
-    die "Failed to read $args{federation_metadata_file_path}: $ERRNO"
-        unless -r $self->{federation_metadata_file_path};
+    my $root = $doc->documentElement();
+    my $type = $root->nodeName();
 
-    die "Failed to parse file $args{metadata_file}: $ERRNO"
-        unless $self->{federation_metadata_as_xml} =
-            $self->_get_xml_object($args{federation_metadata_file_path});
+    die "incorrect root element type '$type' for file $args{file}, should be
+        'EntitiesDescriptor'" unless $type =~ /EntitiesDescriptor$/;
 
-    my $root = $self->{federation_metadata_as_xml}->documentElement();
+    my $self = {
+        file => $args{file},
+        doc  => $doc
+    };
 
-    die "Root element of file $args{federation_metadata_file_path} is of type '%s'; should be 'EntitiesDescriptor'", $root->nodeName()
-        unless $root->nodeName() =~ /EntitiesDescriptor$/;
+    bless $self, $pkg;
 
-    return 1;
+    return $self;
 }
 
 ## Parse XML structure of metadata to fill a hashref
@@ -48,7 +50,7 @@ sub parse {
     my ($self, %args) = @_;
 
     my %parser_args = (
-        metadata_as_xml    => $self->{federation_metadata_as_xml},
+        metadata_as_xml    => $self->{doc},
         filter_entity_type => 'sp'
     );
 
@@ -70,37 +72,10 @@ sub print {
     my ($self, $fd) = @_;
     $fd = \*STDOUT unless $fd;
 
-    my $root = $self->{federation_metadata_as_xml}->documentElement();
+    my $root = $self->{doc}->documentElement();
     print $fd $root->toString();
 }
 
-## returns a Lib::XML représenting an XML file
-sub _get_xml_object {
-    my ($self, $metadata_file) = @_;
-
-    die "File $metadata_file not found: $ERRNO"
-        unless -f $metadata_file;
-
-    die "Failed to open file $metadata_file: $ERRNO"
-        unless open FH, $metadata_file;
-
-    my $parser = XML::LibXML->new();
-    die "Failed to initialize XML parser" unless $parser;
-
-    $parser->line_numbers(1);
-    my $doc;
-
-    ## Eval() prevents the parsing from killing the main process
-    eval { $doc = $parser->parse_fh(\*FH) };
-    die "Failed to parse file $metadata_file : $EVAL_ERROR"
-        if $EVAL_ERROR;
-
-    die "Failed to parse file $metadata_file : $EVAL_ERROR"
-        unless $doc;
-
-    return $doc;
-}
-
 ## Parse a SAML federation metadata file
 sub _parse_saml_metadata {
     my ($self, %args) = @_;

lib/IdPAccountManager/WebRequest.pm

@@ -249,7 +249,7 @@ sub req_account_wizard {
     }
 
     eval {
-        $federation_metadata->parse());
+        $federation_metadata->parse();
     };
     if ($EVAL_ERROR) {
         push @{ $self->{param_out}->{errors} }, "internal";
@@ -279,35 +279,32 @@ sub req_select_sp {
         return undef;
     }
 
-    my $federation_metadata = IdPAccountManager::SAMLMetadata->new(
-        logger => $self->{logger}
-    );
+    my $federation_metadata;
 
-    unless (
-        $federation_metadata->load(
-            federation_metadata_file_path =>
-              $self->{configuration}->{federation_metadata_file_path}
-        )
-      )
-    {
+    eval {
+        $federation_metadata = IdPAccountManager::SAMLMetadata->new(
+            file => $self->{configuration}->{federation_metadata_file_path}
+        );
+    };
+    if ($EVAL_ERROR) {
         push @{ $self->{param_out}->{errors} }, "internal";
         $self->{logger}->log(
             level   => LOG_ERROR,
-            message => "Failed to load federation metadata : $ERRNO"
+            message => "Failed to load federation metadata: $EVAL_ERROR"
         );
         return undef;
     }
 
-    unless (
+    eval {
         $federation_metadata->parse(
             filter_entity_id => $self->{param_in}->{sp_entityid}
-        )
-      )
-    {
+        );
+    };
+    if ($EVAL_ERROR) {
         push @{ $self->{param_out}->{errors} }, "internal";
         $self->{logger}->log(
             level   => LOG_ERROR,
-            message => "Failed to parse federation metadata : $ERRNO"
+            message => "Failed to parse federation metadata: $EVAL_ERROR"
         );
         return undef;
     }