[Asterisk-code-review] sorcery: Refactor create, update and delete to better deal ... (asterisk[13])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: sorcery:  Refactor create, update and delete to better deal with caches
......................................................................


sorcery:  Refactor create, update and delete to better deal with caches

The ast_sorcery_create, update and delete function have been refactored
to better deal with caches and errors.

The action is now called on all non-caching wizards first. If ANY succeed,
the action is called on all caching wizards and the observers are notified.
This way we don't put something in the cache (or update or delete) before
knowing the action was performed in at least 1 backend and we only call the
observers once even if there were multiple writable backends.

ast_sorcery_create was never adding to caches in the first place which
was preventing contacts from getting added to a memory_cache when they
were created.  In turn this was causing memory_cache to emit errors if
the contact was deleted before being retrieved (which would have
populated the cache).

ASTERISK-25811 #close
Reported-by: Ross Beer

Change-Id: Id5596ce691685a79886e57b0865888458d6e7b46
---
M main/sorcery.c
1 file changed, 76 insertions(+), 23 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/main/sorcery.c b/main/sorcery.c
index 774d001..3a29cfa 100644
--- a/main/sorcery.c
+++ b/main/sorcery.c
@@ -1953,7 +1953,12 @@
 		ast_debug(5, "Sorcery wizard '%s' does not support creation\n", object_wizard->wizard->callbacks.name);
 		return 0;
 	}
-	return (!object_wizard->caching && !object_wizard->wizard->callbacks.create(details->sorcery, object_wizard->data, details->obj)) ? CMP_MATCH | CMP_STOP : 0;
+
+	if (object_wizard->wizard->callbacks.create(details->sorcery, object_wizard->data, details->obj)) {
+		return 0;
+	}
+
+	return CMP_MATCH;
 }
 
 /*! \brief Internal callback function which notifies an individual observer that an object has been created */
@@ -1998,17 +2003,31 @@
 	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
 	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
 		found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
-		if (sorcery_wizard_create(found_wizard, &sdetails, 0) == (CMP_MATCH | CMP_STOP)) {
+		if (!found_wizard->caching && sorcery_wizard_create(found_wizard, &sdetails, 0) == CMP_MATCH) {
 			object_wizard = found_wizard;
-			if(ao2_container_count(object_type->observers)) {
-				struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
+		}
+	}
 
-				if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_create, invocation)) {
-					ao2_cleanup(invocation);
-				}
+	if (object_wizard) {
+		for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+			found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
+			if (found_wizard->caching) {
+				sorcery_wizard_create(found_wizard, &sdetails, 0);
+			}
+		}
+
+		if (ao2_container_count(object_type->observers)) {
+			struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(
+				object_type, object);
+
+			if (invocation
+				&& ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_create,
+					invocation)) {
+				ao2_cleanup(invocation);
 			}
 		}
 	}
+
 	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return object_wizard ? 0 : -1;
@@ -2048,8 +2067,11 @@
 		return 0;
 	}
 
-	return (!object_wizard->wizard->callbacks.update(details->sorcery, object_wizard->data, details->obj) &&
-		!object_wizard->caching) ? CMP_MATCH | CMP_STOP : 0;
+	if (object_wizard->wizard->callbacks.update(details->sorcery, object_wizard->data, details->obj)) {
+		return 0;
+	}
+
+	return CMP_MATCH;
 }
 
 int ast_sorcery_update(const struct ast_sorcery *sorcery, void *object)
@@ -2071,17 +2093,31 @@
 	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
 	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
 		found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
-		if (sorcery_wizard_update(found_wizard, &sdetails, 0) == (CMP_MATCH | CMP_STOP)) {
+		if (!found_wizard->caching && sorcery_wizard_update(found_wizard, &sdetails, 0) == CMP_MATCH) {
 			object_wizard = found_wizard;
-			if (ao2_container_count(object_type->observers)) {
-				struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
+		}
+	}
 
-				if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_update, invocation)) {
-					ao2_cleanup(invocation);
-				}
+	if (object_wizard) {
+		for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+			found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
+			if (found_wizard->caching) {
+				sorcery_wizard_update(found_wizard, &sdetails, 0);
+			}
+		}
+
+		if (ao2_container_count(object_type->observers)) {
+			struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(
+				object_type, object);
+
+			if (invocation
+				&& ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_update,
+					invocation)) {
+				ao2_cleanup(invocation);
 			}
 		}
 	}
+
 	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return object_wizard ? 0 : -1;
@@ -2121,8 +2157,11 @@
 		return 0;
 	}
 
-	return (!object_wizard->wizard->callbacks.delete(details->sorcery, object_wizard->data, details->obj) &&
-		!object_wizard->caching) ? CMP_MATCH | CMP_STOP : 0;
+	if (object_wizard->wizard->callbacks.delete(details->sorcery, object_wizard->data, details->obj)) {
+		return 0;
+	}
+
+	return CMP_MATCH;
 }
 
 int ast_sorcery_delete(const struct ast_sorcery *sorcery, void *object)
@@ -2144,17 +2183,31 @@
 	AST_VECTOR_RW_RDLOCK(&object_type->wizards);
 	for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
 		found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
-		if (sorcery_wizard_delete(found_wizard, &sdetails, 0) == (CMP_MATCH | CMP_STOP)) {
+		if (!found_wizard->caching && sorcery_wizard_delete(found_wizard, &sdetails, 0) == CMP_MATCH) {
 			object_wizard = found_wizard;
-			if (ao2_container_count(object_type->observers)) {
-				struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(object_type, object);
+		}
+	}
 
-				if (invocation && ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_delete, invocation)) {
-					ao2_cleanup(invocation);
-				}
+	if (object_wizard) {
+		for (i = 0; i < AST_VECTOR_SIZE(&object_type->wizards); i++) {
+			found_wizard = AST_VECTOR_GET(&object_type->wizards, i);
+			if (found_wizard->caching) {
+				sorcery_wizard_delete(found_wizard, &sdetails, 0);
+			}
+		}
+
+		if (ao2_container_count(object_type->observers)) {
+			struct sorcery_observer_invocation *invocation = sorcery_observer_invocation_alloc(
+				object_type, object);
+
+			if (invocation
+				&& ast_taskprocessor_push(object_type->serializer, sorcery_observers_notify_delete,
+					invocation)) {
+				ao2_cleanup(invocation);
 			}
 		}
 	}
+
 	AST_VECTOR_RW_UNLOCK(&object_type->wizards);
 
 	return object_wizard ? 0 : -1;

-- 
To view, visit https://gerrit.asterisk.org/2312
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id5596ce691685a79886e57b0865888458d6e7b46
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>