You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/10/12 01:51:17 UTC

svn commit: r1531466 - in /subversion/trunk/subversion/bindings/javahl: native/CreateJ.cpp native/JNIUtil.cpp native/JNIUtil.h src/org/apache/subversion/javahl/ClientNotifyInformation.java

Author: brane
Date: Fri Oct 11 23:51:16 2013
New Revision: 1531466

URL: http://svn.apache.org/r1531466
Log:
Restructure the code in JavaHL that creates error messages from
svn_error_t chains, and expose error chains in the client notifications.

[in subversion/bindings/javahl/src/org/apache/subversion/javahl]
* ClientNotifyInformation.java
  (ClientNotifyInformation.serialVersionUID): Update serialization identity.
  (ClientNotifyInformation.errMsgStack): New private member.
  (ClientNotifyInformation.ClientNotifyInformation): Initialize errMsgStack.
  (ClientNotifyInformation.getErrMsgDetails): New accessor for errMsgStack.

[in subversion/bindings/javahl/native]
* JNIUtil.h (JNIUtil::makeSVNErrorMessage): Change signature.
  (JNIUtil::message_stack_item, JNIUtil::error_message_stack_t): Removed.
  (JNIUtil::assembleErrorMessage): Removed.
* JNIUtil.cpp (JNIUtil::makeSVNErrorMessage): Reimplement. Returns an
   error message and message stack without tracing info.
  (MessageStackItem, ErrorMessageStack): New types in namespace scope.
  (assemble_error_message): New, replaces JNIUtil::assembleErrorMessage.
  (construct_Jmessage_stack): Use the new namespace-scope types.
  (JNIUtil::handleSVNError): Use makeSVNErrorMessage to get the messages.
  (JNIUtil::assembleErrorMessage): Removed.

Modified:
    subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp
    subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
    subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h
    subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ClientNotifyInformation.java

