[open-ils-commits] r15668 - trunk/Open-ILS/src/c-apps (scottmk)
svn at svn.open-ils.org
svn at svn.open-ils.org
Tue Mar 2 22:24:25 EST 2010
Author: scottmk
Date: 2010-03-02 22:24:23 -0500 (Tue, 02 Mar 2010)
New Revision: 15668
Modified:
trunk/Open-ILS/src/c-apps/oils_cstore.c
Log:
1. Encapsulated into a few small functions the management of session-level
information (currently just the transaction ID).
2. Fixed a bug in the treatment of the transaction ID that had caused us to
issue useless extra rollbacks whenever we did a commit or a rollback.
3. Further tinkered with comments.
M Open-ILS/src/c-apps/oils_cstore.c
Modified: trunk/Open-ILS/src/c-apps/oils_cstore.c
===================================================================
--- trunk/Open-ILS/src/c-apps/oils_cstore.c 2010-03-02 23:05:16 UTC (rev 15667)
+++ trunk/Open-ILS/src/c-apps/oils_cstore.c 2010-03-03 03:24:23 UTC (rev 15668)
@@ -85,6 +85,10 @@
static int verifyObjectClass ( osrfMethodContext*, const jsonObject* );
+static void setXactId( osrfMethodContext* ctx );
+static inline const char* getXactId( osrfMethodContext* ctx );
+static inline void clearXactId( osrfMethodContext* ctx );
+
int beginTransaction ( osrfMethodContext* );
int commitTransaction ( osrfMethodContext* );
int rollbackTransaction ( osrfMethodContext* );
@@ -544,7 +548,7 @@
free(tabledef);
- osrfLogDebug( OSRF_LOG_MARK, "%s Investigatory SQL = %s",
+ osrfLogDebug( OSRF_LOG_MARK, "%s Investigatory SQL = %s",
MODULENAME, OSRF_BUFFER_C_STR( query_buf ) );
dbi_result result = dbi_conn_query( writehandle, OSRF_BUFFER_C_STR( query_buf ) );
@@ -648,41 +652,112 @@
@param blob A pointer to the osrfHash to be freed, cast to a void pointer.
This function is a callback, to be called by the application session when it ends.
+ The application session stores the osrfHash via an opaque pointer.
- See also sessionDataFree().
+ If the osrfHash contains an entry for the key "xact_id", it means that an
+ uncommitted transaction is pending. Roll it back.
*/
void userDataFree( void* blob ) {
- osrfHashFree( (osrfHash*) blob );
+ osrfHash* hash = (osrfHash*) blob;
+ if( osrfHashGet( hash, "xact_id" ) && writehandle )
+ dbi_conn_query( writehandle, "ROLLBACK;" );
+
+ osrfHashFree( hash );
}
/**
- @brief Roll back a pending transaction at the end of the application session.
+ @name Managing session data
+ @brief Maintain data stored via the userData pointer of the application session.
+
+ Currently, session-level data is stored in an osrfHash. Other arrangements are
+ possible, and some would be more efficient. The application session calls a
+ callback function to free userData before terminating.
+
+ Currently, the only data we store at the session level is the transaction id. By this
+ means we can ensure that any pending transactions are rolled back before the application
+ session terminates.
+*/
+/*@{*/
+
+/**
+ @brief Free an item in the application session's userData.
@param key The name of a key for an osrfHash.
@param item An opaque pointer to the item associated with the key.
- We store an osrfHash with the application session, and arrange (by installing
- userDataFree() as a different callback) for the session to free that osrfHash before
- terminating.
+ We store an osrfHash as userData with the application session, and arrange (by
+ installing userDataFree() as a different callback) for the session to free that
+ osrfHash before terminating.
This function is a callback for freeing items in the osrfHash. If the item has a key
of "xact_id", the item is a transaction id for a transaction that is still pending.
- So, if we're still connected, we do a rollback.
+ It is just a character string, so we free it.
+
+ Currently the transaction ID is the only thing that we store in this osrfHash. If we
+ ever store anything else in it, we will need to revisit this function so that it will
+ free whatever else needs freeing (which may or may not be just a character string).
*/
static void sessionDataFree( char* key, void* item ) {
if ( !strcmp(key,"xact_id") ) {
- if ( writehandle )
- dbi_conn_query( writehandle, "ROLLBACK;" );
free( item );
}
}
/**
+ @brief Save a transaction id.
+ @param ctx Pointer to the method context.
+
+ Save the session_id of the current application session as a transaction id.
+*/
+static void setXactId( osrfMethodContext* ctx ) {
+ if( ctx && ctx->session ) {
+ osrfAppSession* session = ctx->session;
+
+ // If the session doesn't already have a hash, create one. Make sure
+ // that the application session frees the hash when it terminates.
+ if( NULL == session->userData ) {
+ session->userData = osrfNewHash();
+ osrfHashSetCallback( (osrfHash*) session->userData, &sessionDataFree );
+ ctx->session->userDataFree = &userDataFree;
+ }
+
+ // Save the transaction id in the hash, with the key "xact_id"
+ osrfHashSet( (osrfHash*) session->userData, strdup( session->session_id ),
+ "xact_id" );
+ }
+}
+
+/**
+ @brief Get the transaction ID for the current transaction, if any.
+ @param ctx Pointer to the method context.
+ @return Pointer to the transaction ID.
+
+ The return value points to an internal buffer, and will become invalid upon issuing
+ a commit or rollback.
+*/
+static inline const char* getXactId( osrfMethodContext* ctx ) {
+ if( ctx && ctx->session && ctx->session->userData )
+ return osrfHashGet( (osrfHash*) ctx->session->userData, "xact_id" );
+ else
+ return NULL;
+}
+
+/**
+ @brief Clear the current transaction id.
+ @param ctx Pointer to the method context.
+*/
+static inline void clearXactId( osrfMethodContext* ctx ) {
+ if( ctx && ctx->session && ctx->session->userData )
+ osrfHashRemove( ctx->session->userData, "xact_id" );
+}
+/*@}*/
+
+/**
@brief Implement the transaction.begin method.
- @param Pointer to the method context.
+ @param ctx Pointer to the method context.
@return Zero if successful, or -1 upon error.
- Start a transaction. Return a transaction ID (actually the session id of the
- application session) to the client.
+ Start a transaction. Return a transaction ID (actually the session_id of the
+ application session) to the client, and save it for future reference.
*/
int beginTransaction ( osrfMethodContext* ctx ) {
if(osrfMethodVerifyContext( ctx )) {
@@ -704,22 +779,10 @@
"osrfMethodException", ctx->request, "Error starting transaction" );
return -1;
} else {
- jsonObject* ret = jsonNewObject( ctx->session->session_id );
+ setXactId( ctx );
+ jsonObject* ret = jsonNewObject( getXactId( ctx ) );
osrfAppRespondComplete( ctx, ret );
jsonObjectFree(ret);
-
- // Store a copy of the application session ID as a transaction id in an osrfHash,
- // to be stored with the application session. Install some callbacks so that, if
- // the session ends while the transaction is still active, we will do a rollback.
- if (!ctx->session->userData) {
- ctx->session->userData = osrfNewHash();
- osrfHashSetCallback((osrfHash*)ctx->session->userData, &sessionDataFree);
- }
-
- osrfHashSet( (osrfHash*)ctx->session->userData, strdup( ctx->session->session_id ),
- "xact_id" );
- ctx->session->userDataFree = &userDataFree;
-
}
return 0;
}
@@ -739,8 +802,8 @@
jsonObjectFree(user);
#endif
- if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
- osrfAppSessionStatus(
+ if( getXactId( ctx ) == NULL ) {
+ osrfAppSessionStatus(
ctx->session,
OSRF_STATUS_INTERNALSERVERERROR,
"osrfMethodException",
@@ -759,7 +822,7 @@
"%s: Error creating savepoint %s in transaction %s",
MODULENAME,
spName,
- osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )
+ getXactId( ctx )
);
osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR,
"osrfMethodException", ctx->request, "Error creating savepoint" );
@@ -787,7 +850,7 @@
jsonObjectFree(user);
#endif
- if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
+ if( getXactId( ctx ) == NULL ) {
osrfAppSessionStatus(
ctx->session,
OSRF_STATUS_INTERNALSERVERERROR,
@@ -807,7 +870,7 @@
"%s: Error releasing savepoint %s in transaction %s",
MODULENAME,
spName,
- osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )
+ getXactId( ctx )
);
osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR,
"osrfMethodException", ctx->request, "Error releasing savepoint" );
@@ -835,7 +898,7 @@
jsonObjectFree(user);
#endif
- if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
+ if( getXactId( ctx ) == NULL ) {
osrfAppSessionStatus(
ctx->session,
OSRF_STATUS_INTERNALSERVERERROR,
@@ -855,7 +918,7 @@
"%s: Error rolling back savepoint %s in transaction %s",
MODULENAME,
spName,
- osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )
+ getXactId( ctx )
);
osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR,
"osrfMethodException", ctx->request, "Error rolling back savepoint" );
@@ -881,7 +944,7 @@
jsonObjectFree(user);
#endif
- if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
+ if( getXactId( ctx ) == NULL ) {
osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR,
"osrfMethodException", ctx->request, "No active transaction to commit" );
return -1;
@@ -894,7 +957,7 @@
"osrfMethodException", ctx->request, "Error committing transaction" );
return -1;
} else {
- osrfHashRemove(ctx->session->userData, "xact_id");
+ clearXactId( ctx );
jsonObject* ret = jsonNewObject(ctx->session->session_id);
osrfAppRespondComplete( ctx, ret );
jsonObjectFree(ret);
@@ -915,7 +978,7 @@
jsonObjectFree(user);
#endif
- if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
+ if( getXactId( ctx ) == NULL ) {
osrfAppSessionStatus( ctx->session, OSRF_STATUS_INTERNALSERVERERROR,
"osrfMethodException", ctx->request, "No active transaction to roll back" );
return -1;
@@ -928,7 +991,7 @@
"osrfMethodException", ctx->request, "Error rolling back transaction" );
return -1;
} else {
- osrfHashRemove(ctx->session->userData, "xact_id");
+ clearXactId( ctx );
jsonObject* ret = jsonNewObject(ctx->session->session_id);
osrfAppRespondComplete( ctx, ret );
jsonObjectFree(ret);
@@ -1531,16 +1594,12 @@
}
/**
-Utility function: create a JSON_HASH with a single key/value pair.
-This function is equivalent to:
+ @brief Create a JSON_HASH with a single key/value pair.
+ @param key The key of the key/value pair.
+ @param value the value of the key/value pair.
+ @return Pointer to a newly created jsonObject of type JSON_HASH.
- jsonParseFmt( "{\"%s\":\"%s\"}", key, value )
-
-or, if value is NULL:
-
- jsonParseFmt( "{\"%s\":null}", key )
-
-...but faster because it doesn't create and parse a JSON string.
+ The value of the key/value is either a string or (if @a value is NULL) a null.
*/
static jsonObject* single_hash( const char* key, const char* value ) {
// Sanity check
@@ -1571,10 +1630,7 @@
osrfLogDebug( OSRF_LOG_MARK, "Object seems to be of the correct type" );
- char* trans_id = NULL;
- if( ctx->session && ctx->session->userData )
- trans_id = osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" );
-
+ const char* trans_id = getXactId( ctx );
if ( !trans_id ) {
osrfLogError( OSRF_LOG_MARK, "No active transaction -- required for CREATE" );
@@ -5059,7 +5115,7 @@
return jsonNULL;
}
- if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
+ if( getXactId( ctx ) == NULL ) {
osrfAppSessionStatus(
ctx->session,
OSRF_STATUS_BADREQUEST,
@@ -5086,9 +5142,8 @@
}
dbhandle = writehandle;
+ const char* trans_id = getXactId( ctx );
- char* trans_id = osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" );
-
// Set the last_xact_id
int index = oilsIDL_ntop( target->classname, "last_xact_id" );
if (index > -1) {
@@ -5256,7 +5311,7 @@
osrfHash* meta = osrfHashGet( (osrfHash*) ctx->method->userData, "class" );
- if (!osrfHashGet( (osrfHash*)ctx->session->userData, "xact_id" )) {
+ if( getXactId( ctx ) == NULL ) {
osrfAppSessionStatus(
ctx->session,
OSRF_STATUS_BADREQUEST,
More information about the open-ils-commits
mailing list