Mailing-List: contact torque-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Apache Torque Developers List" <torque-dev@db.apache.org>
Date: Wed, 3 May 2006 23:59:52 -0700 (PDT)
From: Thomas Fischer <tfischer@apache.org>
To: Apache Torque Developers List <torque-dev@db.apache.org>
Subject: RE: Plan to enhance Database Mapping info
In-Reply-To: <8F5843B903F59D4C8C6806BB49A39119062A6F@dukece-mail3.dukece.com>
Message-ID: <20060503225721.K39944@minotaur.apache.org>
References: <8F5843B903F59D4C8C6806BB49A39119062A6F@dukece-mail3.dukece.com>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed


On Fri, 28 Apr 2006, Greg Monroe wrote:

>
>
>> -----Original Message-----
>> From: Thomas Fischer [mailto:fischer@seitenbau.net]
>>
>> Sounds good. However, can you be more specific about how you
>> would link the Torque object to the generated model ?
>> Referencing a generated class from the runtime is a bad idea
>> in my opinion. And I do not see any other clean possibility....
>> I'd rather only generate a class which you then call to
>> load all TableMaps.
>
> I'm using a slightly modified variation of what Sun does with
> some of their factory methods that use property files to map
> names to classes.  It would work like this:
>
> Generator:
>  In the map package/directory defined by the properties:
>    Creates a <DB JavaName>MapInit.java class (ala H.)
>    Creates a torque-<DB Name>MapInit.properties file with:
>       torque.database.initclass=<DB JavaName>MapInit
>
> Notes: <DB JavaName> is generated using XML <DB Name> using
> same process as table class name are generated.  (But I'm
> not adding a JavaName attribute to DTD).
>
> Runtime:
>  DatabaseMap has initialize() method added.  This looks
>  something like:
>
>  if ( isInitialized ) {
>    return;
>  }
>  String fileName = "torque-"+getName()+"MapInit.properties";
>
> // Search the class path for DB map init property
>  InputStream readInfo =
>         this.getClass().getResourceAsStream(fileName);
>  if ( readInfo == null ) {
>    throw new TorqueException(eMsg1);
>  }
>  Properties initProps = new Properties();
>  try {
>    initProps.load(readInfo);
>  } catch ( IOException e ) {
>    throw new TorqueException(eMsg2,e);
>  }
>  String initClassName = initProps.getProperty(INIT_CLASS_KEY);
>  if ( initClassName == null ) {
>    throw new TorqueException(eMsg3);
>  }
>  try {
>    Class initClass = Class.forName(initClassName);
>    Method initMethod = initClass.getMethod("init",null);
>    initMethod.invoke(null, null);
>  } catch ( Exception e ) {
>    throw new TorqueException(eMsg4, e);
>  }
>  isInitialized = true;
>
> So the method for fully initializing a Torque DatabaseMap
> would be:
>
>  DatabaseMap map = Torque.getDatabaseMap(DBName);
>  map.initialize();
>
> The advantage to this over calling the class directly
> is that the DB Schema name does not have to be embedded.
> So code can easily be written to use different DB
> schemas (schemii?).

I have some comments here, which are not addessed by your code above.
1) So far, Torque has been supporting accessing several databases at the 
same time. Your code above only cares for the default db, not for others 
that might be around.
2) Make sure everything you do is also working if you have several schema 
files for the same database as well as several schema files for different 
databases around (the last case is linked to comment 1)
3) When this was discussed last year, some people cared about startup 
time, which could be long for large schema files. Make sure one can still 
use the old way to initialize the database Map. Probably the best way to 
do this would be to add a configuration property.
4) Make sure that the runtime still works without any generated classes 
being around.

>> - Making Peer's getTableMap public is ok with me.
>
>> - I am not convinced about the OM classes' getTableMap()
>> method. Why can't you use the Peer's getTableMap ?
>> Convenience methods are added quickly, but removing them
>> is nearly impossible.
>>
>> - I am also not convinced about the Map's getOmClass()
>> method, for the same reason.
>
> You're right. Especially when you add in Thomas V.'s
> comments about Managers.  A re-write of the coding goal
> would be:
>
> Make sure that given any Torque OM, Peer, Manager, Map,
> etc. class, even if passed in as a "Base" object, you
> can find the others in some way.  This should:
>
> - add a minimum of convenience methods.
> - be balanced against, having to have application code
>  dynamically create classes, etc.  E.g., Don't just
>  pass back a class name string.

