Hi Thomas. > - please use spaces, not tabs (Jackrabbit consistently uses spaces) > - please don't use import ...* > - byte b[] > byte[] b > - public is not required in interfaces > - return does not require (..) > > To fix such problems, I use Checkstyle. If you like I can post the > configuration I use.
Sure, please do that. How do you run checkstyle? > Garbage collection: I like make garbage collection implementation > independent; is it OK for you if > I remove those cases? Sorry, which cases? > AbstractGarbageCollector and AbstractDataStore are not required then (anyway > they are small). The AbstractDataStore class has members that are common to both data stores, and are implementation independent. It can be removed, but we'll en up with a lot of duplicated code in both subclasses. On the other hand, the AbstractGarbageCollector is easier, the only difference is that FileGarbageCollector overrides touchBinaryProperty(), which is different. I guess you can merge both methods in one that calls back to the data store impl to update the record's time, but the way I see it now is probably more complicated. Maybe you can tell me your idea on how to do this, so I can understand better. > It would be great if the database data store would automatically re-connect > if the connection was > lost (important for MySQL). To achieve this, > org.apache.jackrabbit.core.persistence.bundle.util.ConnectionRecoveryManager > can be used. This is our point of most concern. The problem is that if we follow in the data store the pattern used in the PMs (only one connection), it won't be possible to have multiple clients concurrently uploading binaries. That's why we did it that way. Still, we don't think that adding reconnection is going to be a problem, but what concerns us is that doing so forces the one connection pattern. In our app we subclass the DatabaseDataStore class and override the getConection(boolean) method to use a connection pool provided by the DataDirect JDBC drivers. > It's quite tricky that DatabaseRecord extends FilterInputStream... However > I'm not sure if it is > required, is it not possible to just wrap the database BLOB object? Yes, not only tricky but also very ugly. We had to it like that because the way our application handles the binaries. But alas, we fixed that already. I'll send the changes as soon as I figure out how to make a decent patch. > I don't think that FileDataStoreConstants is required. You're right, their just a leftover. Although I do think that the buffer size in FileDataStore should be added as a constant, and maybe increased to 16 or 64KB > Those are just my view, and I'm open to discuss them of course. > If you have time to change it yourself please go ahead. Otherwise I will do > it and post the patch > here before I commit it - but it will take a few more days. Do you have a patch against trunk? If you do, please upload it, so I can update it with our latest changes and test cases. After that I'll have no problem in doing these. Looking at the ConnectionRecoveryManager, I think that its private methods should be delcared as protected instead of private, because it should be possible to override them. Regards, Esteban Franqueiro [EMAIL PROTECTED] Notice: This email message, together with any attachments, may contain information of BEA Systems, Inc., its subsidiaries and affiliated entities, that may be confidential, proprietary, copyrighted and/or legally privileged, and is intended solely for the use of the individual or entity named in this message. If you are not the intended recipient, and have received this message in error, please immediately return this by email and then delete it.
