Author: brane Date: Tue Jan 15 10:20:26 2019 New Revision: 1851333 URL: http://svn.apache.org/viewvc?rev=1851333&view=rev Log: Fix issue #4801: Make JavaHL blame return byte[] file contents in the blame callback instead of assuming they can be converted to String.
[in subversion/bindings/javahl/src/org/apache/subversion/javahl] * ISVNClient.java (ISVNClient.blame): Add a new overload that uses the new BlameLineCallback. Deprecate the other two overloads that use BlameCallback. * SVNClient.java (SVNClient.blame): Implement new native overload and deprecate the old ones. (SVNClient.BlameCallbackAdapter): New helper class. * callback/BlameCallback.java (BlameCallback): Deprecated. * callback/BlameLineCallback.java (BlameLineCallback): New, replaces BlameCallback. [in subversion/bindings/javahl/tests/org/apache/subversion/javahl] * BasicTests.java (testBasicBlame, testBlameWithDiffOptions): Suppress deprecation warnings as these tests use the old API, and should continue to do so in order to test the callback adapter. (testBinaryBlame): New test case. (collectBlameLines, BlameCallbackImpl): Suppress deprecation warnings. (BlameLineCallbackImpl): New helper class. * ExceptionTests.java (testBlameCallback): Use the new API in this test case. [in subversion/bindings/javahl/native] * org_apache_subversion_javahl_SVNClient.cpp (Java_org_apache_subversion_javahl_SVNClient_blame): Update parameter order. * BlameCallback.cpp (BlameCallback::singleLine): Use BlameLineCallback instead of BlameCallback. Added: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameLineCallback.java (with props) Modified: subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameCallback.java subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/ExceptionTests.java Modified: subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp?rev=1851333&r1=1851332&r2=1851333&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp (original) +++ subversion/trunk/subversion/bindings/javahl/native/BlameCallback.cpp Tue Jan 15 10:20:26 2019 @@ -89,13 +89,13 @@ BlameCallback::singleLine(apr_int64_t li static jmethodID mid = 0; if (mid == 0) { - jclass clazz = env->FindClass(JAVAHL_CLASS("/callback/BlameCallback")); + jclass clazz = env->FindClass(JAVAHL_CLASS("/callback/BlameLineCallback")); if (JNIUtil::isJavaExceptionThrown()) POP_AND_RETURN(SVN_NO_ERROR); mid = env->GetMethodID(clazz, "singleLine", "(JJLjava/util/Map;JLjava/util/Map;" - "Ljava/lang/String;Ljava/lang/String;Z)V"); + "Ljava/lang/String;Z[B)V"); if (JNIUtil::isJavaExceptionThrown() || mid == 0) POP_AND_RETURN(SVN_NO_ERROR); } @@ -117,14 +117,14 @@ BlameCallback::singleLine(apr_int64_t li if (JNIUtil::isJavaExceptionThrown()) POP_AND_RETURN(SVN_NO_ERROR); - jstring jline = JNIUtil::makeJString(line->data); + jbyteArray jline = JNIUtil::makeJByteArray(line); if (JNIUtil::isJavaExceptionThrown()) POP_AND_RETURN(SVN_NO_ERROR); // call the Java method env->CallVoidMethod(m_callback, mid, (jlong)line_no, (jlong)revision, jrevProps, (jlong)mergedRevision, jmergedRevProps, - jmergedPath, jline, (jboolean)localChange); + jmergedPath, (jboolean)localChange, jline); POP_AND_RETURN_EXCEPTION_AS_SVNERROR(); } Modified: subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp?rev=1851333&r1=1851332&r2=1851333&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp (original) +++ subversion/trunk/subversion/bindings/javahl/native/org_apache_subversion_javahl_SVNClient.cpp Tue Jan 15 10:20:26 2019 @@ -1658,8 +1658,8 @@ JNIEXPORT void JNICALL Java_org_apache_subversion_javahl_SVNClient_blame (JNIEnv *env, jobject jthis, jstring jpath, jobject jpegRevision, jobject jrevisionStart, jobject jrevisionEnd, jboolean jignoreMimeType, - jboolean jincludeMergedRevisions, jobject jblameCallback, - jobject jdiffOptions) + jboolean jincludeMergedRevisions, jobject jdiffOptions, + jobject jblameCallback) { JNIEntry(SVNClient, blame); SVNClient *cl = SVNClient::getCppObject(jthis); Modified: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java?rev=1851333&r1=1851332&r2=1851333&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java (original) +++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/ISVNClient.java Tue Jan 15 10:20:26 2019 @@ -1394,17 +1394,34 @@ public interface ISVNClient * @param ignoreMimeType whether or not to ignore the mime-type * @param includeMergedRevisions whether or not to include extra merge * information + * @param options additional options for controlling the output * @param callback callback to receive the file content and the other * information - * @param options additional options for controlling the output * @throws ClientException - * @since 1.9 + * @since 1.12 + */ + void blame(String path, Revision pegRevision, Revision revisionStart, + Revision revisionEnd, boolean ignoreMimeType, + boolean includeMergedRevisions, + DiffOptions options, BlameLineCallback callback) + throws ClientException; + + /** + * Retrieve the content together with the author, the revision and the date + * of the last change of each line + * <p> + * Behaves like the 1.12 version but uses BlameCallback instead of + * BlameLineCallback. The former expects that file contents can be + * converted from UTF-8 to a String, which is not true in general + * and may throw exceptions. + * @deprecated Use the 1.12 version with BlameLineCallback */ + @Deprecated void blame(String path, Revision pegRevision, Revision revisionStart, Revision revisionEnd, boolean ignoreMimeType, boolean includeMergedRevisions, BlameCallback callback, DiffOptions options) - throws ClientException; + throws ClientException; /** * Retrieve the content together with the author, the revision and the date @@ -1412,12 +1429,14 @@ public interface ISVNClient * <p> * Behaves like the 1.9 version with <code>options</code> set to * their default values. + * @deprecated Use the 1.12 version with BlameLineCallback */ + @Deprecated void blame(String path, Revision pegRevision, Revision revisionStart, Revision revisionEnd, boolean ignoreMimeType, boolean includeMergedRevisions, BlameCallback callback) - throws ClientException; + throws ClientException; /** * Set directory for the configuration information, taking the Modified: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java?rev=1851333&r1=1851332&r2=1851333&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java (original) +++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java Tue Jan 15 10:20:26 2019 @@ -30,6 +30,7 @@ import java.io.OutputStream; import java.io.FileOutputStream; import java.io.FileNotFoundException; import java.io.ByteArrayOutputStream; +import java.io.UnsupportedEncodingException; import java.util.Collection; import java.util.Collections; @@ -705,6 +706,7 @@ public class SVNClient implements ISVNCl boolean ignoreExternals) throws ClientException; + @Deprecated public void blame(String path, Revision pegRevision, Revision revisionStart, Revision revisionEnd, boolean ignoreMimeType, @@ -716,13 +718,27 @@ public class SVNClient implements ISVNCl includeMergedRevisions, callback, null); } + @Deprecated + public void blame(String path, Revision pegRevision, + Revision revisionStart, + Revision revisionEnd, boolean ignoreMimeType, + boolean includeMergedRevisions, + BlameCallback callback, + DiffOptions options) + throws ClientException + { + blame(path, pegRevision, revisionStart, revisionEnd, + ignoreMimeType, includeMergedRevisions, options, + new BlameCallbackAdapter(callback)); + } + public native void blame(String path, Revision pegRevision, Revision revisionStart, Revision revisionEnd, boolean ignoreMimeType, boolean includeMergedRevisions, - BlameCallback callback, - DiffOptions options) - throws ClientException; + DiffOptions options, + BlameLineCallback callback) + throws ClientException; public native void setConfigDirectory(String configDir) throws ClientException; @@ -897,4 +913,42 @@ public class SVNClient implements ISVNCl null); } } + + /** + * A private class that adapts from BlameLineCallback to BlameCallback. + */ + @Deprecated + private class BlameCallbackAdapter implements BlameLineCallback + { + private BlameCallback wrappedCallback = null; + + public BlameCallbackAdapter(BlameCallback callback) + { + wrappedCallback = callback; + } + + // Implementation of BlameLineCallback + public void singleLine(long lineNum, long revision, + Map<String, byte[]> revProps, long mergedRevision, + Map<String, byte[]> mergedRevProps, + String mergedPath, boolean localChange, + byte[] line) + throws ClientException + { + if (wrappedCallback == null) + return; + + String convertedLine = null; + try { + convertedLine = new String(line, "UTF-8"); + } + catch (UnsupportedEncodingException ex) { + throw ClientException.fromException(ex); + } + + wrappedCallback.singleLine(lineNum, revision, + revProps, mergedRevision, mergedRevProps, + mergedPath, convertedLine, localChange); + } + } } Modified: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameCallback.java URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameCallback.java?rev=1851333&r1=1851332&r2=1851333&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameCallback.java (original) +++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameCallback.java Tue Jan 15 10:20:26 2019 @@ -31,7 +31,9 @@ import org.apache.subversion.javahl.ISVN /** * This interface is used to receive every single line for a file on a * the {@link ISVNClient#blame} call. + * @deprecated use {@link BlameLineCallback} instead. */ +@Deprecated public interface BlameCallback { /** Added: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameLineCallback.java URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameLineCallback.java?rev=1851333&view=auto ============================================================================== --- subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameLineCallback.java (added) +++ subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameLineCallback.java Tue Jan 15 10:20:26 2019 @@ -0,0 +1,56 @@ +/** + * @copyright + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * ==================================================================== + * @endcopyright + */ + +package org.apache.subversion.javahl.callback; + +import org.apache.subversion.javahl.ClientException; + +import java.util.Map; +import org.apache.subversion.javahl.ISVNClient; + +/** + * This interface is used to receive every single line for a file on a + * the {@link ISVNClient#blame} call. + * @since 1.12 + */ +public interface BlameLineCallback +{ + /** + * the method will be called for every line in a file. + * @param lineNum the line number for this line + * @param revision the revision of the last change. + * @param revProps the revision properties for this revision. + * @param mergedRevision the revision of the last merged change. + * @param mergedRevProps the revision properties for the last merged + * change. + * @param mergedPath the path of the last merged change. + * @param localChange true if the line was locally modified. + * @param line the line in the file. + */ + public void singleLine(long lineNum, long revision, + Map<String, byte[]> revProps, long mergedRevision, + Map<String, byte[]> mergedRevProps, + String mergedPath, boolean localChange, + byte[] line) + throws ClientException; +} Propchange: subversion/trunk/subversion/bindings/javahl/src/org/apache/subversion/javahl/callback/BlameLineCallback.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java?rev=1851333&r1=1851332&r2=1851333&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java (original) +++ subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/BasicTests.java Tue Jan 15 10:20:26 2019 @@ -22,6 +22,8 @@ */ package org.apache.subversion.javahl; +import static org.junit.Assert.*; + import org.apache.subversion.javahl.callback.*; import org.apache.subversion.javahl.types.*; @@ -4093,6 +4095,7 @@ public class BasicTests extends SVNTests * @throws Throwable * @since 1.5 */ + @SuppressWarnings("deprecation") public void testBasicBlame() throws Throwable { OneTest thisTest = new OneTest(); @@ -4122,6 +4125,7 @@ public class BasicTests extends SVNTests * Test blame with diff options. * @since 1.9 */ + @SuppressWarnings("deprecation") public void testBlameWithDiffOptions() throws Throwable { OneTest thisTest = new OneTest(); @@ -4155,6 +4159,46 @@ public class BasicTests extends SVNTests } /** + * Test the new 1.12 blame interface on a file with null bytes. + * @throws Throwable + * @since 1.12 + */ + public void testBinaryBlame() throws Throwable + { + final byte[] lineIn = {0x0, 0x0, 0x0, 0xa}; + final byte[] lineOut = {0x0, 0x0, 0x0}; + + OneTest thisTest = new OneTest(); + // Modify the file iota, adding null bytes. + File iota = new File(thisTest.getWorkingCopy(), "iota"); + FileOutputStream stream = new FileOutputStream(iota, false); + stream.write(lineIn); + stream.close(); + Set<String> srcPaths = new HashSet<String>(1); + srcPaths.add(thisTest.getWCPath()); + try { + client.username("rayjandom"); + client.commit(srcPaths, Depth.infinity, false, false, null, null, + new ConstMsg("NUL bytes written to /iota"), null); + } finally { + client.username("jrandom"); + } + + // Test the current interface + BlameLineCallbackImpl callback = new BlameLineCallbackImpl(); + client.blame(thisTest.getWCPath() + "/iota", Revision.HEAD, + Revision.getInstance(0), Revision.HEAD, + false, false, null, callback); + assertEquals(1, callback.numberOfLines()); + + BlameLineCallbackImpl.BlameLine line = callback.getBlameLine(0); + assertNotNull(line); + assertEquals(2, line.getRevision()); + assertEquals("rayjandom", line.getAuthor()); + assertArrayEquals(lineOut, line.getLine()); + } + + /** * Test commit of arbitrary revprops. * @throws Throwable * @since 1.5 @@ -4679,6 +4723,7 @@ public class BasicTests extends SVNTests return callback.getMessages(); } + @SuppressWarnings("deprecation") private byte[] collectBlameLines(String path, Revision pegRevision, Revision revisionStart, Revision revisionEnd, @@ -4766,6 +4811,7 @@ public class BasicTests extends SVNTests } /* A blame callback implementation. */ + @SuppressWarnings("deprecation") protected class BlameCallbackImpl implements BlameCallback { @@ -4978,6 +5024,146 @@ public class BasicTests extends SVNTests } } } + + /* A blame callback implementation. */ + protected class BlameLineCallbackImpl implements BlameLineCallback + { + + /** list of blame records (lines) */ + private List<BlameLine> lines = new ArrayList<BlameLine>(); + + public void singleLine(long lineNum, long rev, + Map<String, byte[]> revProps, + long mergedRevision, + Map<String, byte[]> mergedRevProps, + String mergedPath, boolean localChange, + byte[] line) + throws ClientException + { + DateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS"); + + try { + insertLine( + df.parse(new String(revProps.get("svn:date"))), + rev, + new String(revProps.get("svn:author")), + mergedRevProps == null ? null + : df.parse(new String(mergedRevProps.get("svn:date"))), + mergedRevision, + mergedRevProps == null ? null + : new String(mergedRevProps.get("svn:author")), + mergedPath, line); + } catch (ParseException e) { + throw ClientException.fromException(e); + } + } + + private Date getDate(Date date, Date merged_date) { + return (merged_date == null ? date : merged_date); + } + + private String getAuthor(String author, String merged_author) { + return (merged_author == null ? author : merged_author); + } + + private long getRevision(long revision, long merged_revision) { + return (merged_revision == -1 ? revision : merged_revision); + } + + private void insertLine(Date date, long revision, String author, + Date merged_date, long merged_revision, + String merged_author, String merged_path, + byte[] line) + { + this.lines.add(new BlameLine(getRevision(revision, merged_revision), + getAuthor(author, merged_author), + getDate(date, merged_date), + line)); + } + + /** + * Retrieve the number of line of blame information + * @return number of lines of blame information + */ + public int numberOfLines() + { + return this.lines.size(); + } + + /** + * Retrieve blame information for specified line number + * @param i the line number to retrieve blame information about + * @return Returns object with blame information for line + */ + public BlameLine getBlameLine(int i) + { + if (i >= this.lines.size()) + { + return null; + } + return this.lines.get(i); + } + + /** + * Class represeting one line of the lines, i.e. a blame record + */ + public final class BlameLine + { + private long revision; + private String author; + private Date changed; + private byte[] line; + + /** + * Constructor + * + * @param revision + * @param author + * @param changed + * @param line + */ + public BlameLine(long revision, String author, + Date changed, byte[] line) + { + this.revision = revision; + this.author = author; + this.changed = changed; + this.line = line; + } + + /** + * @return Returns the author. + */ + public String getAuthor() + { + return author; + } + + /** + * @return Returns the date changed. + */ + public Date getChanged() + { + return changed; + } + + /** + * @return Returns the source line content. + */ + public byte[] getLine() + { + return line; + } + + /** + * @return Returns the revision. + */ + public long getRevision() + { + return revision; + } + } + } /** A helper which calls update with a bunch of default args. */ private long update(OneTest thisTest) Modified: subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/ExceptionTests.java URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/ExceptionTests.java?rev=1851333&r1=1851332&r2=1851333&view=diff ============================================================================== --- subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/ExceptionTests.java (original) +++ subversion/trunk/subversion/bindings/javahl/tests/org/apache/subversion/javahl/ExceptionTests.java Tue Jan 15 10:20:26 2019 @@ -206,18 +206,17 @@ public class ExceptionTests extends SVNT { client.blame(thisTest.getWorkingCopy() + "/iota", Revision.getInstance(1), Revision.getInstance(1), - Revision.getInstance(1), false, false, - new BlameCallback() - { - public void singleLine(long lineNum, long revision, - Map<String, byte[]> revProps, long mergedRevision, - Map<String, byte[]> mergedRevProps, - String mergedPath, String line, - boolean localChange) - { - throw new TestException("inner", theException); - } - }); + Revision.getInstance(1), false, false, null, + new BlameLineCallback() { + public void singleLine(long lineNum, long revision, + Map<String, byte[]> revProps, long mergedRevision, + Map<String, byte[]> mergedRevProps, + String mergedPath, boolean localChange, + byte[] line) + { + throw new TestException("inner", theException); + } + }); } catch (ClientException e) {