[open-ils-commits] r12227 - trunk/Open-ILS/src/c-apps (scottmk)

[svn at svn.open-ils.org]

Author: scottmk
Date: 2009-02-18 21:12:46 -0500 (Wed, 18 Feb 2009)
New Revision: 12227

Modified:
   trunk/Open-ILS/src/c-apps/oils_cstore.c
Log:
In searchJOIN: substantially rewrote the code for the condition where
the JSON query specifies neither of the join columns.

1. Corrected an apparent bug whereby we could join in only one
direction, even when a join in the opposite direction was
valid and equivalent.

2. Rewrote the search loops for better performance.

3. When testing for the success of the searches for join columns,
success should mean that we have identified both columns.  Identifying
just one isn't enough.



Modified: trunk/Open-ILS/src/c-apps/oils_cstore.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_cstore.c	2009-02-19 00:53:45 UTC (rev 12226)
+++ trunk/Open-ILS/src/c-apps/oils_cstore.c	2009-02-19 02:12:46 UTC (rev 12227)
@@ -2080,35 +2080,46 @@
 		} else if (!field && !fkey) {
 			osrfHash* _links = oilsIDLFindPath("/%s/links", leftclass);
 
-			int i = 0;
-			const char* tmp_fkey;
-			osrfStringArray* keys = osrfHashKeys( _links );
-			while ( (tmp_fkey = osrfStringArrayGetString(keys, i++)) ) {
-				if ( !strcmp( (char*)oilsIDLFindPath("/%s/links/%s/class", leftclass, tmp_fkey), class) ) {
-					field = strdup( (char*)oilsIDLFindPath("/%s/links/%s/key", leftclass, tmp_fkey) );
-					fkey = strdup(tmp_fkey);
+			// For each link defined for the left class:
+			// see if the link references the joined class
+			osrfHashIterator* itr = osrfNewHashIterator( _links );
+			osrfHash* curr_link = NULL;
+			while( (curr_link = osrfHashIteratorNext( itr ) ) ) {
+				const char* other_class = osrfHashGet( curr_link, "class" );
+				if( other_class && !strcmp( other_class, class ) ) {
+
+					// Found a link between the classes
+					fkey = strdup( osrfHashIteratorKey( itr ) );
+					const char* other_key = osrfHashGet( curr_link, "key" );
+					field = other_key ? strdup( other_key ) : NULL;
 					break;
 				}
 			}
-			osrfStringArrayFree(keys);
-
-			if (!field && !fkey) {
+			osrfHashIteratorFree( itr );
+			
+			if (!field || !fkey) {
+				// Do another such search, with the classes reversed
 				_links = oilsIDLFindPath("/%s/links", class);
 
-				i = 0;
-				const char* tmp_fld;
-				keys = osrfHashKeys( _links );
-				while ( (tmp_fld = osrfStringArrayGetString(keys, i++)) ) {
-					if ( !strcmp( (char*)oilsIDLFindPath("/%s/links/%s/class", class, tmp_fld), class) ) {
-						fkey = strdup( (char*)oilsIDLFindPath("/%s/links/%s/key", class, tmp_fld) );
-						field = strdup( tmp_fld );
+				// For each link defined for the joined class:
+				// see if the link references the left class
+				osrfHashIterator* itr = osrfNewHashIterator( _links );
+				osrfHash* curr_link = NULL;
+				while( (curr_link = osrfHashIteratorNext( itr ) ) ) {
+					const char* other_class = osrfHashGet( curr_link, "class" );
+					if( other_class && !strcmp( other_class, leftclass ) ) {
+
+						// Found a link between the classes
+						fkey = strdup( osrfHashIteratorKey( itr ) );
+						const char* other_key = osrfHashGet( curr_link, "key" );
+						field = other_key ? strdup( other_key ) : NULL;
 						break;
 					}
 				}
-				osrfStringArrayFree(keys);
+				osrfHashIteratorFree( itr );
 			}
 
-			if (!field && !fkey) {
+			if (!field || !fkey) {
 				osrfLogError(
 					OSRF_LOG_MARK,
 					"%s: JOIN failed.  No link defined between %s and %s",