Torque
  1. Torque
  2. TORQUE-120

Inappropriate dependencies on Torque from TorqueInstance

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3
    • Fix Version/s: 4.0-beta1
    • Component/s: Runtime
    • Labels:
      None
    • Environment:
      Mac OS X Tiger (10.4.11) / PPC

      Description

      I found a number of inappropriate invocations from org.apache.torque.TorqueInstance to static methods of org.apache.torque.Torque. I'm not sure but I suspect this bug was created during the transition from Torque to TorqueInstance.

      Index: src/java/org/apache/torque/TorqueInstance.java
      ===================================================================
      — src/java/org/apache/torque/TorqueInstance.java (revision 721316)
      +++ src/java/org/apache/torque/TorqueInstance.java (working copy)
      @@ -312,7 +312,7 @@

      // check that at least the default database has got an adapter.
      Database defaultDatabase

      • = (Database) databases.get(Torque.getDefaultDB());
        + = (Database) databases.get(getDefaultDB());
        if (defaultDatabase == null
        defaultDatabase.getAdapter() == null)
        {
        @@ -323,7 +323,7 @@
        + "."
        + Torque.DATABASE_KEY
        + "."
      • + Torque.getDefaultDB()
        + + getDefaultDB()
        + "."
        + DB.ADAPTER_KEY;
        log.error(error);
        @@ -903,7 +903,7 @@
        public Connection getConnection(String name)
        throws TorqueException
        {
      • if (!Torque.isInit())
        + if (!isInit()) { throw new TorqueException("Torque is not initialized"); }
        @@ -966,7 +966,7 @@
        String password)
        throws TorqueException
        {
        - if (!Torque.isInit())
        + if (!isInit())
        { throw new TorqueException("Torque is not initialized"); }

        Activity

        Hide
        Moriyoshi Koizumi added a comment -

        From a design perspective, I don't think the instance method should call the static method to check if it's been initialized as long as it's possible that each TorqueInstance instance has a separate class loader,

        Show
        Moriyoshi Koizumi added a comment - From a design perspective, I don't think the instance method should call the static method to check if it's been initialized as long as it's possible that each TorqueInstance instance has a separate class loader,
        Hide
        Thomas Fox added a comment -

        I agree that the calls to getDefaultDb() should be on the instance (though it does not make any functional difference, see below)
        I am not sure whether the calls to Torque.isInit() should be to the instance, for the following reason:
        The TorqueInstance needs to be a singleton, because it keeps track of the Initialisation status of torque, which depends on which Peer classes are loaded by the class loader and which are not. So, there can be only one registered Torquenstance Singleton in a Class loading scope, otherwise Torque will not work correctly (From this it follows that whether one calls Torque.something() or something() from TorqueInstance makes no functional difference). So to find the initialisation status of Torque, I'd think that one should ask the one true registered instance of TorqueInstance whether it is initialized or not, and not any TorqueInstance instance which happens to float around somewhere.
        On the other hand, if a container like Spring ensures that only one TorqueInstance exists, reverting the isInit() call to an instance call would make sense.
        It seems to me that the division between Torque and TorqueInstance is not based on any concept by now. This is the true thing that needs to be changed.

        Show
        Thomas Fox added a comment - I agree that the calls to getDefaultDb() should be on the instance (though it does not make any functional difference, see below) I am not sure whether the calls to Torque.isInit() should be to the instance, for the following reason: The TorqueInstance needs to be a singleton, because it keeps track of the Initialisation status of torque, which depends on which Peer classes are loaded by the class loader and which are not. So, there can be only one registered Torquenstance Singleton in a Class loading scope, otherwise Torque will not work correctly (From this it follows that whether one calls Torque.something() or something() from TorqueInstance makes no functional difference). So to find the initialisation status of Torque, I'd think that one should ask the one true registered instance of TorqueInstance whether it is initialized or not, and not any TorqueInstance instance which happens to float around somewhere. On the other hand, if a container like Spring ensures that only one TorqueInstance exists, reverting the isInit() call to an instance call would make sense. It seems to me that the division between Torque and TorqueInstance is not based on any concept by now. This is the true thing that needs to be changed.

          People

          • Assignee:
            Thomas Fox
            Reporter:
            Moriyoshi Koizumi
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development