LP#1468422 Auth efficiency improvements

1. Adds an oils_utils function for retrieving the ID of the root org
unit.

2. Avoid multiple cstore/db lookups for the root org unit by caching the
ID at the process level.

3. Move permission checks from open-ils.storage to open-ils.cstore.

Signed-off-by: Bill Erickson <berickxx@gmail.com>
Signed-off-by: Dan Wells <dbw2@calvin.edu>

diff --git a/Open-ILS/include/openils/oils_utils.h b/Open-ILS/include/openils/oils_utils.h
index 3554d21..6b391f2 100644 (file)
--- a/Open-ILS/include/openils/oils_utils.h
+++ b/Open-ILS/include/openils/oils_utils.h
@@ -110,6 +110,11 @@ long oilsUtilsIntervalToSeconds( const char* interval );
  */
 int oilsUtilsTrackUserActivity( long usr, const char* ewho, const char* ewhat, const char* ehow );
 
+/**
+ * Returns the ID of the root org unit (parent_ou = NULL)
+ */
+int oilsUtilsGetRootOrgId();
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/Open-ILS/src/c-apps/oils_auth.c b/Open-ILS/src/c-apps/oils_auth.c
index 22cf96e..fd9b5e7 100644 (file)
--- a/Open-ILS/src/c-apps/oils_auth.c
+++ b/Open-ILS/src/c-apps/oils_auth.c
@@ -319,18 +319,10 @@ int oilsAuthInitBarcode(osrfMethodContext* ctx) {
 // returns true if the provided identifier matches the barcode regex.
 static int oilsAuthIdentIsBarcode(const char* identifier) {
 
-    // before we can fetch the barcode regex unit setting,
-    // first determine what the root org unit ID is.
+    // Assumes barcode regex is a global setting.
     // TODO: add an org_unit param to the .init API for future use?
-    
-    jsonObject *params = jsonParse("{\"parent_ou\":null}");
-    jsonObject *org_unit_id = oilsUtilsCStoreReq(
-        "open-ils.cstore.direct.actor.org_unit.id_list", params);
-    jsonObjectFree(params);
-
     char* bc_regex = oilsUtilsFetchOrgSetting(
-        (int) jsonObjectGetNumber(org_unit_id), "opac.barcode_regex");
-    jsonObjectFree(org_unit_id);
+        oilsUtilsGetRootOrgId(), "opac.barcode_regex");
 
     if (!bc_regex) {
         // if no regex is set, assume any identifier starting

diff --git a/Open-ILS/src/c-apps/oils_utils.c b/Open-ILS/src/c-apps/oils_utils.c
index cb334ac..155274f 100644 (file)
--- a/Open-ILS/src/c-apps/oils_utils.c
+++ b/Open-ILS/src/c-apps/oils_utils.c
@@ -164,49 +164,65 @@ int oilsUtilsTrackUserActivity(long usr, const char* ewho, const char* ewhat, co
 }
 
 
+static int rootOrgId = 0; // cache the ID of the root org unit.
+int oilsUtilsGetRootOrgId() {
+
+    // return the cached value if we have it.
+    if (rootOrgId > 0) return rootOrgId;
+
+    jsonObject* where_clause = jsonParse("{\"parent_ou\":null}");
+    jsonObject* org = oilsUtilsQuickReq(
+        "open-ils.cstore",
+        "open-ils.cstore.direct.actor.org_unit.search",
+        where_clause
+    );
+
+    rootOrgId = (int) 
+        jsonObjectGetNumber(oilsFMGetObject(org, "id"));
+
+    jsonObjectFree(where_clause);
+    jsonObjectFree(org);
+
+    return rootOrgId;
+}
 
 oilsEvent* oilsUtilsCheckPerms( int userid, int orgid, char* permissions[], int size ) {
-       if (!permissions) return NULL;
-       int i;
-       oilsEvent* evt = NULL;
-
-       // Find the root org unit, i.e. the one with no parent.
-       // Assumption: there is only one org unit with no parent.
-       if (orgid == -1) {
-               jsonObject* where_clause = jsonParse( "{\"parent_ou\":null}" );
-               jsonObject* org = oilsUtilsQuickReq(
-                       "open-ils.cstore",
-                       "open-ils.cstore.direct.actor.org_unit.search",
-                       where_clause
-               );
-               jsonObjectFree( where_clause );
-
-               orgid = (int)jsonObjectGetNumber( oilsFMGetObject( org, "id" ) );
-
-               jsonObjectFree(org);
-       }
+    if (!permissions) return NULL;
+    int i;
 
-       for( i = 0; i < size && permissions[i]; i++ ) {
+    // Check perms against the root org unit if no org unit is provided.
+    if (orgid == -1)
+        orgid = oilsUtilsGetRootOrgId();
 
-               char* perm = permissions[i];
-               jsonObject* params = jsonParseFmt("[%d, \"%s\", %d]", userid, perm, orgid);
-               jsonObject* o = oilsUtilsQuickReq( "open-ils.storage",
-                       "open-ils.storage.permission.user_has_perm", params );
+    for( i = 0; i < size && permissions[i]; i++ ) {
+        oilsEvent* evt = NULL;
+        char* perm = permissions[i];
 
-               char* r = jsonObjectToSimpleString(o);
+        jsonObject* params = jsonParseFmt(
+            "{\"from\":[\"permission.usr_has_perm\",\"%d\",\"%s\",\"%d\"]}",
+            userid, perm, orgid
+        );
 
-               if(r && !strcmp(r, "0"))
-                       evt = oilsNewEvent3( OSRF_LOG_MARK, OILS_EVENT_PERM_FAILURE, perm, orgid );
+        // Execute the query
+        jsonObject* result = oilsUtilsCStoreReq(
+            "open-ils.cstore.json_query", params);
 
-               jsonObjectFree(params);
-               jsonObjectFree(o);
-               free(r);
+        const jsonObject* hasPermStr = 
+            jsonObjectGetKeyConst(result, "permission.usr_has_perm");
 
-               if(evt)
-                       break;
-       }
+        if (!oilsUtilsIsDBTrue(jsonObjectGetString(hasPermStr))) {
+            evt = oilsNewEvent3(
+                OSRF_LOG_MARK, OILS_EVENT_PERM_FAILURE, perm, orgid);
+        }
+
+        jsonObjectFree(params);
+        jsonObjectFree(result);
+
+        // return first failed permission check.
+        if (evt) return evt;
+    }
 
-       return evt;
+    return NULL; // all perm checks succeeded
 }
 
 /**