Merge pull request #1587 from mozilla/fix-geomodel-magic-strings

Replace magic strings with constructor methods

alerts/geomodel/locality.py

@@ -35,6 +35,13 @@ class Locality(NamedTuple):
     longitude: float
     radius: int
 
+    def index_name():
+        '''Locality state is stored in an index called "locality" that used to
+        be stored in a magic string.
+        '''
+
+        return 'locality'
+
 
 class State(NamedTuple):
     '''Represents the state tracked for each user regarding their localities.
@@ -44,6 +51,12 @@ class State(NamedTuple):
     username: str
     localities: List[Locality]
 
+    def new(username: str, localities: List[Locality]) -> 'State':
+        '''Construct a new State with all fields populated.
+        '''
+
+        return State(Locality.index_name(), username, localities)
+
 
 class Entry(NamedTuple):
     '''A top-level container for locality state that will be inserted into
@@ -89,10 +102,12 @@ def wrap_journal(client: ESClient) -> JournalInterface:
     def wrapper(entry: Entry, esindex: str):
         document = dict(entry.state._asdict())
 
+        _id = '' if entry.identifier is None else entry.identifier
+
         client.save_object(
             index=esindex,
             body=document,
-            doc_id=entry.identifier)
+            doc_id=_id)
 
     return wrapper
 
@@ -120,7 +135,7 @@ def wrapper(query: SearchQuery, esindex: str) -> Optional[Entry]:
             ]
 
             eid = results[0]['_id']
-            state = State(**_dict_take(state_dict, State._fields))
+            state = State.new(state_dict['username'], state_dict['localities'])
 
             return Entry(eid, state)
         except TypeError:
@@ -170,7 +185,7 @@ def find(qes: QueryInterface, username: str, index: str) -> Optional[Entry]:
 
     search = SearchQuery()
     search.add_must([
-        TermMatch('type_', 'locality'),
+        TermMatch('type_', Locality.index_name()),
         TermMatch('username', username)
     ])
 
@@ -230,7 +245,7 @@ def remove_outdated(state: State, days_valid: int) -> Update:
     ]
 
     return Update(
-        state=State(state.type_, state.username, new_localities),
+        state=State.new(state.username, new_localities),
         did_update=len(new_localities) != len(state.localities))
 
 

alerts/geomodel_location.py

@@ -121,11 +121,10 @@ def onAggregation(self, agg):
 
         entry_from_es = locality.find(query, username, cfg.localities.es_index)
 
-        new_state = locality.State('locality', username, locs_from_evts)
+        new_state = locality.State.new(username, locs_from_evts)
 
         if entry_from_es is None:
-            entry_from_es = locality.Entry(
-                '', locality.State('locality', username, []))
+            entry_from_es = locality.Entry.new(locality.State.new(username, []))
 
         cleaned = locality.remove_outdated(
             entry_from_es.state, cfg.localities.valid_duration_days)
@@ -139,7 +138,9 @@ def onAggregation(self, agg):
         updated = locality.update(cleaned.state, new_state)
 
         if updated.did_update:
-            entry_from_es = locality.Entry(entry_from_es.identifier, updated.state)
+            entry_from_es = locality.Entry(
+                entry_from_es.identifier,
+                updated.state)
 
             journal(entry_from_es, cfg.localities.es_index)