Hm, not convinced about that. I have another belief in this matter, which 
is "No class should know about any other class, unless it has to".

Why should the database map care about how the database is accessed (i.e. 
the peer classes) and the database objects ? It is just a map of the 
structure of the database and has no need to access teh database in any 
way.

Also, because of Torque's support of inheritance in data objects, there is 
no 1:1 mapping between database tables and the database object classes. 
Several database object classes can live in one database table. So there 
is no 1:1 mapping between peer / map classes on the one side and database 
objects on the other. A data object class can only have one peer, but a 
peer can be used by more than one data object class.

>
>> I'm definitely -1 on adding a getPeer() method to the OM class. 
>> Remember that all Peer methods are static:
>
> .. some excellent discussion on the evils of static
> methods removed...
>
> All good points, I'll have to put some more thought into
> how to meet the above goal. (unless vetoed.)  Just to
> float a top of mind idea...
>
> Should there be some set of instance methods in the
> *Peer classes that match similar static methods?

That would clutter the class too much in my opinion.

>
> Or maybe there should be a generated *PeerInstance class
> (like TorqueInstance) with matching methods?

That would be better. So one still has the convenience of the static
methods and get the benefits of object inheritance.

>
> I know it's probably blasphamey, but just look at
> some of the things stuff like LargeSelect and other
> code has to do to deal with the fact that Peer class
> methods are all static.
>
> Thoughts?
>
>>
>>>
>>> 3) Fix some misc Mapping issues like TableMap using a Hash table
>>> which looses the XML ordering of columns. (Already done...)
>>
>> How ist this done? Using commons-collection's ListOrderedMap?
>
> I did it using all standard J2SE (1.4+) classes.  E.g.: the fields
> were defined as Map and created using stuff like:
>
> Collections.synchronizedMap(new LinkedHashMap())

As far as I understand it, ListOrderedMap is equivalent to LinkedHashMap.
However, ListOrderedMap works with java 1.3, whereas LinkedHashMap does 
not. So far the runtime has been java 1.3 compatible, so please use 
ListOrderedMap.

>
> Re-reading the docs on this, reminded me to check some of the uses
> of this for synchronization statements. Does ListOrderedMap handle
> synchronization better?  The docs I saw didn't indicate it did.

As far as I know, all of the "modern" map implementations need to be 
wrapped in a synchronizedMap to be synchronized.

>
>>> adding
>>> a JavaName property to the TableMap so that the Key used in the
>>> DatabaseMap can be gotten from the object;
>>
>> Not so sure about this one. Why not use TableMap's
>> getDatabaseMap() method
>> and ask the Database map for its name ?
>
> Hmm, that might work. The underlying need is to be able to find
> the JavaName values for tables and columns created at Generate
> time from in during runtime execution. This is a Map responsibility
> so I thought I'd put it there. FWIW, my specific case is to used
> this in XML generation since the Torque generated DTD uses
> JavaNames as elementa and attribute identifiers.

Sorry, I misunderstood you. I thought you were proposing to create a 
method which retrieves the name of the database. The javaName property 
sounds good.

>
>>
>>> and maybe redo-ing the TableMap.vm to use Column.set..
>>
>> I'm not sure what you mean by this. Can you be more specific please ?
>
> The generated template currently creates the TableMap classes with
> calls to various addColumn(<bunch of params>). In the supporting
> TableMap class, there are a series of methods like: addColumn(param1),
> addColumn(param1,param2) and the like. I'm pretty sure this represents
> a history of how parameters were added in a way to ensure backwards
> compatiblity.
>
> So to add new information to be saved with a column and maintain
> compatiblity, you end up converting a bunch of old methods "chain"
> methods that call the new method with the additional parameter. It's
> a pain and causes major method creep.
>
> The quick fix to this is to have the existing addColumn methods
> return the column object created.  Then adding a new parameter
> could be done by adding set/get methods to ColumnMap and changing
> the template to call them after the bulk addColumn is done.
>
> Another way would be to totally re-write the Template to just
> use Set/Get methods and drop using the old add methods. Then mark
> the add methods as deprecated.

Now I understand what you want to do. I like the second way better 
(that is the one you proposed at first, as I understand it). This will 
also ease reading of MapBuilder.vm.

     Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: torque-dev-unsubscribe@db.apache.org
For additional commands, e-mail: torque-dev-help@db.apache.org