Merge pull request #228 from bigspider/173-hardcoded-strings

Avoid hardcoded strings

Author: sr-gi

common/appointment.py

@@ -1,4 +1,12 @@
-import struct
+from enum import Enum
+
+
+class AppointmentStatus(str, Enum):
+    """Represents all the possible states of an appointment in the system, or in a response to a client request."""
+
+    NOT_FOUND = "not_found"
+    BEING_WATCHED = "being_watched"
+    DISPUTE_RESPONDED = "dispute_responded"
 
 
 class Appointment:

contrib/client/test/test_teos_client.py

@@ -9,7 +9,7 @@
 
 import common.receipts as receipts
 from common.tools import compute_locator, is_compressed_pk
-from common.appointment import Appointment
+from common.appointment import Appointment, AppointmentStatus
 from common.cryptographer import Cryptographer
 from common.exceptions import InvalidParameter, InvalidKey, TowerResponseError
 
@@ -247,7 +247,7 @@ def test_get_appointment():
     # Response of get_appointment endpoint is an appointment with status added to it.
     response = {
         "locator": dummy_appointment_dict.get("locator"),
-        "status": "being_watched",
+        "status": AppointmentStatus.BEING_WATCHED,
         "appointment": dummy_appointment_dict,
     }
 

teos/api.py

@@ -3,15 +3,17 @@
 from waitress import serve as wsgi_serve
 from flask import Flask, request, jsonify
 
+
 import common.errors as errors
+from common.appointment import AppointmentStatus
+from common.exceptions import InvalidParameter
+from common.constants import HTTP_OK, HTTP_BAD_REQUEST, HTTP_SERVICE_UNAVAILABLE, HTTP_NOT_FOUND
 from teos.inspector import Inspector, InspectionFailed
 from teos.protobuf.user_pb2 import RegisterRequest
 from teos.protobuf.tower_services_pb2_grpc import TowerServicesStub
 from teos.protobuf.appointment_pb2 import Appointment, AddAppointmentRequest, GetAppointmentRequest
 
-from common.exceptions import InvalidParameter
 from teos.logger import setup_logging, get_logger
-from common.constants import HTTP_OK, HTTP_BAD_REQUEST, HTTP_SERVICE_UNAVAILABLE, HTTP_NOT_FOUND
 
 
 # NOTCOVERED: not sure how to monkey patch this one. May be related to #77
@@ -257,9 +259,11 @@ def get_appointment(self):
             A ``status`` flag is added to the data provided by either the :obj:`Watcher <teos.watcher.Watcher>` or the
             :obj:`Responder <teos.responder.Responder>` that signals the status of the appointment.
 
-            - Appointments hold by the :obj:`Watcher <teos.watcher.Watcher>` are flagged as ``being_watched``.
-            - Appointments hold by the :obj:`Responder <teos.responder.Responder>` are flagged as ``dispute_triggered``.
-            - Unknown appointments are flagged as ``not_found``.
+            - Appointments held by the :obj:`Watcher <teos.watcher.Watcher>` are flagged as
+              ``AppointmentStatus.BEING_WATCHED``.
+            - Appointments held by the :obj:`Responder <teos.responder.Responder>` are flagged as
+              ``AppointmentStatus.DISPUTE_RESPONDED``.
+            - Unknown appointments are flagged as ``AppointmentStatus.NOT_FOUND``.
         """
 
         # Getting the real IP if the server is behind a reverse proxy
@@ -299,6 +303,6 @@ def get_appointment(self):
 
         except (InspectionFailed, grpc.RpcError):
             rcode = HTTP_NOT_FOUND
-            response = {"locator": locator, "status": "not_found"}
+            response = {"locator": locator, "status": AppointmentStatus.NOT_FOUND}
 
         return jsonify(response), rcode

teos/chain_monitor.py

@@ -32,7 +32,8 @@ class ChainMonitor:
     detected.
     Finally, once the ``terminate`` method is called, the ``status`` is changed to ``ChainMonitorStatus.TERMINATED``,
     the chain monitor stops monitoring the chain and no receiving queue will be notified about new blocks (including
-    any block that is currently in the internal queue). A final ``"END"`` message is sent to all the subscribers.
+    any block that is currently in the internal queue). A final ``ChainMonitor.END_MESSAGE`` is sent to all the
+    subscribers.
 
     Args:
         receiving_queues (:obj:`list`): a list of :obj:`Queue` objects that will be notified when the chain_monitor is
@@ -53,6 +54,8 @@ class ChainMonitor:
             ``ChainMonitorStatus.LISTENING``, ``ChainMonitorStatus.ACTIVE`` or ``ChainMonitorStatus.TERMINATED``.
     """
 
