Repository: DevoInc/python-sdk

Issue:
Lookup.send_headers method not working as expected with key_index parameter
By checking the Lookup docs, I can see the following parameters recommendation for the `Lookup.send_headers` method:
https://github.com/DevoInc/python-sdk/blob/84a4e2c9da8d3013cd8f012e4399994d64817035/docs/sender/lookup.md#L107-L108

If we check the `Lookup.send_headers` method: 
https://github.com/DevoInc/python-sdk/blob/84a4e2c9da8d3013cd8f012e4399994d64817035/devo/sender/lookup.py#L64-L78

We can see that the `key` parameter has the default `"KEY"` value. If we now check the method's first call, `Lookup.list_to_headers`:
https://github.com/DevoInc/python-sdk/blob/84a4e2c9da8d3013cd8f012e4399994d64817035/devo/sender/lookup.py#L308-L334

We can see that the initial `out` will be always assigned since `key` is the first evaluated variable, leaving `key_index` unusable when the call comes from the initial method.

All this means that a snippet like this one:

```python
headers = ["col1", "col2", "col3"]
fields = ["a", "b", "c"]
lookup.send_headers(headers=headers, key_index=0, event="START", action="FULL")
lookup.send_data_line(key_index=0, fields=fields)
lookup.send_headers(headers=headers, key_index=0, event="END", action="FULL")
```

Will create a lookup like this one: 

| KEY | col1 | col2 | col3 |
|-----|------|------|------|
| a   | b    | c    |      |

Which is not the expected/desired behaviour and I believe should be fixed.

Here are the solutions I can propose:

1. Remove the `"KEY"` default argument → that might introduce some breaking changes; we do not know if someone is using this method expecting this behaviour.
2. Switch the `Lookup.list_to_headers` condition order → apparently this approach is less harmful and would not require changing any method's signature but I am unsure if this will work or not.

[EXTRA] Besides that, I also found that in `Lookup.process_fields` we are deleting the key's field:
https://github.com/DevoInc/python-sdk/blob/84a4e2c9da8d3013cd8f012e4399994d64817035/devo/sender/lookup.py#L360-L368
I think we should not modify what we are passing by argument since the user might want to do something else with those fields later. For instance, this snippet:

```python
fields = ["a", "b", "c"]
lookup.send_data_line(key_index=0, fields=fields)
logger.debug(f"sent {item}")
```

Would print `["b", "c"]` only.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d82fb96..28e86d5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## Unreleased - 2021-11-18
 ### Fixed
 * Fix and skip some non-working tests
+* `Lookup.send_headers` method not working as expected with key_index parameter
+* Add `type_of_key` parameter to `Lookup.send_headers` method
+* Avoid lookups' key element to be deleted when `Lookup.send_data_line` is invoked.
 
 
 ## [3.4.2] - 2021-05-18
diff --git a/devo/sender/lookup.py b/devo/sender/lookup.py
index a0ecea0..d92908e 100644
--- a/devo/sender/lookup.py
+++ b/devo/sender/lookup.py
@@ -1,9 +1,9 @@
 # -*- coding: utf-8 -*-
 """ File with utils for send Lookups to Devo """
-import time
-import sys
 import csv
 import re
+import sys
+import time
 
 
 def find_key_index(value=None, headers=None):
@@ -62,7 +62,8 @@ class Lookup:
     # Helper methods
     # --------------------------------------------------------------------------
     def send_headers(self, headers=None, key="KEY", event='START',
-                     action='FULL', types=None, key_index=None):
+                     action='FULL', types=None, key_index=None,
+                     type_of_key="str"):
         """
         Send only the headers
         :param headers: list with headers names
@@ -71,10 +72,12 @@ class Lookup:
         :param event: START or END
         :param action: FULL or INC to send new full lookup or for update one
         :param types: dict with types of each header
+        :param type_of_key: the key's type (str by default)
         :return:
         """
         p_headers = Lookup.list_to_headers(headers=headers, key=key,
-                                           types=types, key_index=key_index)
+                                           types=types, key_index=key_index,
+                                           type_of_key=type_of_key)
         self.send_control(event=event, headers=p_headers, action=action)
 
     def send_data_line(self, key=None, fields=None,
@@ -320,16 +323,16 @@ class Lookup:
         :result str:
         """
         # First the key
-        if key is not None:
-            out = '[{"%s":{"type":"%s","key":true}}' % (key, types[key_index]
-                                                        if key_index and types
-                                                        else type_of_key)
-        elif key_index is not None:
+        if key_index is not None:
             key = headers[key_index]
             out = '[{"%s":{"type":"%s","key":true}}' % (key,
                                                         types[key_index]
                                                         if types
                                                         else type_of_key)
+        elif key is not None:
+            out = '[{"%s":{"type":"%s","key":true}}' % (key, types[key_index]
+                                                        if key_index and types
+                                                        else type_of_key)
         else:
             raise Exception("Not key identified")
 
@@ -365,10 +368,10 @@ class Lookup:
         :return:
         """
         # First the key
-        out = '%s' % Lookup.clean_field(fields.pop(key_index))
+        out = '%s' % Lookup.clean_field(fields[key_index])
 
         # The rest of the fields
-        for item in fields:
+        for item in fields[:key_index] + fields[key_index + 1:]:
             out += Lookup.field_to_str(item)
         return out
 


Test patch:
diff --git a/tests/sender/cli.py b/tests/sender/cli.py
index 5347e20..6885cea 100644
--- a/tests/sender/cli.py
+++ b/tests/sender/cli.py
@@ -99,8 +99,8 @@ class TestSender(unittest.TestCase):
                                       "--cert", self.cert,
                                       "--chain", self.chain,
                                       "--tag", self.my_app,
-                                      "--verify_mode", 1,
-                                      '--check_hostname', True,
+                                      "--verify_mode", 0,
+                                      '--check_hostname', False,
                                       "--line", "Test line"])
 
         self.assertIsNone(result.exception)
