tdf#135316 remove OTempFileService pessimisation

Closing the temp file when we read to the end, and then opening
it again later actually is quite expensive, just leave it open.

This takes my load time from 22s to 19s

Also clean up the unit-test that failed, so that I can get
a useful stack trace out of it when something fails,
specifically
(*) throw an exception when something goes wrong, instead
of just writing a message to stdout
(*) don't catch exceptions and write useless messages - just
let the exception flow up to the JUnit handler, which will log
a nice stacktrace with line numbers.

Change-Id: If55c997f91eea4e703e92c632961d68b3453076d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117517
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
(cherry picked from commit 218f36dd614cf828e949f605faaf6a6fd615da26)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/117571

6 files changed, 116 insertions, 261 deletions

diff --git a/unotools/qa/complex/tempfile/TempFileUnitTest.java b/unotools/qa/complex/tempfile/TempFileUnitTest.java
index 2d23db0dbc1a..c737deaed70a 100644
--- a/unotools/qa/complex/tempfile/TempFileUnitTest.java
+++ b/unotools/qa/complex/tempfile/TempFileUnitTest.java
@@ -46,7 +46,7 @@ public class TempFileUnitTest
         }
         catch ( Exception e )
         {
-            fail ( "Cannot get simple file access! Exception: " + e);
+            fail ( "Cannot get simple file access!" + e);
         }
         if ( m_xSFA == null ) {
             fail ( "Cannot get simple file access!" );
@@ -58,12 +58,12 @@ public class TempFileUnitTest
         m_xSFA = null;
     }
 
