From 8147fa45ddd793507afd65e1c562a325ce27d28c Mon Sep 17 00:00:00 2001 From: Rucha Mahabal Date: Mon, 27 Jan 2020 14:07:13 +0530 Subject: [PATCH] fix: anti-pattern code --- frappe/api.py | 4 +--- .../doctype/event_consumer/event_consumer.py | 6 ++++-- .../doctype/event_producer/event_producer.py | 18 +++++++++--------- .../doctype/event_sync_log/event_sync_log.py | 1 + .../event_update_log/event_update_log.py | 10 ++++++++-- frappe/model/document.py | 2 +- 6 files changed, 24 insertions(+), 17 deletions(-) diff --git a/frappe/api.py b/frappe/api.py index d0d1141782..3f2e4f9c70 100644 --- a/frappe/api.py +++ b/frappe/api.py @@ -184,9 +184,7 @@ def validate_auth_via_api_keys(): def validate_api_key_secret(api_key, api_secret, frappe_authorization_source=None): - """ - frappe_authorization_source to provide api key and secret for a doctype apart from User - """ + """frappe_authorization_source to provide api key and secret for a doctype apart from User""" doctype = frappe_authorization_source or 'User' doc = frappe.db.get_value( doctype=doctype, diff --git a/frappe/event_streaming/doctype/event_consumer/event_consumer.py b/frappe/event_streaming/doctype/event_consumer/event_consumer.py index 8848c53cc5..2610d9d425 100644 --- a/frappe/event_streaming/doctype/event_consumer/event_consumer.py +++ b/frappe/event_streaming/doctype/event_consumer/event_consumer.py @@ -39,7 +39,8 @@ class EventConsumer(Document): entry['status'] = frappe.db.get_value('Event Consumer Document Type', {'parent': self.name, 'ref_doctype': ref_doctype}, 'status') event_producer.producer_doctypes = config - # when producer doc is updated it updates the consumer doc, set flag to avoid deadlock + # when producer doc is updated it updates the consumer doc + # set flag to avoid deadlock event_producer.incoming_change = True consumer_site.update(event_producer) @@ -77,7 +78,8 @@ def register_consumer(data): consumer.insert(ignore_permissions=True) frappe.db.commit() - # consumer's 'last_update' field should point to the latest update in producer's update log when subscribing + # consumer's 'last_update' field should point to the latest update + # in producer's update log when subscribing # so that, updates after subscribing are consumed and not the old ones. last_update = str(get_last_update()) return json.dumps({'api_key': api_key, 'api_secret': api_secret, 'last_update': last_update}) diff --git a/frappe/event_streaming/doctype/event_producer/event_producer.py b/frappe/event_streaming/doctype/event_producer/event_producer.py index 4b95e32c07..18b7d9d668 100644 --- a/frappe/event_streaming/doctype/event_producer/event_producer.py +++ b/frappe/event_streaming/doctype/event_producer/event_producer.py @@ -66,7 +66,7 @@ class EventProducer(Document): consumer_doctypes = [] for entry in self.producer_doctypes: if entry.has_mapping: - # if it has mapping then on event consumer's site it should subscribe to remote doctype + # if mapping, subscribe to remote doctype on consumer's site consumer_doctypes.append(frappe.db.get_value('Document Type Mapping', entry.mapping, 'remote_doctype')) else: consumer_doctypes.append(entry.ref_doctype) @@ -98,7 +98,7 @@ class EventProducer(Document): event_consumer.consumer_doctypes = [] for entry in self.producer_doctypes: if entry.has_mapping: - # if it has mapping then on event consumer's site it should subscribe to remote doctype + # if mapping, subscribe to remote doctype on consumer's site ref_doctype = frappe.db.get_value('Document Type Mapping', entry.mapping, 'remote_doctype') else: ref_doctype = entry.ref_doctype @@ -138,7 +138,7 @@ def get_producer_site(producer_url): def get_approval_status(config, ref_doctype): - """check whether the doctype has been marked approved, rejected or pending for consumption""" + """check the approval status for consumption""" for entry in config: if entry.get('ref_doctype') == ref_doctype: return entry.get('status') @@ -153,8 +153,7 @@ def pull_producer_data(): for event_producer in frappe.get_all('Event Producer'): pull_from_node(event_producer.name) return 'success' - else: - return None + return None @frappe.whitelist() @@ -298,9 +297,9 @@ def set_delete(update): def get_updates(producer_site, last_update, doctypes): """Get all updates generated after the last update timestamp""" docs = producer_site.get_list( - doctype = 'Event Update Log', - filters = {'ref_doctype': ('in', doctypes), 'creation': ('>', last_update)}, - fields = ['update_type', 'ref_doctype', 'docname', 'data', 'name', 'creation'] + doctype='Event Update Log', + filters={'ref_doctype': ('in', doctypes), 'creation': ('>', last_update)}, + fields=['update_type', 'ref_doctype', 'docname', 'data', 'name', 'creation'] ) docs.reverse() return [frappe._dict(d) for d in docs] @@ -318,7 +317,8 @@ def get_local_doc(update): def sync_dependencies(document, producer_site): """ - dependencies is a dictionary to store all the docs having dependencies and their sync status, + dependencies is a dictionary to store all the docs + having dependencies and their sync status, which is shared among all nested functions. """ dependencies = {document: True} diff --git a/frappe/event_streaming/doctype/event_sync_log/event_sync_log.py b/frappe/event_streaming/doctype/event_sync_log/event_sync_log.py index 2ea7a8fbba..31b1f863aa 100644 --- a/frappe/event_streaming/doctype/event_sync_log/event_sync_log.py +++ b/frappe/event_streaming/doctype/event_sync_log/event_sync_log.py @@ -6,5 +6,6 @@ from __future__ import unicode_literals # import frappe from frappe.model.document import Document + class EventSyncLog(Document): pass diff --git a/frappe/event_streaming/doctype/event_update_log/event_update_log.py b/frappe/event_streaming/doctype/event_update_log/event_update_log.py index b282d5fe32..21e3b303e8 100644 --- a/frappe/event_streaming/doctype/event_update_log/event_update_log.py +++ b/frappe/event_streaming/doctype/event_update_log/event_update_log.py @@ -11,13 +11,16 @@ from frappe.model import no_value_fields, table_fields class EventUpdateLog(Document): pass + def notify_consumers(doc, _method=None): - """Send update notification updates to event consumers whenever update log is generated""" + """Send update notification updates to event consumers + whenever update log is generated""" enqueued_method = 'frappe.event_streaming.doctype.event_consumer.event_consumer.notify_event_consumers' jobs = get_jobs() if not jobs or enqueued_method not in jobs[frappe.local.site]: frappe.enqueue(enqueued_method, doctype=doc.ref_doctype, queue='long', enqueue_after_commit=True) + def get_update(old, new, for_child=False): """ Get document objects with updates only @@ -37,7 +40,7 @@ def get_update(old, new, for_child=False): if not new: return None - out = frappe._dict(changed = {}, added = {}, removed = {}, row_changed = {}) + out = frappe._dict(changed={}, added={}, removed={}, row_changed={}) for df in new.meta.fields: if df.fieldtype in no_value_fields and df.fieldtype not in table_fields: continue @@ -67,6 +70,7 @@ def make_maps(old_value, new_value): new_row_by_name[d.name] = d return old_row_by_name, new_row_by_name + def check_for_additions(out, df, new_value, old_row_by_name): """check rows for additions, changes""" for _i, d in enumerate(new_value): @@ -83,6 +87,7 @@ def check_for_additions(out, df, new_value, old_row_by_name): out.added[df.fieldname].append(d.as_dict()) return out + def check_for_deletions(out, df, old_value, new_row_by_name): """check for deletions""" for d in old_value: @@ -92,6 +97,7 @@ def check_for_deletions(out, df, old_value, new_row_by_name): out.removed[df.fieldname].append(d.name) return out + def check_docstatus(out, old, new, for_child): """docstatus changes""" if not for_child and old.docstatus != new.docstatus: diff --git a/frappe/model/document.py b/frappe/model/document.py index 0ffcefe816..134cce3b75 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -193,7 +193,7 @@ class Document(BaseDocument): raise frappe.PermissionError def insert(self, ignore_permissions=None, ignore_links=None, ignore_if_duplicate=False, - ignore_mandatory=None, set_name=None, set_child_names=True): + ignore_mandatory=None, set_name=None, set_child_names=True): """Insert the document in the database (as a new document). This will check for user permissions and execute `before_insert`, `validate`, `on_update`, `after_insert` methods if they are written.