[
https://issues.apache.org/jira/browse/JCR-1388?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12575765#action_12575765
]
Thomas Mueller commented on JCR-1388:
-------------------------------------
Hi,
Your new patch is better, but there are still a few issues:
You still have tabs in the source code: 2 to go ;-)
There is still a catch (Exception e) {}. catch (IOException e) {} should at
least have a remark, but it's better to log the exception (with stack trace)
TestTwoGetStreams.streamToString() expects to read everything with one
InputStreamReader.read call. This method could theoretically return a value
smaller than requested in length. You should deal with that in some way; for
example, use DataInputStream.readFully. Also, the conversion to String expects
the data is stored in the current system encoding, that may not be the case.
I wouldn't use new FileInputStream("NOTICE.txt"), I would use new
RandomInputStream(0, 10 * 1024 * 1024). You can't be sure that NOTICE.txt will
always be there and have that name.
The method testTwoGetStreams doesn't have any assertion. It doesn't check if
the data read from the repository is the same as in the files. Only calling
log.info is not a good test I believe.
DatabaseHelper doesn't have a class level javadoc.
getDatabaseResources: is the 'finally' and the boolean success required? Could
the code in 'finally' not be written in the 'catch' block?
DbInputStream.mark and reset are synchronized: I don't understand why this
needs to be synchronized in your class as well. I probably need to ask Arthur
van Hoff then ;-)
Regards,
Thomas
> Jackrabbit does not allow concurrent reads to the data store if
> copyWhenReading=false
> -------------------------------------------------------------------------------------
>
> Key: JCR-1388
> URL: https://issues.apache.org/jira/browse/JCR-1388
> Project: Jackrabbit
> Issue Type: Improvement
> Components: jackrabbit-core
> Affects Versions: 1.4
> Environment: WinXP x64, Eclipse, remote SQL Server 2005
> Reporter: Esteban Franqueiro
> Attachments: JCR-1388-datastore-concurrent-reads.2.patch,
> JCR-1388-datastore-concurrent-reads.4.patch,
> JCR-1388-datastore-concurrent-reads.patch, TestTwoGetStreams.java
>
>
> Jackrabbit does not allow concurrent reads to the data store if
> copyWhenReading=false, even if maxConnections>1.
> See JCR-1184 for a test for this problem (run it with copyWhenReading=false).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