+    END_MESSAGE = "END"
+
     def __init__(self, receiving_queues, block_processor, bitcoind_feed_params):
         self.logger = get_logger(component=ChainMonitor.__name__)
         self.last_tips = []
@@ -151,7 +154,7 @@ def notify_subscribers(self):
 
         while self.status != ChainMonitorStatus.TERMINATED:
             message = self.queue.get()
-            # A special "END" message is added to the queue after the status is set to TERMINATED
+            # A special ChainMonitor.END_MESSAGE is added to the queue after the status is set to TERMINATED
             # In all the other cases, message is a block_hash
             with self.lock:
                 for rec_queue in self.receiving_queues:
@@ -164,7 +167,7 @@ def monitor_chain(self):
         and creates two threads, one per each monitoring approach (``zmq`` and ``polling``).
 
         Raises:
-            :obj:`RuntimeError`: if the ``status`` was not ``ChainMonitor.IDLE`` when the method was called.
+            :obj:`RuntimeError`: if the ``status`` was not ``ChainMonitorStatus.IDLE`` when the method was called.
         """
 
         if self.status != ChainMonitorStatus.IDLE:
@@ -183,7 +186,7 @@ def activate(self):
         is added to the internal queue.
 
         Raises:
-            :obj:`RuntimeError`: if the ``status`` was not ``ChainMonitor.LISTENING`` when the method was called.
+            :obj:`RuntimeError`: if the ``status`` was not ``ChainMonitorStatus.LISTENING`` when the method was called.
         """
 
         if self.status != ChainMonitorStatus.LISTENING:
@@ -195,9 +198,9 @@ def activate(self):
 
     def terminate(self):
         """
-        Changes the ``status`` of the :obj:`ChainMonitor` to terminated and sends the "END" message to the internal
-        queue. All the threads will stop as soon as possible.
+        Changes the ``status`` of the :obj:`ChainMonitor` to terminated and sends the ``ChainMonitor.END_MESSAGE``
+        message to the internal queue. All the threads will stop as soon as possible.
         """
 
         self.status = ChainMonitorStatus.TERMINATED
-        self.queue.put("END")
+        self.queue.put(ChainMonitor.END_MESSAGE)

teos/internal_api.py

@@ -4,7 +4,7 @@
 from google.protobuf.struct_pb2 import Struct
 
 from teos.logger import get_logger
