Attachment 34858 Details for Bug 13506 – Proposed patch

[patch] Proposed patch

0001-Bug-13506-Remove-unused-Sip-Configuration-Classes.patch (text/plain), 7.14 KB, created by Colin Campbell on 2015-01-02 11:14:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Campbell

Created: 2015-01-02 11:14:51 UTC

Size: 7.14 KB

patch

obsolete

>From cee5427134e4d6fab7e3e1623c58fe12705a442a Mon Sep 17 00:00:00 2001
>From: Colin Campbell <colin.campbell@ptfs-europe.com>
>Date: Fri, 2 Jan 2015 09:58:55 +0000
>Subject: [PATCH] Bug 13506: Remove unused Sip/Configuration Classes
>
>Sip::Configuration calls new on Account, Institution and Service
>classes but does not store or subsequently use the returned objects
>( which immediately go out of scope ). Their existence just obscures
>the code and misleads the reader. Remobe them
>
>Removed redundant commented out code from Configyration module which
>was not serving any useful purpose
>
>Ran Configuration.pm through perltidy to make layout more
>consistent
>---
> C4/SIP/Sip/Configuration.pm             | 82 +++++++++++++--------------------
> C4/SIP/Sip/Configuration/Account.pm     | 38 ---------------
> C4/SIP/Sip/Configuration/Institution.pm | 58 -----------------------
> C4/SIP/Sip/Configuration/Service.pm     | 23 ---------
> 4 files changed, 31 insertions(+), 170 deletions(-)
> delete mode 100644 C4/SIP/Sip/Configuration/Account.pm
> delete mode 100644 C4/SIP/Sip/Configuration/Institution.pm
> delete mode 100644 C4/SIP/Sip/Configuration/Service.pm
>
>diff --git a/C4/SIP/Sip/Configuration.pm b/C4/SIP/Sip/Configuration.pm
>index a584469..11fc762 100644
>--- a/C4/SIP/Sip/Configuration.pm
>+++ b/C4/SIP/Sip/Configuration.pm
>@@ -1,4 +1,4 @@
>-# 
>+#
> # parse-config: Parse an XML-format
> # ACS configuration file and build the configuration
> # structure.
>@@ -10,47 +10,41 @@ use strict;
> use warnings;
> use XML::Simple qw(:strict);
> 
>-use Sip::Configuration::Institution;
>-use Sip::Configuration::Account;
>-use Sip::Configuration::Service;
>-
>-my $parser = new XML::Simple( KeyAttr   => { login => '+id',
>-					     institution => '+id',
>-					     service => '+port' },
>-			      GroupTags =>  { listeners => 'service',
>-					      accounts => 'login',
>-					      institutions => 'institution', },
>-			      ForceArray=> [ 'service',
>-					     'login',
>-					     'institution' ],
>-			      ValueAttr =>  { 'error-detect' => 'enabled',
>-					     'min_servers' => 'value',
>-					     'max_servers' => 'value'} );
>+my $parser = new XML::Simple(
>+    KeyAttr => {
>+        login       => '+id',
>+        institution => '+id',
>+        service     => '+port'
>+    },
>+    GroupTags => {
>+        listeners    => 'service',
>+        accounts     => 'login',
>+        institutions => 'institution',
>+    },
>+    ForceArray => [ 'service', 'login', 'institution' ],
>+    ValueAttr  => {
>+        'error-detect' => 'enabled',
>+        'min_servers'  => 'value',
>+        'max_servers'  => 'value'
>+    }
>+);
> 
> sub new {
>-    my ($class, $config_file) = @_;
>+    my ( $class, $config_file ) = @_;
>     my $cfg = $parser->XMLin($config_file);
>     my %listeners;
> 
>-    foreach my $acct (values %{$cfg->{accounts}}) {
>-		new Sip::Configuration::Account $acct;
>-    }
>-
>     # The key to the listeners hash is the 'port' component of the
>     # configuration, which is of the form '[host]:[port]/proto', and
>     # the 'proto' component could be upper-, lower-, or mixed-cased.
>     # Regularize it here to lower-case, and then do the same below in
>     # find_server() when building the keys to search the hash.
> 
>-    foreach my $service (values %{$cfg->{listeners}}) {
>-		new Sip::Configuration::Service $service;
>-		$listeners{lc $service->{port}} = $service;
>+    foreach my $service ( values %{ $cfg->{listeners} } ) {
>+        $listeners{ lc $service->{port} } = $service;
>     }
>     $cfg->{listeners} = \%listeners;
> 
>-    foreach my $inst (values %{$cfg->{institutions}}) {
>-		new Sip::Configuration::Institution $inst;
>-    }
>     return bless $cfg, $class;
> }
> 
>@@ -58,36 +52,22 @@ sub error_detect {
>     my $self = shift;
>     return $self->{'error-detect'};
> }
>+
> sub accounts {
>     my $self = shift;
>-    return values %{$self->{accounts}};
>+    return values %{ $self->{accounts} };
> }
> 
>-# sub policy {
>-#     my $self = shift;
>-#     return values %{$self->{policy}};
>-# }
>-
> sub find_service {
>-    my ($self, $sockaddr, $port, $proto) = @_;
>+    my ( $self, $sockaddr, $port, $proto ) = @_;
>     my $portstr;
>-	foreach my $addr ('', '*:', "$sockaddr:", "[$sockaddr]:") {
>-		$portstr = sprintf("%s%s/%s", $addr, $port, lc $proto);
>-		Sys::Syslog::syslog("LOG_DEBUG", "Configuration::find_service: Trying $portstr");
>-		last if (exists(($self->{listeners})->{$portstr}));
>-	}
>+    foreach my $addr ( '', '*:', "$sockaddr:", "[$sockaddr]:" ) {
>+        $portstr = sprintf( "%s%s/%s", $addr, $port, lc $proto );
>+        Sys::Syslog::syslog( "LOG_DEBUG",
>+            "Configuration::find_service: Trying $portstr" );
>+        last if ( exists( ( $self->{listeners} )->{$portstr} ) );
>+    }
>     return $self->{listeners}->{$portstr};
> }
> 
> 1;
>-__END__
>-
>-    my $config = new Sip::Configuration $ARGV[0];
>-
>-
>-foreach my $acct ($config->accounts) {
>-    print "Found account '", $acct->name, "', part of '"
>-    print $acct->institution, "'\n";
>-}
>-
>-1;
>diff --git a/C4/SIP/Sip/Configuration/Account.pm b/C4/SIP/Sip/Configuration/Account.pm
>deleted file mode 100644
>index 1867a4c..0000000
>--- a/C4/SIP/Sip/Configuration/Account.pm
>+++ /dev/null
>@@ -1,38 +0,0 @@
>-#
>-#
>-#
>-#
>-
>-package Sip::Configuration::Account;
>-
>-use strict;
>-use warnings;
>-
>-sub new {
>-    my ($class, $obj) = @_;
>-    my $type = ref($class) || $class;
>-
>-    if (ref($obj) eq "HASH") {
>-	# Just bless the object
>-	return bless $obj, $type;
>-    }
>-
>-    return bless {}, $type;
>-}
>-
>-sub id {
>-    my $self = shift;
>-    return $self->{id};
>-}
>-
>-sub institution {
>-    my $self = shift;
>-    return $self->{institution};
>-}
>-
>-sub password {
>-    my $self = shift;
>-    return $self->{password};
>-}
>-
>-1;
>diff --git a/C4/SIP/Sip/Configuration/Institution.pm b/C4/SIP/Sip/Configuration/Institution.pm
>deleted file mode 100644
>index cd2c80c..0000000
>--- a/C4/SIP/Sip/Configuration/Institution.pm
>+++ /dev/null
>@@ -1,58 +0,0 @@
>-#
>-#
>-#
>-#
>-
>-package Sip::Configuration::Institution;
>-
>-use strict;
>-use warnings;
>-
>-sub new {
>-    my ($class, $obj) = @_;
>-    my $type = ref($class) || $class;
>-
>-    if (ref($obj) eq "HASH") {
>-    # Just bless the object
>-    return bless $obj, $type;
>-    }
>-
>-    return bless {}, $type;
>-}
>-
>-sub name {
>-    my $self = shift;
>-    return $self->{name};
>-}
>-
>-sub id {
>-    my $self = shift;
>-    return $self->{id};
>-}
>-
>-sub implementation {
>-    my $self = shift;
>-    return $self->{implementation};
>-}
>-
>-sub policy {
>-    my $self = shift;
>-    return $self->{policy};
>-}
>-
>-# 'policy' => {
>-#     'checkout' => 'true',
>-#     'retries' => 5,
>-#     'checkin' => 'true',
>-#     'timeout' => 25,
>-#     'status_update' => 'false',
>-#     'offline' => 'false',
>-#     'renewal' => 'true'
>-# },
>-
>-sub parms {
>-    my $self = shift;
>-    return $self->{parms};
>-}
>-
>-1;
>diff --git a/C4/SIP/Sip/Configuration/Service.pm b/C4/SIP/Sip/Configuration/Service.pm
>deleted file mode 100644
>index 34a02b1..0000000
>--- a/C4/SIP/Sip/Configuration/Service.pm
>+++ /dev/null
>@@ -1,23 +0,0 @@
>-#
>-#
>-#
>-#
>-
>-package Sip::Configuration::Service;
>-
>-use strict;
>-use warnings;
>-
>-sub new {
>-    my ($class, $obj) = @_;
>-    my $type = ref($class) || $class;
>-
>-    if (ref($obj) eq "HASH") {
>-	# Just bless the object
>-	return bless $obj, $type;
>-    }
>-
>-    return bless {}, $type;
>-}
>-
>-1;
>-- 
>2.1.0
>

Actions: View | Diff | Splinter Review

Attachments on bug 13506: 34858 | 35943 | 36083 | 36084