From 2a64ed887c6884423e4ea092bb9ac63ad7a6e08b Mon Sep 17 00:00:00 2001
From: Dinesh Bhor <dinesh.bhor@nttdata.com>
Date: Fri, 20 Jan 2017 16:08:36 +0530
Subject: [PATCH] Remove duplicate columns from list output

If you specify duplicate fields in --fields, then it prints duplicate
columns on the console. By default 'ID' column is added to the output
so if you specified it in --fields, then it shouldn't be displayed
twice.

A user can pass 'NaMe', ' Name ' or 'naMe' in --fields option and it
displays same name values three times under the user supplied column
names. If a user doesn't pass --fields option, then it shows "Name"
column in the list. To maintain consistency between user supplied
column and default column names, converted it into title case and
removed leading and trailing whitespaces. Kept ID field as capital
only('ID') for consistency.

Closes-Bug: #1659742
Change-Id: I98999e4c5934b56cd2e5a3fac1fe4d2a73a0d5a1
---
 cinderclient/tests/unit/v2/test_shell.py | 8 ++++++++
 cinderclient/tests/unit/v3/test_shell.py | 8 ++++++++
 cinderclient/v2/shell.py                 | 7 ++++++-
 cinderclient/v3/shell.py                 | 7 ++++++-
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/cinderclient/tests/unit/v2/test_shell.py b/cinderclient/tests/unit/v2/test_shell.py
index 5053e78..742e9d9 100644
--- a/cinderclient/tests/unit/v2/test_shell.py
+++ b/cinderclient/tests/unit/v2/test_shell.py
@@ -218,6 +218,14 @@ class ShellTest(utils.TestCase):
         mock_print.assert_called_once_with(mock.ANY, key_list,
             exclude_unavailable=True, sortby_index=0)
 
+    @mock.patch("cinderclient.utils.print_list")
+    def test_list_duplicate_fields(self, mock_print):
+        self.run_command('list --field Status,id,Size,status')
+        self.assert_called('GET', '/volumes/detail')
+        key_list = ['ID', 'Status', 'Size']
+        mock_print.assert_called_once_with(mock.ANY, key_list,
+            exclude_unavailable=True, sortby_index=0)
+
     @mock.patch("cinderclient.utils.print_list")
     def test_list_field_with_tenant(self, mock_print):
         self.run_command('list --field Status,Name,Size,Bootable '
diff --git a/cinderclient/tests/unit/v3/test_shell.py b/cinderclient/tests/unit/v3/test_shell.py
index 152cc56..7bc13f3 100644
--- a/cinderclient/tests/unit/v3/test_shell.py
+++ b/cinderclient/tests/unit/v3/test_shell.py
@@ -83,6 +83,14 @@ class ShellTest(utils.TestCase):
         self.run_command(command)
         self.assert_called('GET', '/volumes/detail?group_id=fake_id')
 
+    @mock.patch("cinderclient.utils.print_list")
+    def test_list_duplicate_fields(self, mock_print):
+        self.run_command('list --field Status,id,Size,status')
+        self.assert_called('GET', '/volumes/detail')
+        key_list = ['ID', 'Status', 'Size']
+        mock_print.assert_called_once_with(mock.ANY, key_list,
+            exclude_unavailable=True, sortby_index=0)
+
     def test_list_availability_zone(self):
         self.run_command('availability-zone-list')
         self.assert_called('GET', '/os-availability-zone')
diff --git a/cinderclient/v2/shell.py b/cinderclient/v2/shell.py
index aa9262c..abc2a29 100644
--- a/cinderclient/v2/shell.py
+++ b/cinderclient/v2/shell.py
@@ -17,6 +17,7 @@
 from __future__ import print_function
 
 import argparse
+import collections
 import copy
 import os
 import warnings
@@ -154,7 +155,11 @@ def do_list(cs, args):
         setattr(vol, 'attached_to', ','.join(map(str, servers)))
 
     if field_titles:
-        key_list = ['ID'] + field_titles
+        # Remove duplicate fields
+        key_list = ['ID']
+        unique_titles = [k for k in collections.OrderedDict.fromkeys(
+            [x.title().strip() for x in field_titles]) if k != 'Id']
+        key_list.extend(unique_titles)
     else:
         key_list = ['ID', 'Status', 'Name', 'Size', 'Volume Type',
                     'Bootable', 'Attached to']
diff --git a/cinderclient/v3/shell.py b/cinderclient/v3/shell.py
index debdd4f..d91ce4e 100644
--- a/cinderclient/v3/shell.py
+++ b/cinderclient/v3/shell.py
@@ -17,6 +17,7 @@
 from __future__ import print_function
 
 import argparse
+import collections
 import os
 
 from oslo_utils import strutils
@@ -171,7 +172,11 @@ def do_list(cs, args):
         setattr(vol, 'attached_to', ','.join(map(str, servers)))
 
     if field_titles:
-        key_list = ['ID'] + field_titles
+        # Remove duplicate fields
+        key_list = ['ID']
+        unique_titles = [k for k in collections.OrderedDict.fromkeys(
+            [x.title().strip() for x in field_titles]) if k != 'Id']
+        key_list.extend(unique_titles)
     else:
         key_list = ['ID', 'Status', 'Name', 'Size', 'Volume Type',
                     'Bootable', 'Attached to']