[OFBIZ-3748] Remove test specific code in the GenericDelegator

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Incomplete
• Affects Version/s: Trunk
• Component/s: framework
• Assignee: Unassigned
• Reporter: Bob Morley
• Created: 03/May/10 18:14
• Updated: 22/Apr/14 06:16
• Resolved: 22/Apr/14 06:16

Description

Adam – This is the results from our conversation on consistent rolling back of unit testers. We talked about moving the logic that is in the GenericDelegator that is specific to testing into a sub-class. This patch is NOT meant to be merged at this time, I wanted to put it up for review before I continue down this path ...

Here are the key pieces:

• TestGenericDelegator - test version of a generic delegator that contains the ability to record the list of database operations and then programmatically roll them back in reverse order. This was from existing code in GenericDelegator.

• TestDelegatorFactoryImpl - a new service implementation of DelegatoryFactor that will construct instances of TestGenericDelegator.

Things to consider:

• Should rollback be on the Delegator interface? I sort of field it should not be there; but I left it there for now with GenericDelegator reporting an error if it is called.

• Since there are two implementations of the DelegatorFactory there needs to be a way to determine which one to use; the way I have done this in the past is through configuration. Usually something like ... service.org.ofbiz.entity.DelegatorFactory=org.ofbiz.entity.DelegatorFactoryImpl that could (for Ofbiz) be placed in the start.properties or test.properties file. However, looking at the factory unit tester it looks like each factory should be able to determine if it is applicable based on the incoming parameters. As a result (until more discussion) I have made a choice based on the delegator name – I know this is clearly NOT the go forward method. But would like some suggestions here ... was considering a new attribute on the entityengine.xml delegator definition, but there should be some mechanism to be able to provide control over which implementation is used I would think ...

• I got an inkling that "base delegator name" may not be required anymore. This is because I no longer create the standard delegator and then "clone" to a test version. I simply instantiate the proper version right up front ... Moreover, I let the delegator / dispatcher names be as they are (not adding a random alpha-numeric suffix). Not sure about this, did not research further.

Go forward plan –

• If there are agreement on these changes and a resolution for things to consider point #2 above, I would then re-code my standalone rollback base class for unit tests to leverage this functionality. This would ensure we consistently rollback regardless of executing the test directly or through the test infrastructure.