diff --git a/tests/sender/send_lookup.py b/tests/sender/send_lookup.py
index f7a0652..4e7e416 100644
--- a/tests/sender/send_lookup.py
+++ b/tests/sender/send_lookup.py
@@ -1,147 +1,248 @@
 import unittest
 from ssl import CERT_NONE
+from unittest import mock
+
 from devo.sender import Sender, SenderConfigSSL, Lookup
 from .load_certs import *
 
 
 class TestLookup(unittest.TestCase):
     def setUp(self):
-        self.server = os.getenv('DEVO_SENDER_SERVER', "127.0.0.1")
-        self.port = int(os.getenv('DEVO_SENDER_PORT', 4488))
+        self.server = os.getenv("DEVO_SENDER_SERVER", "127.0.0.1")
+        self.port = int(os.getenv("DEVO_SENDER_PORT", 4488))
 
-        self.key = os.getenv('DEVO_SENDER_KEY', CLIENT_KEY)
-        self.cert = os.getenv('DEVO_SENDER_CERT', CLIENT_CERT)
-        self.chain = os.getenv('DEVO_SENDER_CHAIN', CLIENT_CHAIN)
+        self.key = os.getenv("DEVO_SENDER_KEY", CLIENT_KEY)
+        self.cert = os.getenv("DEVO_SENDER_CERT", CLIENT_CERT)
+        self.chain = os.getenv("DEVO_SENDER_CHAIN", CLIENT_CHAIN)
 
-        self.lookup_name = 'Test_Lookup_of_today'
-        self.lookup_file = "".join((os.path.dirname(os.path.abspath(__file__)),
-                                    os.sep, "testfile_lookup.csv"))
+        self.lookup_name = "Test_Lookup_of_today"
+        self.lookup_file = "".join(
+            (
+                os.path.dirname(os.path.abspath(__file__)),
+                os.sep,
+                "testfile_lookup.csv",
+            )
+        )
 
-        self.lookup_key = 'KEY'
+        self.lookup_key = "KEY"
 
     def test_ssl_lookup_csv_send(self):
 
-        engine_config = SenderConfigSSL(address=(self.server, self.port),
-                                        key=self.key, cert=self.cert,
-                                        chain=self.chain,
-                                        check_hostname=False,
-                                        verify_mode=CERT_NONE)
+        engine_config = SenderConfigSSL(
+            address=(self.server, self.port),
+            key=self.key,
+            cert=self.cert,
+            chain=self.chain,
+            check_hostname=False,
+            verify_mode=CERT_NONE,
+        )
         con = Sender(engine_config)
         lookup = Lookup(name=self.lookup_name, historic_tag=None, con=con)
 
         with open(self.lookup_file) as f:
             line = f.readline()
 
-        lookup.send_csv(self.lookup_file,
-                        headers=line.rstrip().split(","),
-                        key=self.lookup_key)
+        lookup.send_csv(
+            self.lookup_file,
+            headers=line.rstrip().split(","),
+            key=self.lookup_key,
+        )
 
         con.socket.shutdown(0)
 
     # Add new line to lookup
     def test_ssl_lookup_new_line(self):