Modified: subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp?rev=1531466&r1=1531465&r2=1531466&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/CreateJ.cpp Fri Oct 11 23:51:16 2013
@@ -778,7 +778,7 @@ CreateJ::ClientNotifyInformation(const s
                                "L"JAVA_PACKAGE"/types/NodeKind;"
                                "Ljava/lang/String;"
                                "L"JAVA_PACKAGE"/types/Lock;"
-                               "Ljava/lang/String;"
+                               "Ljava/lang/String;Ljava/util/List;"
                                "L"JAVA_PACKAGE"/ClientNotifyInformation$Status;"
                                "L"JAVA_PACKAGE"/ClientNotifyInformation$Status;"
                                "L"JAVA_PACKAGE"/ClientNotifyInformation$LockStatus;"
@@ -811,7 +811,9 @@ CreateJ::ClientNotifyInformation(const s
   if (JNIUtil::isJavaExceptionThrown())
     POP_AND_RETURN_NULL;
 
-  jstring jErr = JNIUtil::makeSVNErrorMessage(wcNotify->err);
+  jstring jErr;
+  jobject jErrStack;
+  JNIUtil::makeSVNErrorMessage(wcNotify->err, &jErr, &jErrStack);
   if (JNIUtil::isJavaExceptionThrown())
     POP_AND_RETURN_NULL;
 
@@ -867,7 +869,7 @@ CreateJ::ClientNotifyInformation(const s
 
   // call the Java method
   jobject jInfo = env->NewObject(clazz, midCT, jPath, jAction,
-                                 jKind, jMimeType, jLock, jErr,
+                                 jKind, jMimeType, jLock, jErr, jErrStack,
                                  jContentState, jPropState, jLockState,
                                  (jlong) wcNotify->revision, jChangelistName,
                                  jMergeRange, jpathPrefix, jpropName,

Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp?rev=1531466&r1=1531465&r2=1531466&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp (original)
+++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.cpp Fri Oct 11 23:51:16 2013
@@ -403,16 +403,6 @@ void JNIUtil::raiseThrowable(const char 
   env->DeleteLocalRef(clazz);
 }
 
-jstring JNIUtil::makeSVNErrorMessage(svn_error_t *err)
-{
-  if (err == NULL)
-    return NULL;
-  std::string buffer;
-  assembleErrorMessage(err, 0, APR_SUCCESS, buffer);
-  jstring jmessage = makeJString(buffer.c_str());
-  return jmessage;
-}
-
 void
 JNIUtil::throwNativeException(const char *className, const char *msg,
                               const char *source, int aprErr)
@@ -514,8 +504,91 @@ JNIUtil::putErrorsInTrace(svn_error_t *e
 }
 
 namespace {
-jobject construct_Jmessage_stack(
-    const JNIUtil::error_message_stack_t& message_stack)
+struct MessageStackItem
+{
+  apr_status_t m_code;
+  std::string m_message;
+  bool m_generic;
+
+  MessageStackItem(apr_status_t code, const char* message,
+                   bool generic = false)
+    : m_code(code),
+      m_message(message),
+      m_generic(generic)
+    {}
+};
+typedef std::vector<MessageStackItem> ErrorMessageStack;
+
+/*
+ * Build the error message from the svn error into buffer.  This
+ * method iterates through all the chained errors
+ *
+ * @param err               the subversion error
+ * @param buffer            the buffer where the formated error message will
+ *                          be stored
+ * @return An array of error codes and messages
+ */
+ErrorMessageStack assemble_error_message(
+    svn_error_t *err, std::string &result)
+{
+  // buffer for a single error message
+  char errbuf[1024];
+  apr_status_t parent_apr_err = 0;
+  ErrorMessageStack message_stack;
+
+  /* Pretty-print the error */
+  /* Note: we can also log errors here someday. */
+
+  for (int depth = 0; err;
+       ++depth, parent_apr_err = err->apr_err, err = err->child)
+    {
+      /* When we're recursing, don't repeat the top-level message if its
+       * the same as before. */
+      if (depth == 0 || err->apr_err != parent_apr_err)
+        {
+          const char *message;
+          /* Is this a Subversion-specific error code? */
+          if ((err->apr_err > APR_OS_START_USEERR)
+              && (err->apr_err <= APR_OS_START_CANONERR))
+            message = svn_strerror(err->apr_err, errbuf, sizeof(errbuf));
+          /* Otherwise, this must be an APR error code. */
+          else
+            {
+              /* Messages coming from apr_strerror are in the native
+                 encoding, it's a good idea to convert them to UTF-8. */
+              apr_strerror(err->apr_err, errbuf, sizeof(errbuf));
+              svn_error_t* utf8_err =
+                svn_utf_cstring_to_utf8(&message, errbuf, err->pool);
+              if (utf8_err)
+                {
+                  /* Use fuzzy transliteration instead. */
+                  svn_error_clear(utf8_err);
+                  message = svn_utf_cstring_from_utf8_fuzzy(errbuf, err->pool);
+                }
+            }
+
+          message_stack.push_back(
+              MessageStackItem(err->apr_err, message, true));
+        }
+      if (err->message)
+        {
+          message_stack.push_back(
+              MessageStackItem(err->apr_err, err->message));
+        }
+    }
+
+  for (ErrorMessageStack::const_iterator it = message_stack.begin();
+       it != message_stack.end(); ++it)
+    {
+      if (!it->m_generic)
+        result += "svn: ";
+      result += it->m_message;
+      result += '\n';
+    }
+  return message_stack;
+}
+
+jobject construct_Jmessage_stack(const ErrorMessageStack& message_stack)
 {
   JNIEnv *env = JNIUtil::getEnv();
   env->PushLocalFrame(LOCAL_FRAME_SIZE);
@@ -544,8 +617,7 @@ jobject construct_Jmessage_stack(
   if (JNIUtil::isJavaExceptionThrown())
     POP_AND_RETURN_NULL;
 
-  for (JNIUtil::error_message_stack_t::const_iterator
-         it = message_stack.begin();
+  for (ErrorMessageStack::const_iterator it = message_stack.begin();
        it != message_stack.end(); ++it)
     {
       jobject jmessage = JNIUtil::makeJString(it->m_message.c_str());
@@ -567,12 +639,37 @@ jobject construct_Jmessage_stack(
 }
 } // anonymous namespace
 
+std::string JNIUtil::makeSVNErrorMessage(svn_error_t *err,
+                                         jstring *jerror_message,
+                                         jobject *jmessage_stack)
+{
+  if (jerror_message)
+    *jerror_message = NULL;
+  if (jmessage_stack)
+    *jmessage_stack = NULL;
+
+  std::string buffer;
+  err = svn_error_purge_tracing(err);
+  if (err == NULL || err->apr_err == 0
+      || !(jerror_message || jmessage_stack))
+  return buffer;
+
+  ErrorMessageStack message_stack = assemble_error_message(err, buffer);
+  if (jerror_message)
+    *jerror_message = makeJString(buffer.c_str());
+  if (jmessage_stack)
+    *jmessage_stack = construct_Jmessage_stack(message_stack);
+  return buffer;
+}
+
 void JNIUtil::handleSVNError(svn_error_t *err)
 {
-  std::string msg;
-  error_message_stack_t message_stack;
-  assembleErrorMessage(svn_error_purge_tracing(err),
-                       0, APR_SUCCESS, msg, &message_stack);
+  jstring jmessage;
+  jobject jstack;
+  std::string msg = makeSVNErrorMessage(err, &jmessage, &jstack);
+  if (JNIUtil::isJavaExceptionThrown())
+    return;
+
   const char *source = NULL;
 #ifdef SVN_DEBUG
 #ifndef SVN_ERR__TRACING
@@ -616,17 +713,10 @@ void JNIUtil::handleSVNError(svn_error_t
   if (isJavaExceptionThrown())
     POP_AND_RETURN_NOTHING();
 
-  jstring jmessage = makeJString(msg.c_str());
-  if (isJavaExceptionThrown())
-    POP_AND_RETURN_NOTHING();
   jstring jsource = makeJString(source);
   if (isJavaExceptionThrown())
     POP_AND_RETURN_NOTHING();
 
-  jobject jmessageStack = construct_Jmessage_stack(message_stack);
-  if (isJavaExceptionThrown())
-    POP_AND_RETURN_NOTHING();
-
   jmethodID mid = env->GetMethodID(clazz, "<init>",
                                    "(Ljava/lang/String;"
                                    "Ljava/lang/String;I"
@@ -634,7 +724,7 @@ void JNIUtil::handleSVNError(svn_error_t
   if (isJavaExceptionThrown())
     POP_AND_RETURN_NOTHING();
   jobject nativeException = env->NewObject(clazz, mid, jmessage, jsource,
-                                           jint(err->apr_err), jmessageStack);
+                                           jint(err->apr_err), jstack);
   if (isJavaExceptionThrown())
     POP_AND_RETURN_NOTHING();
 
@@ -1044,71 +1134,6 @@ jbyteArray JNIUtil::makeJByteArray(const
 }
 
 /**
- * Build the error message from the svn error into buffer.  This
- * method calls itselft recursively for all the chained errors
- *
- * @param err               the subversion error
- * @param depth             the depth of the call, used for formating
- * @param parent_apr_err    the apr of the previous level, used for formating
- * @param buffer            the buffer where the formated error message will
- *                          be stored
- * @param message_stack     an array of error codes and messages
- */
-void JNIUtil::assembleErrorMessage(svn_error_t *err, int depth,
-                                   apr_status_t parent_apr_err,
-                                   std::string &buffer,
-                                   error_message_stack_t* message_stack)
-{
-  // buffer for a single error message
-  char errbuf[1024];
-
-  /* Pretty-print the error */
-  /* Note: we can also log errors here someday. */
-
-  /* When we're recursing, don't repeat the top-level message if its
-   * the same as before. */
-  if (depth == 0 || err->apr_err != parent_apr_err)
-    {
-      const char *message;
-      /* Is this a Subversion-specific error code? */
-      if ((err->apr_err > APR_OS_START_USEERR)
-          && (err->apr_err <= APR_OS_START_CANONERR))
-        message = svn_strerror(err->apr_err, errbuf, sizeof(errbuf));
-      /* Otherwise, this must be an APR error code. */
-      else
-        {
-          /* Messages coming from apr_strerror are in the native
-             encoding, it's a good idea to convert them to UTF-8. */
-          apr_strerror(err->apr_err, errbuf, sizeof(errbuf));
-          svn_error_t* utf8_err =
-            svn_utf_cstring_to_utf8(&message, errbuf, err->pool);
-          if (utf8_err)
-            {
-              /* Use fuzzy transliteration instead. */
-              svn_error_clear(utf8_err);
-              message = svn_utf_cstring_from_utf8_fuzzy(errbuf, err->pool);
-            }
-        }
-
-      if (message_stack)
-        message_stack->push_back(
-            message_stack_item(err->apr_err, message, true));
-      buffer.append(message);
-      buffer.append("\n");
-    }
-  if (err->message)
-    {
-      if (message_stack)
-        message_stack->push_back(
-            message_stack_item(err->apr_err, err->message));
-      buffer.append(_("svn: ")).append(err->message).append("\n");
-    }
-
-  if (err->child)
-    assembleErrorMessage(err->child, depth + 1, err->apr_err, buffer);
-}
-
-/**
  * Throw a Java NullPointerException.  Used when input parameters
  * which should not be null are that.
  *

Modified: subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h?rev=1531466&r1=1531465&r2=1531466&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h (original)
+++ subversion/trunk/subversion/bindings/javahl/native/JNIUtil.h Fri Oct 11 23:51:16 2013
@@ -122,7 +122,9 @@ class JNIUtil
    */
   static void handleSVNError(svn_error_t *err);
 
-  static jstring makeSVNErrorMessage(svn_error_t *err);
+  static std::string makeSVNErrorMessage(svn_error_t *err,
+                                         jstring *jerror_message,
+                                         jobject *jmessage_stack);
 
   /**
    * Create and throw a java.lang.Throwable instance.
@@ -150,31 +152,12 @@ class JNIUtil
   enum { formatBufferSize = 2048 };
   enum { noLog, errorLog, exceptionLog, entryLog } LogLevel;
 
-  struct message_stack_item
-  {
-    apr_status_t m_code;
-    std::string m_message;
-    bool m_generic;
-
-    message_stack_item(apr_status_t code, const char* message,
-                       bool generic = false)
-      : m_code(code),
-        m_message(message),
-        m_generic(generic)
-      {}
-  };
-  typedef std::vector<message_stack_item> error_message_stack_t;
-
   /**
    * Mutex that secures the global configuration object.
    */
   static JNIMutex *g_configMutex;
 
  private:
-  static void assembleErrorMessage(svn_error_t *err, int depth,
-                                   apr_status_t parent_apr_err,
-                                   std::string &buffer,
-                                   error_message_stack_t* message_stack = NULL);
   static void putErrorsInTrace(svn_error_t *err,
                                std::vector<jobject> &stackTrace);
   /**

Modified: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ClientNotifyInformation.java
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ClientNotifyInformation.java?rev=1531466&r1=1531465&r2=1531466&view=diff
==============================================================================
--- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ClientNotifyInformation.java (original)
+++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ClientNotifyInformation.java Fri Oct 11 23:51:16 2013
@@ -23,10 +23,12 @@
 
 package org.apache.subversion.javahl;
 
+import org.apache.subversion.javahl.types.*;
+import org.apache.subversion.javahl.callback.ClientNotifyCallback;
+
+import java.util.List;
 import java.util.Map;
 import java.util.EventObject;
-import org.apache.subversion.javahl.callback.ClientNotifyCallback;
-import org.apache.subversion.javahl.types.*;
 
 /**
  * The event passed to the {@link ClientNotifyCallback#onNotify}
@@ -41,7 +43,7 @@ public class ClientNotifyInformation ext
     // http://java.sun.com/j2se/1.4/pdf/serial-spec.pdf
     // http://java.sun.com/j2se/1.5.0/docs/guide/serialization/spec/version.html#6678
     // http://java.sun.com/javase/6/docs/platform/serialization/spec/version.html#6678
-    private static final long serialVersionUID = 1L;
+    private static final long serialVersionUID = 2L;
 
     /**
      * The {@link Action} which triggered this event.
@@ -69,6 +71,12 @@ public class ClientNotifyInformation ext
     private String errMsg;
 
     /**
+     * A detailed stack of error messages for the item.
+     * @see ClientException
+     */
+    private List<ClientException.ErrorMessage> errMsgStack;
+
+    /**
      * The {@link Status} of the content of the item.
      */
     private Status contentState;
@@ -157,6 +165,7 @@ public class ClientNotifyInformation ext
         this.mimeType = mimeType;
         this.lock = lock;
         this.errMsg = errMsg;
+        this.errMsgStack = null;
         this.contentState = contentState;
         this.propState = propState;
         this.lockState = lockState;
@@ -176,6 +185,34 @@ public class ClientNotifyInformation ext
     }
 
     /**
+     * This constructor will be called only by the native code.
+     *
+     * In addition to all the other parameters, sets the detailed
+     * message stack.
+     */
+    protected ClientNotifyInformation(String path, Action action, NodeKind kind,
+                             String mimeType, Lock lock, String errMsg,
+                             List<ClientException.ErrorMessage> errMsgStack,
+                             Status contentState, Status propState,
+                             LockStatus lockState, long revision,
+                             String changelistName, RevisionRange mergeRange,
+                             String pathPrefix, String propName,
+                             Map<String, String> revProps, long oldRevision,
+                             long hunkOriginalStart, long hunkOriginalLength,
+                             long hunkModifiedStart, long hunkModifiedLength,
+                             long hunkMatchedLine, int hunkFuzz)
+    {
+        this(path, action, kind, mimeType, lock, errMsg,
+             contentState, propState, lockState, revision,
+             changelistName, mergeRange, pathPrefix, propName,
+             revProps, oldRevision,
+             hunkOriginalStart, hunkOriginalLength,
+             hunkModifiedStart, hunkModifiedLength,
+             hunkMatchedLine, hunkFuzz);
+        this.errMsgStack = errMsgStack;
+    }
+
+    /**
      * @return The path of the item, which is the source of the event.
      */
     public String getPath()
@@ -224,6 +261,14 @@ public class ClientNotifyInformation ext
     }
 
     /**
+     * @return Details about the error message for the item.
+     */
+    public List<ClientException.ErrorMessage> getErrMsgDetails()
+    {
+        return errMsgStack;
+    }
+
+    /**
      * @return The {@link Status} of the content of the item.
      */
     public Status getContentState()