Mailing-List: contact jdo-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: jdo-dev@db.apache.org
Received-SPF: pass (asf.osuosl.org: local policy)
Message-ID: <431CB432.1030300@spree.de>
Date: Mon, 05 Sep 2005 23:10:10 +0200
From: Michael Bouschen <mbo.tech@spree.de>
Organization: Tech@Spree Engineering
User-Agent: Mozilla Thunderbird 1.0.2 (Windows/20050317)
MIME-Version: 1.0
To: jdo-dev@db.apache.org
CC: JDO Expert Group <jdo-experts-ext@Sun.COM>
Subject: Re: Testing persistent interfaces
References: <DC7D8E2D-6EA7-4B57-8459-D3B7CAF9A70E@Sun.COM>
 <431C74A4.30104@spree.de> <2FEA11F4-6C50-494D-B7E4-5C953715A498@Sun.COM>
In-Reply-To: <2FEA11F4-6C50-494D-B7E4-5C953715A498@Sun.COM>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi Craig,

looks good!

Two minors:
- The patch file misses a newline at the end of the file which causes 
problems when applying the patch. Could you please check whether file 
companyNoRelationships.xml has a newline at the end?
- Do we use upper case letters for names of constants? If yes you might 
want to change the name of factoryPropertyName and 
defaultFactoryClassName in class CompanyFactoryRegistry.

Regards Michael

> Hi Michael,
>
> Thanks for the comments. A final code review is attached. I'll send 
> the xml file updates as a separate review.
>
> ------------------------------------------------------------------------
>
>
> On Sep 5, 2005, at 9:39 AM, Michael Bouschen wrote:
>
>> Hi Craig,
>>
>> I like the idea of using the xml bean factory instance pattern 
>> instead of the static bean factory. I was not aware that I can add a 
>> bean instance to the CompanyModelReader using an API such as 
>> addSingleton instead of defining in xml. This solves the problem of 
>> attaching the current pm to the company factory which is then used by 
>> the CompanyModelReader when creating pc instances.
>>
>> A few remarks about the patch:
>> - Today CompanyFactoryRegistry.registerFactory takes the name of the 
>> company factory class as an argument. This means all the callers need 
>> to get the value of the system property 
>> jdo.tck.mapping.companyfactory and pass this to the call. Would it 
>> make sense to add another method registerFactory taking just the PM:
>>     public static final String companyFactoryClassName =
>>         System.getProperty("jdo.tck.mapping.companyfactory");
>>
>>     public static void registerFactory(PersistenceManager pm) {
>>         registerFactory(companyFactoryClassName, pm);
>>     }
>> This keeps the handling of the property 
>> jdo.tck.mapping.companyfactory local in class CompanyFactoryRegistry.
>
>
> Good.
>
>>
>> - Class CompanyModelReader should define a constant for the name of 
>> the companyFactory bean in the xml:
>>     public static final String COMPANY_FACTORY_BEAN = "companyFactory";
>> It is used in the addSingleton call in method configureFactory.
>
>
> Right.
>
>>
>> - The patch includes a changed version of derby.property where you 
>> uncommented the special Mac property for derby. I guess you are not 
>> going to check in this change, correct?
>
>
> Correct.
>
>>
>> Regards Michael
>>
>>
>>> Javadogs,
>>> Please check out the Wiki page 
>>> http://wiki.apache.org/jdo/PersistentInterfaces and this patch.
>>> I've tested the companyNoRelationships.xml but haven't updated the 
>>> other testdata files, pending a review. The idea is to replace the 
>>> testdata xml files with the factory concept so they can be used by 
>>> the standard CompletenessTest as well as the interface test.
>>> Craig
>>> =
>>> ------------------------------------------------------------------------
>>> Craig Russell
>>> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
>>> 408 276-5638 mailto:Craig.Russell@sun.com
>>> P.S. A good JDO? O, Gasp!
>>>
>>
>>
>> -- 
>> Michael Bouschen        Tech@Spree Engineering GmbH
>> mailto:mbo.tech@spree.de    http://www.tech.spree.de/
>> Tel.:++49/30/235 520-33        Buelowstr. 66            
>> Fax.:++49/30/2175 2012        D-10783 Berlin            
>>
>
> Craig Russell
>
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
>
> 408 276-5638 mailto:Craig.Russell@sun.com
>
> P.S. A good JDO? O, Gasp!
>
>
> =


-- 
Michael Bouschen		Tech@Spree Engineering GmbH
mailto:mbo.tech@spree.de	http://www.tech.spree.de/
Tel.:++49/30/235 520-33		Buelowstr. 66			
Fax.:++49/30/2175 2012		D-10783 Berlin