-        engine_config = SenderConfigSSL(address=(self.server, self.port),
-                                        key=self.key, cert=self.cert,
-                                        chain=self.chain,
-                                        check_hostname=False,
-                                        verify_mode=CERT_NONE)
+        engine_config = SenderConfigSSL(
+            address=(self.server, self.port),
+            key=self.key,
+            cert=self.cert,
+            chain=self.chain,
+            check_hostname=False,
+            verify_mode=CERT_NONE,
+        )
         con = Sender(engine_config)
 
         lookup = Lookup(name=self.lookup_name, historic_tag=None, con=con)
-        p_headers = Lookup.list_to_headers(['KEY', 'HEX', 'COLOR'], 'KEY')
-        lookup.send_control('START', p_headers, 'INC')
+        p_headers = Lookup.list_to_headers(["KEY", "HEX", "COLOR"], "KEY")
+        lookup.send_control("START", p_headers, "INC")
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
+            raise Exception("Not msg sent!")
         lookup.send_data_line(key="11", fields=["11", "HEX12", "COLOR12"])
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
-        lookup.send_control('END', p_headers, 'INC')
+            raise Exception("Not msg sent!")
+        lookup.send_control("END", p_headers, "INC")
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
+            raise Exception("Not msg sent!")
+
+        con.socket.shutdown(0)
+
+    def test_create_lookup_key_index_preserves_structure(self):
+        engine_config = SenderConfigSSL(
+            address=(self.server, self.port),
+            key=self.key,
+            cert=self.cert,
+            chain=self.chain,
+            check_hostname=False,
+            verify_mode=CERT_NONE,
+        )
+        con = Sender(engine_config)
+        lookup = Lookup(name=self.lookup_name, con=con)
+        headers = ["col1", "col2", "col3"]
+        fields = ["a", "b", "c"]
+
+        expected_headers = '[{"col1":{"type":"str","key":true}},{"col2":{"type":"str"}},{"col3":{"type":"str"}}]'
+        with mock.patch.object(
+            lookup, "send_control", wraps=lookup.send_control
+        ) as lookup_spy:
+            lookup.send_headers(
+                headers=headers, key_index=0, event="START", action="FULL"
+            )
+            lookup_spy.assert_called_with(
+                action="FULL", event="START", headers=expected_headers
+            )
+            lookup.send_data_line(key_index=0, fields=fields)
+            lookup.send_headers(
+                headers=headers, key_index=0, event="END", action="FULL"
+            )
+            lookup_spy.assert_called_with(
+                action="FULL", event="END", headers=expected_headers
+            )
+        con.socket.shutdown(0)
 
+    def test_send_headers_with_type_of_key(self):
+        engine_config = SenderConfigSSL(
+            address=(self.server, self.port),
+            key=self.key,
+            cert=self.cert,
+            chain=self.chain,
+            check_hostname=False,
+            verify_mode=CERT_NONE,
+        )
+        con = Sender(engine_config)
+        lookup = Lookup(name=self.lookup_name, con=con)
+        headers = ["col1", "col2", "col3"]
+
+        expected_headers = '[{"col1":{"type":"int4","key":true}},{"col2":{"type":"str"}},{"col3":{"type":"str"}}]'
+        with mock.patch.object(
+            lookup, "send_control", wraps=lookup.send_control
+        ) as lookup_spy:
+            lookup.send_headers(
+                headers=headers,
+                key_index=0,
+                type_of_key="int4",
+                event="START",
+                action="FULL",
+            )
+            lookup_spy.assert_called_with(
+                action="FULL", event="START", headers=expected_headers
+            )
         con.socket.shutdown(0)
 
     # add new line deleting previous data
     def test_ssl_lookup_override(self):
-        engine_config = SenderConfigSSL(address=(self.server, self.port),
-                                        key=self.key, cert=self.cert,
-                                        chain=self.chain,
-                                        check_hostname=False,
-                                        verify_mode=CERT_NONE)
+        engine_config = SenderConfigSSL(
+            address=(self.server, self.port),
+            key=self.key,
+            cert=self.cert,
+            chain=self.chain,
+            check_hostname=False,
+            verify_mode=CERT_NONE,
+        )
         con = Sender(engine_config)
 
         lookup = Lookup(name=self.lookup_name, historic_tag=None, con=con)
-        p_headers = Lookup.list_to_headers(['KEY', 'HEX', 'COLOR'], 'KEY')
-        lookup.send_control('START', p_headers, 'FULL')
+        p_headers = Lookup.list_to_headers(["KEY", "HEX", "COLOR"], "KEY")
+        lookup.send_control("START", p_headers, "FULL")
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
+            raise Exception("Not msg sent!")
         lookup.send_data_line(key="11", fields=["11", "HEX12", "COLOR12"])
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
-        lookup.send_control('END', p_headers, 'FULL')
+            raise Exception("Not msg sent!")
+        lookup.send_control("END", p_headers, "FULL")
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
+            raise Exception("Not msg sent!")
 
         con.socket.shutdown(0)
 
     # delete a line from lookup
     def test_ssl_lookup_delete_line(self):