-from common.appointment import Appointment
+from common.appointment import Appointment, AppointmentStatus
 from common.exceptions import InvalidParameter
 
 from teos.protobuf.appointment_pb2 import (
@@ -122,7 +122,7 @@ def get_appointment(self, request, context):
         with self.rw_lock.gen_rlock():
             try:
                 data, status = self.watcher.get_appointment(request.locator, request.signature)
-                if status == "being_watched":
+                if status == AppointmentStatus.BEING_WATCHED:
                     data = AppointmentData(
                         appointment=AppointmentProto(
                             locator=data.get("locator"),

teos/responder.py

@@ -2,7 +2,7 @@
 from threading import Thread
 
 from teos.cleaner import Cleaner
-
+from teos.chain_monitor import ChainMonitor
 from teos.logger import get_logger
 from common.constants import IRREVOCABLY_RESOLVED
 
@@ -148,7 +148,7 @@ def __init__(self, db_manager, gatekeeper, carrier, block_processor):
     def awake(self):
         """
             Starts a new thread to monitor the blockchain to make sure triggered appointments get enough depth.
-            The thread will run until the :obj:`ChainMonitor` adds the ``"END"`` message to the queue.
+            The thread will run until the :obj:`ChainMonitor` adds the ``ChainMonitor.END_MESSAGE`` to the queue.
 
             Returns:
                 :obj:`Thread <multithreading.Thread>`: The thread object that was just created and is already running.
@@ -273,8 +273,8 @@ def do_watch(self):
         while True:
             block_hash = self.block_queue.get()
 
-            # When the ChainMonitor is stopped, a final "END" message is sent
-            if block_hash == "END":
+            # When the ChainMonitor is stopped, a final ChainMonitor.END_MESSAGE is sent
+            if block_hash == ChainMonitor.END_MESSAGE:
                 break
 
             block = self.block_processor.get_block(block_hash)

teos/watcher.py

@@ -5,13 +5,13 @@
 
 from teos.logger import get_logger
 import common.receipts as receipts
+from common.appointment import AppointmentStatus
 from common.tools import compute_locator
-from common.exceptions import BasicException
-from common.exceptions import EncryptionError
+from common.exceptions import BasicException, EncryptionError, InvalidParameter, SignatureError
 from common.cryptographer import Cryptographer, hash_160
-from common.exceptions import InvalidParameter, SignatureError
 
 from teos.cleaner import Cleaner
+from teos.chain_monitor import ChainMonitor
 from teos.extended_appointment import ExtendedAppointment
 from teos.block_processor import InvalidTransactionFormat
 
@@ -251,7 +251,7 @@ def n_responder_trackers(self):
     def awake(self):
         """
             Starts a new thread to monitor the blockchain for channel breaches. The thread will run until the
-            :obj:`ChainMonitor` adds the ``"END"`` message to the queue.
+            :obj:`ChainMonitor` adds ``ChainMonitor.END_MESSAGE`` to the queue.
 
             Returns:
                 :obj:`Thread <multithreading.Thread>`: The thread object that was just created and is already running.
@@ -289,8 +289,8 @@ def get_appointment(self, locator, user_signature):
             user_signature (:obj:`str`): the signature of the request by the user.
 
         Returns:
-            :obj:`tuple`: A tuple containing the appointment data and the status (either ``"being_watched"`` or
-            ``"dispute_responded"``).
+            :obj:`tuple`: A tuple containing the appointment data and the status, either
+                ``AppointmentStatus.BEING_WATCHED`` or ``AppointmentStatus.DISPUTE_RESPONDED``
 
         Raises:
             :obj:`AppointmentNotFound`: if the appointment is not found in the tower.
@@ -302,10 +302,10 @@ def get_appointment(self, locator, user_signature):
 
         if uuid in self.appointments:
             appointment_data = self.db_manager.load_watcher_appointment(uuid)
-            status = "being_watched"
+            status = AppointmentStatus.BEING_WATCHED
         elif uuid in self.responder.trackers:
             appointment_data = self.db_manager.load_responder_tracker(uuid)
-            status = "dispute_responded"
+            status = AppointmentStatus.DISPUTE_RESPONDED
         else:
             raise AppointmentNotFound("Cannot find {}".format(locator))
 
@@ -452,8 +452,8 @@ def do_watch(self):
         while True:
             block_hash = self.block_queue.get()
 
-            # When the ChainMonitor is stopped, a final "END" message is sent
-            if block_hash == "END":
+            # When the ChainMonitor is stopped, a final ChainMonitor.END_MESSAGE message is sent
+            if block_hash == ChainMonitor.END_MESSAGE:
                 break
 
             block = self.block_processor.get_block(block_hash)
@@ -661,7 +661,7 @@ def get_registered_user_ids(self):
 
     def get_user_info(self, user_id):
         """
-        Returns the data hold by the tower about the user given an ``user_id``.
+        Returns the data held by the tower about the user given an ``user_id``.
 
         Args:
             user_id (:obj:`str`): the id of the requested user.

test/common/unit/test_appointment.py

@@ -1,4 +1,3 @@
-import struct
 import pytest
 
 from common.appointment import Appointment

test/teos/e2e/test_basic_e2e.py

@@ -9,7 +9,7 @@
 import common.receipts as receipts
 from common.exceptions import TowerResponseError
 from common.tools import compute_locator
-from common.appointment import Appointment
+from common.appointment import Appointment, AppointmentStatus
 from common.cryptographer import Cryptographer
 
 from teos.cli.teos_cli import RPCClient
@@ -106,7 +106,7 @@ def test_appointment_life_cycle(run_bitcoind):
 
     # Get the information from the tower to check that it matches
     appointment_info = get_appointment_info(locator)
-    assert appointment_info.get("status") == "being_watched"
+    assert appointment_info.get("status") == AppointmentStatus.BEING_WATCHED
     assert appointment_info.get("locator") == locator
     assert appointment_info.get("appointment") == appointment.to_dict()
 
@@ -121,7 +121,7 @@ def test_appointment_life_cycle(run_bitcoind):
     # Trigger a breach and check again
     generate_block_with_transactions(commitment_tx)
     appointment_info = get_appointment_info(locator)
-    assert appointment_info.get("status") == "dispute_responded"
+    assert appointment_info.get("status") == AppointmentStatus.DISPUTE_RESPONDED
     assert appointment_info.get("locator") == locator
     appointments_in_watcher -= 1
     appointments_in_responder += 1
@@ -230,7 +230,7 @@ def test_appointment_malformed_penalty(run_bitcoind):
 
     # Get the information from the tower to check that it matches
     appointment_info = get_appointment_info(locator)
-    assert appointment_info.get("status") == "being_watched"
+    assert appointment_info.get("status") == AppointmentStatus.BEING_WATCHED
     assert appointment_info.get("locator") == locator
     assert appointment_info.get("appointment") == appointment.to_dict()
 
@@ -298,7 +298,7 @@ def test_two_identical_appointments(run_bitcoind):
     # The last appointment should have made it to the Responder
     appointment_info = get_appointment_info(locator)
 
-    assert appointment_info.get("status") == "dispute_responded"
+    assert appointment_info.get("status") == AppointmentStatus.DISPUTE_RESPONDED
     assert appointment_info.get("appointment").get("penalty_rawtx") == penalty_tx
 
 
@@ -325,9 +325,9 @@ def test_two_identical_appointments(run_bitcoind):
 #
 #     # Check that we can get it from both users
 #     appointment_info = get_appointment_info(locator)
-#     assert appointment_info.get("status") == "being_watched"
+#     assert appointment_info.get("status") == AppointmentStatus.BEING_WATCHED
 #     appointment_info = get_appointment_info(locator, sk=tmp_user_sk)
-#     assert appointment_info.get("status") == "being_watched"
+#     assert appointment_info.get("status") == AppointmentStatus.BEING_WATCHED
 #
 #     # Broadcast the commitment transaction and mine a block
 #     generate_block_with_transactions(commitment_tx)
@@ -344,7 +344,7 @@ def test_two_identical_appointments(run_bitcoind):
 #
 #     appointment_info = appointment_info if appointment_info is None else appointment_dup_info
 #
-#     assert appointment_info.get("status") == "dispute_responded"
+#     assert appointment_info.get("status") == AppointmentStatus.DISPUTE_RESPONDED
 #     assert appointment_info.get("appointment").get("penalty_rawtx") == penalty_tx
 
 
@@ -385,7 +385,7 @@ def test_two_appointment_same_locator_different_penalty_different_users(run_bitc
         appointment_info = appointment2_info
         appointment1_data = appointment2_data
 
-    assert appointment_info.get("status") == "dispute_responded"
+    assert appointment_info.get("status") == AppointmentStatus.DISPUTE_RESPONDED
     assert appointment_info.get("locator") == appointment1_data.get("locator")
     assert appointment_info.get("appointment").get("penalty_tx") == appointment1_data.get("penalty_tx")
 
@@ -402,7 +402,7 @@ def test_add_appointment_trigger_on_cache(run_bitcoind):
     # Send the data to the tower and request it back. It should have gone straightaway to the Responder
     appointment = teos_client.create_appointment(appointment_data)
     add_appointment(appointment)
-    assert get_appointment_info(locator).get("status") == "dispute_responded"
+    assert get_appointment_info(locator).get("status") == AppointmentStatus.DISPUTE_RESPONDED
 
 
 def test_add_appointment_invalid_trigger_on_cache(run_bitcoind):
@@ -483,15 +483,15 @@ def test_appointment_shutdown_teos_trigger_back_online(run_bitcoind):
     # Check that the appointment is still in the Watcher
     appointment_info = get_appointment_info(locator)
 
-    assert appointment_info.get("status") == "being_watched"
+    assert appointment_info.get("status") == AppointmentStatus.BEING_WATCHED
     assert appointment_info.get("appointment") == appointment.to_dict()
 
     # Trigger appointment after restart
     generate_block_with_transactions(commitment_tx)
 
     # The appointment should have been moved to the Responder
     appointment_info = get_appointment_info(locator)
-    assert appointment_info.get("status") == "dispute_responded"
+    assert appointment_info.get("status") == AppointmentStatus.DISPUTE_RESPONDED
 
 
 def test_appointment_shutdown_teos_trigger_while_offline(run_bitcoind):
@@ -509,7 +509,7 @@ def test_appointment_shutdown_teos_trigger_while_offline(run_bitcoind):
 
     # Check that the appointment is still in the Watcher
     appointment_info = get_appointment_info(locator)
-    assert appointment_info.get("status") == "being_watched"
+    assert appointment_info.get("status") == AppointmentStatus.BEING_WATCHED
     assert appointment_info.get("appointment") == appointment.to_dict()
 
     # Shutdown and trigger
@@ -525,4 +525,4 @@ def test_appointment_shutdown_teos_trigger_while_offline(run_bitcoind):
 
     # The appointment should have been moved to the Responder
     appointment_info = get_appointment_info(locator)
-    assert appointment_info.get("status") == "dispute_responded"
+    assert appointment_info.get("status") == AppointmentStatus.DISPUTE_RESPONDED