[hibernate-issues] [Hibernate-JIRA] Created: (HBX-836) Delegating ReverseEngineeringStrategy problems

[Eric Kershner (JIRA)]

Delegating ReverseEngineeringStrategy problems
----------------------------------------------

         Key: HBX-836
         URL: http://opensource.atlassian.com/projects/hibernate/browse/HBX-836
     Project: Hibernate Tools
        Type: Bug

  Components: reverse-engineer  
    Versions: 3.2beta8    
 Environment: Hibernate 3.2 GA, Oracle 10g
    Reporter: Eric Kershner


The delegate pattern of the DelegatingReverseEngineeringStrategy is broken with respect to several methods, when a reveng.xml file is used. This is a bear to explain in code, but here is a way to duplicate this type of error:

DDL:
  CREATE TABLE "TEST"."AA_ELIGIBILITY" 
   (	"AA_ELIGIBILITY_ID" NUMBER NOT NULL ENABLE, 
	"AA_REVIEW_ID" NUMBER NOT NULL ENABLE, 
	"TEST_FIELD1" NUMBER, 
	 CONSTRAINT "AA_ELIGIBILITY_P" PRIMARY KEY ("AA_ELIGIBILITY_ID") ENABLE, 
	 CONSTRAINT "AA_RVW_F01_AA_ELIGIBILITY" FOREIGN KEY ("AA_REVIEW_ID")
	  REFERENCES "TEST"."AA_REVIEW" ("AA_REVIEW_ID") ON DELETE CASCADE ENABLE
   ) ;

  CREATE TABLE "TEST"."AA_REVIEW" 
   (	"AA_REVIEW_ID" NUMBER NOT NULL ENABLE, 
	"ADPT_PRNT_LGLLY_RSPNSBLE_FLAG" NUMBER, 
	"TEST_FIELD2" NUMBER, 
	 CONSTRAINT "AA_REVIEW_P" PRIMARY KEY ("AA_REVIEW_ID") ENABLE,
   ) ;

hibernate.reveng.xml snippet:
	<table name="AA_ELIGIBILITY" class="org.whomever.framework.model.AthleteAssistanceEligibility">
	</table>
	<table name="AA_REVIEW" class="org.whomever.framework.model.AthleteAssistanceReview">
	</table>

Now, when the DelegatingReverseEngineeringStrategy anonymous inner class of the OverrideRepository executes the tableToClassName method, it will correctly return AthleteAssistanceEligibility for the first table and AthleteAssistanceReview for the second. However, when the foreignKeyToEntityName in the aforementioned DelegatingReverseEngineeringStrategy is executed, it passes execution to the DefaultReverseEngineeringStrategy (since the foreign-key element was not explicitly defined), and you end up with foreign key entity names like aaEligibility and aaReview because it uses the DefaultReverseEngineeringStrategy's tableToClassName method instead of the OverrideRepository's anonymous DelegatingReverseEngineeringStrategy's tableToClassName. I'm sure that we would rather have that return athleteAssistanceEligibility and athleteAssistanceReview for the foreign key entity names.

Right now, the generated POJO looks like:
public class AthleteAssistanceEligibility {
    private AthleteAssistanceReview aaReview;
    public AthleteAssistanceReview getAaReview() {
        return this.aaReview;
    }
    public void setAaReview(AthleteAssistanceReview aaReview) {
        this.aaReview = aaReview;
    }
     ...

But a preferable POJO would be:
public class AthleteAssistanceEligibility {
    private AthleteAssistanceReview athleteAssistanceReview;
    public AthleteAssistanceReview getAthleteAssistanceReview() {
        return this.aaReview;
    }
    public void setAthleteAssistanceReview(AthleteAssistanceReview athleteAssistanceReview) {
        this.aaReview = aaReview;
    }
     ...

Now, you may say, "Well, just specify the foreign-key in the reveng.xml file with appropriate set property and many-to-one property names." While that does fix the problem intermediately, unfortunately I have about 400 tables to reverse engineer! That would mean explicitly specifying around 650 foreign key constraints in the reveng.xml file. Then you might say, "Wow! That's a lot of tables! You are better off overriding the foreignKeyToEntityName in a custom DelegatingReverseEngineeringStrategy." Well, that's pretty much what I did in order for the foreignKeyToEntityName to read the tableToClassName from the anonymous inner class of the OverrideReposiitory instead of the DefaultReverseEngineeringStrategy file. But that breaks the delegate contract since I essentially call super.foreignKeytoEntityName and then throw away the result. Here is the foreignKeyToEntityName method of my custom DelegatingReverseEngineeringStrategy:
	public String foreignKeyToEntityName(String keyname, TableIdentifier fromTable, List fromColumnNames,
			TableIdentifier referencedTable, List referencedColumnNames, boolean uniqueReference) {
		// Have to call this since some foreign keys are explicity defined in the reveng.xml file
		String propertyName = super.foreignKeyToEntityName(keyname, fromTable, fromColumnNames, referencedTable,
				referencedColumnNames, uniqueReference);
		// Duplicate code from DefaultReverseEngineeringStrategy (simplified)
		String defaultReverseEngineeringPropertyName = Introspector.decapitalize(
				ReverseEngineeringStrategyUtil.toUpperCamelCase(referencedTable.getName()));
		// When the DefaultReverseEngineeringStrategy generated the name...
		if (propertyName != null && propertyName.equals(defaultReverseEngineeringPropertyName)) {
			//...
			//... I do some other stuff here to manipulate non-unique foreign keys...
			//...

			// Run the same code as DefaultreverseEngineeringStrategy, except use the
			// OverrideRepository's DelegatingReverseEngineeringStrategy (or this class') tableToClassName! Eww!
			if (uniqueReference) {
				propertyName = Introspector.decapitalize(StringHelper.unqualify(tableToClassName(referencedTable)));
			}
		}
		return propertyName;
	}

You can see how ugly that code is, in that if the foreignKeyToEntityName method in the DefaultReverseEngineeringStrategy changes (an unlikely change, but possible none the less), this code breaks. But currently, this is the only way to generate the proper decapitalized class name of the referenced table for the foreign key entity name where your reveng.xml file has both custom classes for tables and explicitly described foreign keys. Note that this is not the only method that exhibits this behavior, as foreignKeyToCollectionName has the same problem.

I have not had much time to think about a possible solution, but off of the top of my head, you can replace the delegate pattern with a chain pattern, or make the delegates aware of the delegators (which, of course, probably breaks the delegate contract).

Another side note (I can't believe you read this far already): the OverrideRepository's DelegatingReverseEngineeringStrategy's excludeTable method never calls super.excludeTable, so if you are using Oracle and a reveng.xml file, you still must explicitly exclude the BIN$ tables. The tableToMetaAttributes, columnToMetaAttributes and excludeColumn exhibit the same behavior, but since the DefaultReverseEngineeringStrategy returns false or null for those methods by default, it's no worry. Just make sure to make a note in the DefaultReverseEngineeringStrategy javadocs/comments for those methods that if there are changes to them that the DelegatingReverseEngineeringStrategy in the OverrideRepository must be verified.

Sorry for the super long posting, perhaps I should have attached the code as test cases, but I'm relatively new to JIRA. This all came about because I'm trying to get some work done on HBX-447 (generating bidirectional collection management code in the POJOs). It is near impossible to do if the setter on the entity side is not able to be calculated.


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://opensource.atlassian.com/projects/hibernate/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira