Include the synchronization configuration in show() output. Provide a

new object method that subclasses can override to add attribute
information to show() and remove the documentation about overriding
show().

5 files changed, 77 insertions, 20 deletions

diff --git a/TODO b/TODO
index 6673cc5..3eb2383 100644
--- a/TODO
+++ b/TODO
@@ -2,8 +2,6 @@
 
 Required to replace leland_srvtab:
 
-* Include sync configuration in show output.
-
 * Add support for limiting the enctypes of created keytabs by setting the
   enctype attribute on the object and include the enctypes in the object
   show display.
diff --git a/docs/design-api b/docs/design-api
index c0af5d5..8c5c1d5 100644
--- a/docs/design-api
+++ b/docs/design-api
@@ -47,6 +47,19 @@ Object API
     a "type_data <attribute>" argument) as part of storing the attribute
     in the database to update the history information.
 
+  attr_show()
+
+    Returns formatted attribution information for inclusion in show().
+    Object implementations should not have to override show() (and
+    generally should not).  Instead, if there is any type-specific data,
+    they should implement this method and return that metadata, formatted
+    as key:value pairs with the keys right-aligned in the first 15
+    characters, followed by a space, a colon, and the value.  Each line
+    should end in a newline.  If any error occurs reading the data, return
+    undef set the internal reror and show() will abort with an error.
+
+    The default implementation of this method returns the empty string.
+
   create(NAME, TYPE, DBH, PRINCIPAL, HOSTNAME [, DATETIME])
 
     Creates a new object of a particular type.  The parent method will
@@ -117,14 +130,6 @@ Object API
     successfully storing the data to update the history and trace
     information.
 
-  show()
-
-    Normally, new backends don't need to override this method, since it
-    displays all the metadata in the database.  It's only necessary to
-    override it if the backend stores additional metadata separately.
-    When overriding, call the parent method first and then edit the
-    resulting string to add additional information as needed.
-
 ACL Verifier API
 
     New ACL verifiers should inherit from Wallet::ACL::Base.  There are
diff --git a/perl/Wallet/Object/Base.pm b/perl/Wallet/Object/Base.pm
index b4e5939..478382e 100644
--- a/perl/Wallet/Object/Base.pm
+++ b/perl/Wallet/Object/Base.pm
@@ -284,6 +284,13 @@ sub attr {
     return;
 }
 
+# Format the object attributes for inclusion in show().  The default
+# implementation just returns the empty string.
+sub attr_show {
+    my ($self) = @_;
+    return '';
+}
+
 # Get or set the expires value of an object.  Expects an expiration time in
 # seconds since epoch.  If setting the expiration, trace information must also
 # be provided.
@@ -502,6 +509,11 @@ sub show {
             if (@flags) {
                 $output .= sprintf ("%15s: %s\n", 'Flags', "@flags");
             }
+            my $attr_output = $self->attr_show;
+            if (not defined $attr_output) {
+                return undef;
+            }
+            $output .= $attr_output;
         }
         next unless defined $data[$i];
         if ($attrs[$i][0] =~ /^ob_(owner|acl_)/) {
@@ -659,6 +671,15 @@ arguments are used for logging and history and should indicate the user and
 host from which the change is made and the time of the change.  Returns true
 on success and false on failure.
 
+=item attr_show()
+
+Returns a formatted text description of the type-specific attributes of the
+object, or undef on error.  The default implementation of this method always
+returns the empty string.  If there are any type-specific attributes set,
+this method should return that metadata, formatted as key: value pairs with
+the keys right-aligned in the first 15 characters, followed by a space, a
+colon, and the value.
+
 =item destroy(PRINCIPAL, HOSTNAME [, DATETIME])
 
 Destroys the object by removing all record of it from the database.  The
@@ -741,14 +762,13 @@ change.
 =item show()
 
 Returns a formatted text description of the object suitable for human
-display, or undef on error.  The default implementation shows all of the
-base metadata about the object, formatted as key: value pairs with the keys
-aligned in the first 15 characters followed by a space, a colon, and the
-value.  If any ACLs or an owner are set, after this data there is a blank
-line and then the information for each unique ACL, separated by blank lines.
-Object implementations with additional data to display can rely on that
-format to add additional settings into the formatted output or at the end
-with a matching format.
+display, or undef on error.  All of the base metadata about the object,
+formatted as key: value pairs with the keys aligned in the first 15
+characters followed by a space, a colon, and the value.  The attr_show()
+method of the object is also called and any formatted output it returns will
+be included.  If any ACLs or an owner are set, after this data there is a
+blank line and then the information for each unique ACL, separated by blank
+lines.
 
 =item store(DATA, PRINCIPAL, HOSTNAME [, DATETIME])
 
diff --git a/perl/Wallet/Object/Keytab.pm b/perl/Wallet/Object/Keytab.pm
index 5f128b0..c78adc2 100644
--- a/perl/Wallet/Object/Keytab.pm
+++ b/perl/Wallet/Object/Keytab.pm
@@ -475,6 +475,19 @@ sub attr {
     }
 }
 