-    @Test public void ExecuteTest01() {
+    @Test public void ExecuteTest01() throws Exception {
         Test01 aTest = new Test01( m_xMSF, m_xSFA );
         assertTrue( "Test01 failed!", aTest.test() );
     }
 
-    @Test public void ExecuteTest02() {
+    @Test public void ExecuteTest02() throws Exception {
         Test02 aTest = new Test02( m_xMSF, m_xSFA );
         assertTrue( "Test02 failed!", aTest.test() );
     }
diff --git a/unotools/qa/complex/tempfile/Test01.java b/unotools/qa/complex/tempfile/Test01.java
index ed8e729854c8..f93b02186e8d 100644
--- a/unotools/qa/complex/tempfile/Test01.java
+++ b/unotools/qa/complex/tempfile/Test01.java
@@ -34,70 +34,57 @@ public class Test01 {
         m_aTestHelper = new TestHelper( "Test01: ");
     }
 
-    public boolean test() {
+    public boolean test() throws Exception {
         XTempFile xTempFile = null;
         String sFileURL = null;
         String sFileName = null;
         //create a temporary file.
-        try {
-            Object oTempFile = m_xMSF.createInstance( "com.sun.star.io.TempFile" );
-            xTempFile = UnoRuntime.queryInterface(XTempFile.class, oTempFile);
-            m_aTestHelper.Message( "Tempfile created." );
-            UnoRuntime.queryInterface(XTruncate.class, oTempFile);
-        } catch( Exception e ) {
-            m_aTestHelper.Error( "Cannot create TempFile. exception: " + e );
-            return false;
-        }
+        Object oTempFile = m_xMSF.createInstance( "com.sun.star.io.TempFile" );
+        xTempFile = UnoRuntime.queryInterface(XTempFile.class, oTempFile);
+        m_aTestHelper.Message( "Tempfile created." );
+        UnoRuntime.queryInterface(XTruncate.class, oTempFile);
 
         //retrieve the tempfile URL
-        if ( xTempFile == null ) {
-            m_aTestHelper.Error( "Cannot get XTempFile interface." );
-            return false;
-        }
+        if ( xTempFile == null )
+            throw new java.lang.Exception( "Cannot get XTempFile interface." );
 
-        try {
-            //compare the file name with the name in the URL.
-            sFileURL = m_aTestHelper.GetTempFileURL( xTempFile );
-            sFileName = m_aTestHelper.GetTempFileName( xTempFile );
-            m_aTestHelper.CompareFileNameAndURL( sFileName, sFileURL );
+        //compare the file name with the name in the URL.
+        sFileURL = m_aTestHelper.GetTempFileURL( xTempFile );
+        sFileName = m_aTestHelper.GetTempFileName( xTempFile );
+        m_aTestHelper.CompareFileNameAndURL( sFileName, sFileURL );
 
-            //write to the stream using the service.
-            byte pBytesIn[] = new byte[9];
-            byte pBytesOut1[][] = new byte [1][9];
-            byte pBytesOut2[][] = new byte [1][9];
-            Random oRandom = new Random();
-            oRandom.nextBytes( pBytesIn );
-            m_aTestHelper.WriteBytesWithStream( pBytesIn, xTempFile );
+        //write to the stream using the service.
+        byte pBytesIn[] = new byte[9];
+        byte pBytesOut1[][] = new byte [1][9];
+        byte pBytesOut2[][] = new byte [1][9];
+        Random oRandom = new Random();
+        oRandom.nextBytes( pBytesIn );
+        m_aTestHelper.WriteBytesWithStream( pBytesIn, xTempFile );
 
-            //check the result by reading from the service.
-            xTempFile.seek(0);
-            m_aTestHelper.ReadBytesWithStream( pBytesOut1, pBytesIn.length + 1, xTempFile );
-            for ( int i = 0; i < pBytesIn.length ; i++ ) {
-                if ( pBytesOut1[0][i] != pBytesIn[i] ) {
-                    m_aTestHelper.Error( "Tempfile outputs false data!" );
-                }
+        //check the result by reading from the service.
+        xTempFile.seek(0);
+        m_aTestHelper.ReadBytesWithStream( pBytesOut1, pBytesIn.length + 1, xTempFile );
+        for ( int i = 0; i < pBytesIn.length ; i++ ) {
+            if ( pBytesOut1[0][i] != pBytesIn[i] ) {
+                throw new java.lang.Exception( "Tempfile outputs false data!" );
             }
+        }
 
-            //check the result by reading from the file directly.
-            m_aTestHelper.ReadDirectlyFromTempFile( pBytesOut2, pBytesIn.length + 1, m_xSFA, sFileURL );
-            for ( int i = 0; i < pBytesIn.length; i++ ) {
-                if ( pBytesOut2[0][i] != pBytesIn[i] ) {
-                    m_aTestHelper.Error( "Tempfile contains false data!" );
-                }
+        //check the result by reading from the file directly.
+        m_aTestHelper.ReadDirectlyFromTempFile( pBytesOut2, pBytesIn.length + 1, m_xSFA, sFileURL );
+        for ( int i = 0; i < pBytesIn.length; i++ ) {
+            if ( pBytesOut2[0][i] != pBytesIn[i] ) {
+                throw new java.lang.Exception( "Tempfile contains false data!" );
             }
+        }
 
-            //close the object(by closing input and output), check that the file was removed.
-            xTempFile.setRemoveFile( false );
-            m_aTestHelper.CloseTempFile( xTempFile );
-            if( !m_aTestHelper.IfTempFileExists( m_xSFA, sFileURL ) ) {
-                m_aTestHelper.Error( "TempFile mistakenly removed. " );
-            } else {
-                m_aTestHelper.KillTempFile( sFileURL, m_xSFA );
-            }
-        } catch ( Exception e ) {
-            m_aTestHelper.Error( "Exception: " + e );
-            return false;
+        //close the object(by closing input and output), check that the file was removed.
+        xTempFile.setRemoveFile( false );
+        m_aTestHelper.CloseTempFile( xTempFile );
+        if( !m_aTestHelper.IfTempFileExists( m_xSFA, sFileURL ) ) {
+            throw new java.lang.Exception( "TempFile mistakenly removed. " );
         }
+        m_aTestHelper.KillTempFile( sFileURL, m_xSFA );
         return true;
     }
 }
diff --git a/unotools/qa/complex/tempfile/Test02.java b/unotools/qa/complex/tempfile/Test02.java
index afef403fbfe4..528f0cb60a02 100644
--- a/unotools/qa/complex/tempfile/Test02.java
+++ b/unotools/qa/complex/tempfile/Test02.java
@@ -37,47 +37,38 @@ public class Test02 {
         m_aTestHelper = new TestHelper( "Test02: ");
     }
 
-    public boolean test() {
+    public boolean test() throws Exception {
         Object oTempFile = null;
         XTempFile xTempFile = null;
         String sFileURL = null;
         //create a temporary file.
-        try {
-            oTempFile = m_xMSF.createInstance( "com.sun.star.io.TempFile" );
-            xTempFile = UnoRuntime.queryInterface(XTempFile.class, oTempFile);
-            m_aTestHelper.Message( "Tempfile created." );
-            UnoRuntime.queryInterface(XTruncate.class, oTempFile);
-        } catch(Exception e) {
-            m_aTestHelper.Error( "Cannot create TempFile. exception: " + e );
-            return false;
-        }
-        try {
-            //write something.
-            byte pBytesIn[] = new byte[9];
-            byte pBytesOut[][] = new byte[1][9];
-            Random oRandom = new Random();
-            oRandom.nextBytes( pBytesIn );
-            m_aTestHelper.WriteBytesWithStream( pBytesIn, xTempFile );
+        oTempFile = m_xMSF.createInstance( "com.sun.star.io.TempFile" );
+        xTempFile = UnoRuntime.queryInterface(XTempFile.class, oTempFile);
+        m_aTestHelper.Message( "Tempfile created." );
+        UnoRuntime.queryInterface(XTruncate.class, oTempFile);
+
+        //write something.
+        byte pBytesIn[] = new byte[9];
+        byte pBytesOut[][] = new byte[1][9];
+        Random oRandom = new Random();
+        oRandom.nextBytes( pBytesIn );
+        m_aTestHelper.WriteBytesWithStream( pBytesIn, xTempFile );
 
-            //get the URL.
-            sFileURL = m_aTestHelper.GetTempFileURL( xTempFile );
+        //get the URL.
+        sFileURL = m_aTestHelper.GetTempFileURL( xTempFile );
 
-            //let the service not to remove the URL.
-            m_aTestHelper.SetTempFileRemove( xTempFile, false );
+        //let the service not to remove the URL.
+        m_aTestHelper.SetTempFileRemove( xTempFile, false );
 
-            //close the tempfile by closing input and output.
-            m_aTestHelper.CloseTempFile( xTempFile );
+        //close the tempfile by closing input and output.
+        m_aTestHelper.CloseTempFile( xTempFile );
 
-            //check that the file is still available.
-            m_aTestHelper.ReadDirectlyFromTempFile( pBytesOut, pBytesIn.length + 1, m_xSFA, sFileURL );
-            for ( int i = 0; i < pBytesIn.length; i++ ) {
-                if ( pBytesOut[0][i] != pBytesIn[i] ) {
-                    m_aTestHelper.Error( "Tempfile contains false data!" );
-                }
+        //check that the file is still available.
+        m_aTestHelper.ReadDirectlyFromTempFile( pBytesOut, pBytesIn.length + 1, m_xSFA, sFileURL );
+        for ( int i = 0; i < pBytesIn.length; i++ ) {
+            if ( pBytesOut[0][i] != pBytesIn[i] ) {
+                throw new Exception( "Tempfile contains false data!" );
             }
-        } catch ( Exception e) {
-            m_aTestHelper.Error("Exception: " + e);
-            return false;
         }
         return true;
     }
diff --git a/unotools/qa/complex/tempfile/TestHelper.java b/unotools/qa/complex/tempfile/TestHelper.java
index 237c95b9ef11..821c33da054b 100644
--- a/unotools/qa/complex/tempfile/TestHelper.java
+++ b/unotools/qa/complex/tempfile/TestHelper.java
@@ -17,183 +17,105 @@
  */
 package complex.tempfile;
 
-
-
 import com.sun.star.io.*;
-
+import com.sun.star.uno.*;
 import com.sun.star.uno.AnyConverter;
 import com.sun.star.ucb.XSimpleFileAccess;
-
+import java.io.*;
 
 public class TestHelper {
 
     private String m_sTestPrefix;
 
     public TestHelper( String sTestPrefix ) {
-
         m_sTestPrefix = sTestPrefix;
     }
+
     public void SetTempFileRemove( XTempFile xTempFile, boolean b ) {
-        try {
-            xTempFile.setRemoveFile( b );
-        } catch( Exception e ) {
-            Error( "Cannot set TempFileRemove. exception: " + e );
-        }
+        xTempFile.setRemoveFile( b );
     }
 
-    public String GetTempFileURL ( XTempFile xTempFile ) {
-        String sTempFileURL = null;
-        try {
-            sTempFileURL = AnyConverter.toString( xTempFile.getUri() );
-        } catch (Exception e) {
-            Error ( "Cannot get TempFileURL. exception: " + e );
-        }
+    public String GetTempFileURL ( XTempFile xTempFile ) throws java.lang.Exception {
+        String sTempFileURL = AnyConverter.toString( xTempFile.getUri() );
         if ( sTempFileURL == null || sTempFileURL.equals("") ) {
-            Error ( "Temporary file not valid." );
+            throw new java.lang.Exception( "Temporary file not valid." );
         }
         return sTempFileURL;
     }
 
-    public String GetTempFileName( XTempFile xTempFile ) {
-        String sTempFileName = null;
-        try {
-            sTempFileName = AnyConverter.toString( xTempFile.getResourceName() );
-        } catch ( Exception e ) {
-            Error( "Cannot get TempFileName. exception: " + e );
-        }
+    public String GetTempFileName( XTempFile xTempFile ) throws java.lang.Exception {
+        String sTempFileName = AnyConverter.toString( xTempFile.getResourceName() );
         if ( sTempFileName == null || sTempFileName.equals("") ) {
-            Error( "Temporary file not valid." );
+            throw new java.lang.Exception( "Temporary file not valid." );
         }
         return sTempFileName;
     }
 
-    public boolean CompareFileNameAndURL ( String sTempFileName, String sTempFileURL ) {
-        boolean bRet = false;
-        try {
-            bRet = sTempFileURL.endsWith( sTempFileName.replaceAll( "\\\\" , "/" ) );
-            Message ( "Compare file name and URL: " +
-                    ( bRet ? "OK." : "ERROR: FILE NAME AND URL DO NOT MATCH." ) );
-        }
-        catch ( Exception e ) {
-            Error ( "exception: " + e);
-        }
+    public boolean CompareFileNameAndURL ( String sTempFileName, String sTempFileURL ) throws java.lang.Exception {
+        boolean bRet = sTempFileURL.endsWith( sTempFileName.replaceAll( "\\\\" , "/" ) );
+        if (!bRet)
+            throw new java.lang.Exception("FILE NAME AND URL DO NOT MATCH." );
         return bRet;
     }
 
-    public void WriteBytesWithStream( byte [] pBytes, XTempFile xTempFile ) {
-        try {
-            XOutputStream xOutTemp = xTempFile.getOutputStream();
-            if ( xOutTemp == null ) {
-                Error( "Cannot get output stream." );
-            } else {
-                xOutTemp.writeBytes( pBytes );
-                Message ( "Write to tempfile successfully." );
-            }
-        } catch ( Exception e ) {
-            Error( "Cannot write to stream. exception: " + e );
-        }
+    public void WriteBytesWithStream( byte [] pBytes, XTempFile xTempFile ) throws java.lang.Exception {
+        XOutputStream xOutTemp = xTempFile.getOutputStream();
+        if ( xOutTemp == null )
+            throw new java.lang.Exception( "Cannot get output stream." );
+        xOutTemp.writeBytes( pBytes );
+        xOutTemp.flush();
+        Message ( "Write " + pBytes.length + " bytes to tempfile successfully." );
     }
 
-    public void ReadBytesWithStream( byte [][] pBytes, int nBytes, XTempFile xTempFile ) {
-        try {
-            XInputStream xInTemp = xTempFile.getInputStream();
-            if ( xInTemp == null ) {
-                Error( "Cannot get input stream from tempfile." );
-            } else {
-                xInTemp.readBytes( pBytes, nBytes );
-                Message ( "Read from tempfile successfully." );
-            }
-        } catch ( Exception e ) {
-            Error( "Cannot read from stream. exception: " + e );
-        }
+    public void ReadBytesWithStream( byte [][] pBytes, int nBytes, XTempFile xTempFile ) throws java.lang.Exception {
+        XInputStream xInTemp = xTempFile.getInputStream();
+        if ( xInTemp == null )
+            throw new java.lang.Exception( "Cannot get input stream from tempfile." );
+        int n = xInTemp.readBytes( pBytes, nBytes );
+        Message ( "Read " + n + " bytes from tempfile successfully." );
     }
+
     public void ReadDirectlyFromTempFile( byte [][] pBytes, int nBytes,  XSimpleFileAccess xSFA, String sTempFileURL )
+         throws java.lang.Exception
     {
-        try
-        {
-            if ( xSFA != null ) {
-                XInputStream xInTemp = xSFA.openFileRead( sTempFileURL );
-                if ( xInTemp != null )
-                {
-                    xInTemp.readBytes( pBytes, nBytes );
-                    xInTemp.closeInput();
-                    Message ( "Read directly from tempfile successfully." );
-                } else {
-                    Error ( "Cannot create input stream from URL." );
-                }
-            }
-        }
-        catch ( Exception e)
-        {
-            Error( "Exception caught in TestHelper." +
-                    "ReadDirectlyFromTempFile(). exception: " + e );
-        }
+        XInputStream xInTemp = xSFA.openFileRead( sTempFileURL );
+        if ( xInTemp == null )
+            throw new java.lang.Exception("Cannot create input stream from URL.");
+        int n = xInTemp.readBytes( pBytes, nBytes );
+        xInTemp.closeInput();
+        Message ( "Read " + n + " bytes directly from tempfile successfully. " + sTempFileURL );
     }
 
-    public void CloseTempFile( XTempFile xTempFile ) {
+    public void CloseTempFile( XTempFile xTempFile ) throws java.lang.Exception {
         XOutputStream xOutTemp = null;
         XInputStream xInTemp = null;
-        try {
-            xOutTemp = xTempFile.getOutputStream();
-            if ( xOutTemp == null ) {
-                Error( "Cannot get output stream." );
-            }
-        } catch ( Exception e ) {
-            Error( "Cannot get output stream. exception:" + e );
+        xOutTemp = xTempFile.getOutputStream();
+        if ( xOutTemp == null ) {
+            throw new java.lang.Exception( "Cannot get output stream." );
         }
-        try {
-            xOutTemp.closeOutput();
-        } catch( Exception e ) {
-            Error( "Cannot close output stream. exception:" + e );
-        }
-        try {
-            xInTemp = xTempFile.getInputStream();
-            if ( xInTemp == null ) {
-                Error( "Cannot get input stream." );
-            }
-        } catch ( Exception e ) {
-            Error( "Cannot get input stream. exception:" + e );
-        }
-        try {
-            xInTemp.closeInput();
-            Message ( "Tempfile closed successfully." );
-        } catch( Exception e ) {
-            Error( "Cannot close input stream. exception:" + e );
+        xOutTemp.closeOutput();
+        xInTemp = xTempFile.getInputStream();
+        if ( xInTemp == null ) {
+            throw new java.lang.Exception( "Cannot get input stream." );
         }
+        xInTemp.closeInput();
+        Message ( "Tempfile closed successfully." );
     }
 
-    public void KillTempFile ( String sTempFileURL, XSimpleFileAccess xSFA ) {
-        try {
-            if ( sTempFileURL != null && xSFA != null ) {
-                xSFA.kill( sTempFileURL );
-                Message ( "Tempfile killed successfully." );
-            }
-        }
-        catch ( Exception e ) {
-            Error ( "Exception caught in TestHelper." +
-                    "KillTempFile(): " + e);
-        }
+    public void KillTempFile ( String sTempFileURL, XSimpleFileAccess xSFA ) throws com.sun.star.uno.Exception {
+        xSFA.kill( sTempFileURL );
+        Message ( "Tempfile killed successfully." );
     }
 
-    public boolean IfTempFileExists( XSimpleFileAccess xSFA, String sTempFileURL ) {
+    public boolean IfTempFileExists( XSimpleFileAccess xSFA, String sTempFileURL )
+        throws com.sun.star.uno.Exception
+    {
         boolean bRet = false;
-        try {
-            if ( sTempFileURL != null && xSFA != null ) {
-                bRet = xSFA.exists( sTempFileURL );
-                Message ( "Tempfile " + ( bRet ? "still " : "no longer " ) + "exists." );
-            }
-        }
-        catch( Exception e ) {
-            Error( "Exception caught in TestHelper." +
-                    "IfTempFileExists(): " + e );
-        }
+        bRet = xSFA.exists( sTempFileURL );
+        Message ( "Tempfile " + ( bRet ? "still " : "no longer " ) + "exists." );
         return bRet;
     }
 
-    public void Error( String sError ) {
-        System.out.println( m_sTestPrefix + "Error: " + sError );
-    }
-
     public void Message( String sMessage ) {
         System.out.println( m_sTestPrefix + sMessage );
     }
diff --git a/unotools/source/ucbhelper/XTempFile.hxx b/unotools/source/ucbhelper/XTempFile.hxx
index f390f37ba958..80036263f0b9 100644
--- a/unotools/source/ucbhelper/XTempFile.hxx
+++ b/unotools/source/ucbhelper/XTempFile.hxx
@@ -55,9 +55,6 @@ protected:
     bool mbInClosed;
     bool mbOutClosed;
 
-    sal_Int64 mnCachedPos;
-    bool mbHasCachedPos;
-
     void checkError () const;
     void checkConnected ();
 
diff --git a/unotools/source/ucbhelper/xtempfile.cxx b/unotools/source/ucbhelper/xtempfile.cxx
index 7eb7e3677e18..ae8140526441 100644
--- a/unotools/source/ucbhelper/xtempfile.cxx
+++ b/unotools/source/ucbhelper/xtempfile.cxx
@@ -32,9 +32,6 @@ OTempFileService::OTempFileService(css::uno::Reference< css::uno::XComponentCont
 , mbRemoveFile( true )
 , mbInClosed( false )
 , mbOutClosed( false )
-, mnCachedPos( 0 )
-, mbHasCachedPos( false )
-
 {
     mpTempFile.reset(new utl::TempFile());
     mpTempFile->EnableKillingFile();
@@ -127,18 +124,6 @@ sal_Int32 SAL_CALL OTempFileService::readBytes( css::uno::Sequence< sal_Int8 >&
     if (nRead < o3tl::make_unsigned(aData.getLength()))
         aData.realloc( nRead );
 
-    if ( sal::static_int_cast<sal_uInt32>(nBytesToRead) > nRead )
-    {
-        // usually that means that the stream was read till the end
-        // TODO/LATER: it is better to get rid of this optimization by avoiding using of multiple temporary files ( there should be only one temporary file? )
-        mnCachedPos = mpStream->Tell();
-        mbHasCachedPos = true;
-
-        mpStream = nullptr;
-        if ( mpTempFile )
-            mpTempFile->CloseStream();
-    }
-
     return nRead;
 }
 sal_Int32 SAL_CALL OTempFileService::readSomeBytes( css::uno::Sequence< sal_Int8 >& aData, sal_Int32 nMaxBytesToRead )
@@ -233,17 +218,6 @@ void SAL_CALL OTempFileService::closeOutput(  )
 
     mbOutClosed = true;
 
-    // TODO/LATER: it is better to get rid of this optimization by avoiding using of multiple temporary files ( there should be only one temporary file? )
-    if ( mpStream )
-    {
-        mnCachedPos = mpStream->Tell();
-        mbHasCachedPos = true;
-
-        mpStream = nullptr;
-        if ( mpTempFile )
-            mpTempFile->CloseStream();
-    }
-
     if ( mbInClosed )
     {
         // stream will be deleted by TempFile implementation
@@ -260,23 +234,7 @@ void OTempFileService::checkError () const
 void OTempFileService::checkConnected ()
 {
     if (!mpStream && mpTempFile)
-    {
         mpStream = mpTempFile->GetStream( StreamMode::STD_READWRITE );
-        if ( mpStream && mbHasCachedPos )
-        {
-            mpStream->Seek( sal::static_int_cast<std::size_t>(mnCachedPos) );
-            if ( mpStream->SvStream::GetError () == ERRCODE_NONE )
-            {
-                mbHasCachedPos = false;
-                mnCachedPos = 0;
-            }
-            else
-            {
-                mpStream = nullptr;
-                mpTempFile->CloseStream();
-            }
-        }
-    }
 
     if (!mpStream)
         throw css::io::NotConnectedException ( OUString(), static_cast < css::uno::XWeak * > (this ) );