I've done a quick search through a few common JDBC drivers to see how
they handle closing of blob input streams (see attached file).
Closing twice is not a problem for those either.
I didn't think closing the input stream twice was the main worry. My
main concern was the possibility of reading the input stream after
reaching EOF. One way to achieve this would be with a stream that
returned true for markSupported(). reset() could be called at any
time, and the whole stream could be re-read without re-opening. (Note
- the DB2 driver supports this).
So it's clear - there could be code out there that would get broken by
this change, but the fix would be easy.
I would prefer updating the JavaDoc over adding the boolean parameter.
The extra parameter would need to propagate up to all the public
callers of POIFSFileSystem (listed below). Many of these methods
already have been overloaded for other reasons.
I think the greater good (allowing simplification of 99.x% of calling
code) should win out. However, if it is decided not to make the
change, we should add a test case to demonstrate this requirement.
I've attached a attached a version of the junit test which asserts the
inverse (stream never closed).
-josh
-- -- -- --
//Public callers of POIFSFileSystem.<init>(InputStream):
org.apache.poi.hdf.model.HDFObjectFactory.getTypes(InputStream)
org.apache.poi.hdf.model.HDFObjectFactory.HDFObjectFactory(InputStream,
HDFLowLevelParsingListener)
org.apache.poi.hdgf.extractor.VisioTextExtractor.VisioTextExtractor(InputStream)
org.apache.poi.hslf.extractor.PowerPointExtractor.PowerPointExtractor(InputStream)
org.apache.poi.hsmf.MAPIMessage.MAPIMessage(InputStream)
org.apache.poi.hssf.usermodel.HSSFWorkbook.HSSFWorkbook(InputStream, boolean)
org.apache.poi.hwpf.HWPFDocument.verifyAndBuildPOIFS(InputStream)
org.apache.poi.hslf.dev.SlideShowDumper.SlideShowDumper(InputStream)
org.apache.poi.hslf.extractor.QuickButCruddyTextExtractor.QuickButCruddyTextExtractor(InputStream)
org.apache.poi.hslf.HSLFSlideShow.HSLFSlideShow(InputStream)
On Feb 7, 2008 3:48 AM, Nick Burch <[EMAIL PROTECTED]> wrote:
> On Thu, 7 Feb 2008, Yegor Kozlov wrote:
> > With java.io.* streams calling fis.close() twice doesn't do any harm.
> > BUT think of reading via JDBC input streams ( from Oracle or MySQL
> > blobs, etc.). Are you sure they will be happy with closing of already
> > closed stream?
>
> My worry is there will be badly behaved inputstreams out there, which we
> may start breaking. Servlet engines, odd jdbc drivers etc
>
> > What we can do is to create a special constructor:
> > POIFSFileSystem(InputStream stream, boolean autoClose ).
> >
> > autoClose=false by default.
>
> That looks good to me. It makes it clearer to users that autoClose isn't
> automatically done (since there are now two constructors, one which offers
> it at an option), and allows users who really want autoclose to have it
> done for them. All without breaking existing code :)
>
> Nick
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
DB2 db2jcc.jar v8.1.7
COM.ibm.db2.app.BlobInputStream.close()
public void close() {
blob = null;
}
oracle ojdbc14.jar - v10.2.0.1.0
oracle.jdbc.driver.OracleBufferedStream.close()
public void close() throws IOException {
closed = true;
}
MS msbase.jar - v2.2.0037
com.microsoft.jdbc.base.BaseBlobInputStream
// close() not implemented
MYSQL mysql-connector-java-3.1.14
com.mysql.jdbc.Blob.getBinaryStream()
public java.io.InputStream getBinaryStream() throws SQLException {
return new ByteArrayInputStream(getBinaryData());
}
HSQL (v1.8.0.7)
org.hsqldb.jdbc.jdbcBlob.getBinaryStream()
public InputStream getBinaryStream() throws SQLException {
final byte[] ldata = data;
return new ByteArrayInputStream(ldata);
}
/* ====================================================================
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.
==================================================================== */
package org.apache.poi.poifs.filesystem;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import junit.framework.TestCase;
/**
* Tests for POIFSFileSystem
*
* @author Josh Micich
*/
public final class TestPOIFSFileSystem extends TestCase {
public TestPOIFSFileSystem(String testName) {
super(testName);
}
/**
* Mock exception used to ensure correct error handling
*/
private static final class MyEx extends RuntimeException {
public MyEx() {
// no fields to initialise
}
}
/**
* Helps facilitate testing. Keeps track of whether close() was called.
* Also can throw an exception at a specific point in the stream.
*/
private static final class TestIS extends InputStream {
private final InputStream _is;
private final int _failIndex;
private int _currentIx;
private boolean _isClosed;
public TestIS(InputStream is, int failIndex) {
_is = is;
_failIndex = failIndex;
_currentIx = 0;
_isClosed = false;
}
public int read() throws IOException {
int result = _is.read();
if(result >=0) {
checkRead(1);
}
return result;
}
public int read(byte[] b, int off, int len) throws IOException {
int result = _is.read(b, off, len);
checkRead(result);
return result;
}
private void checkRead(int nBytes) {
_currentIx += nBytes;
if(_failIndex > 0 && _currentIx > _failIndex) {
throw new MyEx();
}
}
public void close() throws IOException {
_isClosed = true;
_is.close();
}
public boolean isClosed() {
return _isClosed;
}
}
/**
* Callers of POIFSFileSystem may need to use the input stream after
POIFSFileSystem has
* finished with it. During normal operation, POIFSFileSystem reads
until EOF, but some input
* streams (ones for which <tt>markSupported()</tt> returns
<code>true</code>) can continue
* to be used after this. It should always be the responsibility of
the caller to close the
* input stream.
*/
public void testNeverClose() {
TestIS testIS;
// Normal case - read until EOF and close
testIS = new TestIS(openSampleStream("13224.xls"), -1);
try {
new POIFSFileSystem(testIS);
} catch (IOException e) {
throw new RuntimeException(e);
}
assertFalse("input stream was closed", testIS.isClosed());
try {
testIS.close();
} catch (IOException e) {
throw new RuntimeException(e);
}
// intended to crash after reading 10000 bytes
testIS = new TestIS(openSampleStream("13224.xls"), 10000);
try {
new POIFSFileSystem(testIS);
fail("ex should have been thrown");
} catch (IOException e) {
throw new RuntimeException(e);
} catch (MyEx e) {
// expected
}
assertFalse("input stream was closed", testIS.isClosed());
try {
testIS.close();
} catch (IOException e) {
throw new RuntimeException(e);
}
}
private static InputStream openSampleStream(String sampleName) {
String dataDirName = System.getProperty("HSSF.testdata.path");
if(dataDirName == null) {
throw new RuntimeException("Missing system property '" +
"HSSF.testdata.path" + "'");
}
File f = new File(dataDirName + "/" + sampleName);
try {
return new FileInputStream(f);
} catch (FileNotFoundException e) {
throw new RuntimeException("Sample file '" + f.getAbsolutePath() +
"' not found");
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