+# Override attr_show to display the sync attribute.
+sub attr_show {
+    my ($self) = @_;
+    my @targets = $self->attr ('sync');
+    if (not @targets and $self->error) {
+        return undef;
+    } elsif (@targets) {
+        return sprintf ("%15s: %s\n", 'Synced with', "@targets");
+    } else {
+        return '';
+    }
+}
+
 # Override create to start by creating the principal in Kerberos and only
 # create the entry in the database if that succeeds.  Error handling isn't
 # great here since we don't have a way to communicate the error back to the
diff --git a/perl/t/keytab.t b/perl/t/keytab.t
index 14e1df7..c419806 100755
--- a/perl/t/keytab.t
+++ b/perl/t/keytab.t
@@ -3,7 +3,7 @@
 #
 # t/keytab.t -- Tests for the keytab object implementation.
 
-use Test::More tests => 158;
+use Test::More tests => 160;
 
 use Wallet::Config;
 use Wallet::Object::Keytab;
@@ -397,7 +397,7 @@ SKIP: {
 
 # Tests for kaserver synchronization support.
 SKIP: {
-    skip 'no keytab configuration', 92 unless -f 't/data/test.keytab';
+    skip 'no keytab configuration', 94 unless -f 't/data/test.keytab';
 
     # Test the principal mapping.  We can do this without having a kaserver
     # configuration.  We only need a basic keytab object configuration.  Do
@@ -444,6 +444,16 @@ SKIP: {
 
     # Test setting synchronization attributes, which can also be done without
     # configuration.
+    my $show = $one->show;
+    $show =~ s/^(\s*Created on:) \d+$/$1 0/mg;
+    my $expected = <<"EOO";
+           Type: keytab
+           Name: wallet/one
+     Created by: $user
+   Created from: $host
+     Created on: 0
+EOO
+    is ($show, $expected, 'Show output displays no attributes');
     is ($one->attr ('foo', [ 'bar' ], @trace), undef,
         'Setting unknown attribute fails');
     is ($one->error, 'unknown attribute foo', ' with the right error');
@@ -464,6 +474,17 @@ SKIP: {
     is (scalar (@targets), 1, ' and now one target is set');
     is ($targets[0], 'kaserver', ' and it is correct');
     is ($one->error, undef, ' and there is no error');
+    $show = $one->show;
+    $show =~ s/^(\s*Created on:) \d+$/$1 0/mg;
+    $expected = <<"EOO";
+           Type: keytab
+           Name: wallet/one
+    Synced with: kaserver
+     Created by: $user
+   Created from: $host
+     Created on: 0
+EOO
+    is ($show, $expected, ' and show now displays the attribute');
 
     # Set up our configuration.
     skip 'no AFS kaserver configuration', 27 unless -f 't/data/test.srvtab';