-        engine_config = SenderConfigSSL(address=(self.server, self.port),
-                                        key=self.key, cert=self.cert,
-                                        chain=self.chain,
-                                        check_hostname=False,
-                                        verify_mode=CERT_NONE)
+        engine_config = SenderConfigSSL(
+            address=(self.server, self.port),
+            key=self.key,
+            cert=self.cert,
+            chain=self.chain,
+            check_hostname=False,
+            verify_mode=CERT_NONE,
+        )
         con = Sender(engine_config)
 
         lookup = Lookup(name=self.lookup_name, historic_tag=None, con=con)
-        p_headers = Lookup.list_to_headers(['KEY', 'HEX', 'COLOR'], 'KEY')
-        lookup.send_control('START', p_headers, 'INC')
+        p_headers = Lookup.list_to_headers(["KEY", "HEX", "COLOR"], "KEY")
+        lookup.send_control("START", p_headers, "INC")
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
-        lookup.send_data_line(key="11", fields=["11", "HEX12", "COLOR12"],
-                              delete=True)
+            raise Exception("Not msg sent!")
+        lookup.send_data_line(
+            key="11", fields=["11", "HEX12", "COLOR12"], delete=True
+        )
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
-        lookup.send_control('END', p_headers, 'INC')
+            raise Exception("Not msg sent!")
+        lookup.send_control("END", p_headers, "INC")
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
+            raise Exception("Not msg sent!")
 
         con.socket.shutdown(0)
 
     def test_ssl_lookup_simplify(self):
-        engine_config = SenderConfigSSL(address=(self.server, self.port),
-                                        key=self.key, cert=self.cert,
-                                        chain=self.chain,
-                                        check_hostname=False,
-                                        verify_mode=CERT_NONE)
+        engine_config = SenderConfigSSL(
+            address=(self.server, self.port),
+            key=self.key,
+            cert=self.cert,
+            chain=self.chain,
+            check_hostname=False,
+            verify_mode=CERT_NONE,
+        )
         con = Sender(engine_config)
 
         lookup = Lookup(name=self.lookup_name, historic_tag=None, con=con)
-        lookup.send_headers(headers=['KEY', 'HEX', 'COLOR'], key='KEY',
-                            action='START')
+        lookup.send_headers(
+            headers=["KEY", "HEX", "COLOR"], key="KEY", action="START"
+        )
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
+            raise Exception("Not msg sent!")
         lookup.send_data_line(key="11", fields=["11", "HEX12", "COLOR12"])
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
-        lookup.send_headers(headers=['KEY', 'HEX', 'COLOR'], key='KEY',
-                            action='END')
+            raise Exception("Not msg sent!")
+        lookup.send_headers(
+            headers=["KEY", "HEX", "COLOR"], key="KEY", action="END"
+        )
         if len(con.socket.recv(1000)) == 0:
-            raise Exception('Not msg sent!')
+            raise Exception("Not msg sent!")
 
         con.socket.shutdown(0)
 
     def test_check_is_number(self):
-        self.assertTrue(Lookup.is_number('5'))
-        self.assertTrue(Lookup.is_number('5.0'))
+        self.assertTrue(Lookup.is_number("5"))
+        self.assertTrue(Lookup.is_number("5.0"))
 
     def test_check_is_not_a_number(self):
-        self.assertFalse(Lookup.is_number('5551,HNBId=001D4C-1213120051,'
-                                            'Fsn=1213120051,bSRName=,'
-                                            'manualPscUsed=false'))
-        self.assertFalse(Lookup.is_number('5.'))
-        self.assertFalse(Lookup.is_number('5,0'))
+        self.assertFalse(
+            Lookup.is_number(
+                "5551,HNBId=001D4C-1213120051,"
+                "Fsn=1213120051,bSRName=,"
+                "manualPscUsed=false"
+            )
+        )
+        self.assertFalse(Lookup.is_number("5."))
+        self.assertFalse(Lookup.is_number("5,0"))
+
+    def test_process_fields_does_not_modify_arguments(self):
+        fields = ["a", "b", "c"]
+
+        processed_fields = Lookup.process_fields(fields, key_index=1)
+
+        self.assertEqual(fields, ["a", "b", "c"])
+        self.assertEqual(processed_fields, '"b","a","c"')
 
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     unittest.main()