Jackrabbit Content Repository
  1. Jackrabbit Content Repository
  2. JCR-1525

Jackrabbit depends on Oracle driver for BLOB support in Oracle versions previous than 10.2

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0-beta5
    • Fix Version/s: 2.0-beta6
    • Component/s: jackrabbit-core
    • Labels:
      None

      Description

      In Oracle versions previous to 10.2, Jackrabbit explicitly uses a class from the Oracle driver to provide BLOB support (see OracleFileSystem.init()). This special handling is no longer necesary for Oracle 10.2+, so we should provide a new implementation. As discussed on the list, we can create a new class for Oracle 10.2+, make it inherit from DbFileSystem, and override the createSchema(), and table space related methods, which are the ones that need special handling. Furthermore, we could refactor the current OracleFileSystem and break it into two clases, one of them to keep the current behavior and a new one to keep the common code (which we could rename to OracleBaseFileSystem or similar, to maintain compatiblity with code that uses OracleFileSystem for versions previous to 10.2). Then we make the Oracle10FileSystem inherit from the latter.

      1. JCR-1525.patch
        3 kB
        Martijn Hendriks
      2. JCR-1525.patch
        51 kB
        Jared Roberts
      3. JCR-1525.patch
        27 kB
        Esteban Franqueiro

        Activity

        Hide
        Esteban Franqueiro added a comment -

        I'm working on this refactoring and I ended up with an empty Oracle10R2FileSystem. From a code point of view this class should be removed, but from a user's perspective, I think its more clear to have a class with a more accurate name.
        The other possibility is to rename the already mentioned OracleBaseFaileSystem as Oracle10R2FileSystem, and make OracleFileSystem inheriit from it. I don't like this either.
        WDYYT?
        Regards,

        Show
        Esteban Franqueiro added a comment - I'm working on this refactoring and I ended up with an empty Oracle10R2FileSystem. From a code point of view this class should be removed, but from a user's perspective, I think its more clear to have a class with a more accurate name. The other possibility is to rename the already mentioned OracleBaseFaileSystem as Oracle10R2FileSystem, and make OracleFileSystem inheriit from it. I don't like this either. WDYYT? Regards,
        Hide
        Esteban Franqueiro added a comment -

        Is it worth doing this work for the Oracle DB Persistence Managers (not the bundle PMs) too?
        The bundle PMs for Oracle already support 10.2+, but there's a name mismatch because there's an Oracle9PM, and OraclePM is for 10+.

        Show
        Esteban Franqueiro added a comment - Is it worth doing this work for the Oracle DB Persistence Managers (not the bundle PMs) too? The bundle PMs for Oracle already support 10.2+, but there's a name mismatch because there's an Oracle9PM, and OraclePM is for 10+.
        Hide
        Thomas Mueller added a comment -

        Hi,
        I don't know how much it is used. I would deprecate those persistence managers soon, as they are much slower then the bundle persistence managers. So in my view it doesn't make sense to work on non-bundle persistence managers any more.
        Regards,
        Thomas

        Show
        Thomas Mueller added a comment - Hi, I don't know how much it is used. I would deprecate those persistence managers soon, as they are much slower then the bundle persistence managers. So in my view it doesn't make sense to work on non-bundle persistence managers any more. Regards, Thomas
        Hide
        Esteban Franqueiro added a comment -

        This patch applies the suggested fix. We should decide what to do regarding what I mentioned in my previous comments.
        As suggested by Thomas, this doesn't include changes for the older PMs.
        I tested it on Oracle 10.2.0.3.0 and all tests passed.

        Show
        Esteban Franqueiro added a comment - This patch applies the suggested fix. We should decide what to do regarding what I mentioned in my previous comments. As suggested by Thomas, this doesn't include changes for the older PMs. I tested it on Oracle 10.2.0.3.0 and all tests passed.
        Hide
        Esteban Franqueiro added a comment -

        Hi all.
        What should we do regarding this issue? I've been running this code for some time now and haven't had any issues. Should we apply it to trunk? Or should we wait?
        Regards,

        Show
        Esteban Franqueiro added a comment - Hi all. What should we do regarding this issue? I've been running this code for some time now and haven't had any issues. Should we apply it to trunk? Or should we wait? Regards,
        Hide
        Martijn Hendriks added a comment -

        We've been running your patch also and encountered no problems. It looks fine. +1 for committing the patch.

        Can we solve the naming inconsistency now that the trunk is on 2.0-SNAPSHOT?

        Show
        Martijn Hendriks added a comment - We've been running your patch also and encountered no problems. It looks fine. +1 for committing the patch. Can we solve the naming inconsistency now that the trunk is on 2.0-SNAPSHOT?
        Hide
        Thomas Mueller added a comment -

        +1 for the patch except:

        The class names should match the names used in the bundle PMs. Currently the patch uses:
        Oracle10R2FileSystem (for Oracle 10 and newer)
        OracleFileSystem (for Oracle 9)

        The class names should be:
        OracleFileSystem (for Oracle 10 and newer)
        Oracle9FileSystem (for Oracle 9)

        Show
        Thomas Mueller added a comment - +1 for the patch except: The class names should match the names used in the bundle PMs. Currently the patch uses: Oracle10R2FileSystem (for Oracle 10 and newer) OracleFileSystem (for Oracle 9) The class names should be: OracleFileSystem (for Oracle 10 and newer) Oracle9FileSystem (for Oracle 9)
        Hide
        Jared Roberts added a comment -

        I took the liberty of updating the patch with a couple of modifications. The first is to rename the classes to align with the conventions that Thomas mentioned. Instead of Oracle9FileSystem, I chose Oracle10R1FileSystem, though. It seemed to make more sense, because that's technically the last version it supports. The second change I made was to remove the user/password initialization in the OracleBaseFileSystem constructor. Initializing them to empty strings causes problems when a JNDI data source is being used. They should be left null unless explicitly configured.

        Show
        Jared Roberts added a comment - I took the liberty of updating the patch with a couple of modifications. The first is to rename the classes to align with the conventions that Thomas mentioned. Instead of Oracle9FileSystem, I chose Oracle10R1FileSystem, though. It seemed to make more sense, because that's technically the last version it supports. The second change I made was to remove the user/password initialization in the OracleBaseFileSystem constructor. Initializing them to empty strings causes problems when a JNDI data source is being used. They should be left null unless explicitly configured.
        Hide
        Martijn Hendriks added a comment -

        Attached a new version of the proposed patch because the previous ones do not match anymore after the JCR-1456 merge.

        Show
        Martijn Hendriks added a comment - Attached a new version of the proposed patch because the previous ones do not match anymore after the JCR-1456 merge.
        Hide
        Martijn Hendriks added a comment -

        Fixed in revision: 887881.

        Please note that this change is not backwards compatible: the OracleFileSystem class now does not use special blob handling required for 10R1 and earlier. If you need that, use the Oracle9FileSystem. We migh want to add this in the RELEASE-NOTES.txt.

        Show
        Martijn Hendriks added a comment - Fixed in revision: 887881. Please note that this change is not backwards compatible: the OracleFileSystem class now does not use special blob handling required for 10R1 and earlier. If you need that, use the Oracle9FileSystem. We migh want to add this in the RELEASE-NOTES.txt.
        Hide
        Sébastien Launay added a comment -

        > Please note that this change is not backwards compatible: the OracleFileSystem class now does not use special blob handling required for 10R1 and earlier. If you need that, use the Oracle9FileSystem. We migh want to add this in the RELEASE-NOTES.txt.

        Done in revision 888290.

        Show
        Sébastien Launay added a comment - > Please note that this change is not backwards compatible: the OracleFileSystem class now does not use special blob handling required for 10R1 and earlier. If you need that, use the Oracle9FileSystem. We migh want to add this in the RELEASE-NOTES.txt. Done in revision 888290.

          People

          • Assignee:
            Unassigned
            Reporter:
            Esteban Franqueiro
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development