dhis2-devs team mailing list archive

Thread
Date
Re: [Branch ~dhis2-devs-core/dhis2/trunk] Rev 1018: Fixed the database internationalisation. Applying translations through AOP.

To: DHIS 2 developers <dhis2-devs@xxxxxxxxxxxxxxxxxxx>
From: Lars Helge Øverland <larshelge@xxxxxxxxx>
Date: Tue, 10 Nov 2009 12:50:48 +0100
In-reply-to: <20091110114712.22283.10290.launchpad@loganberry.canonical.com>
This was a tricky one. Previously a Hibernate PostLoadEventListener was used
to inspect and translate objects provided by Hibernate. The event listener
was previously registered with the HibernateConfigurationProvider with a
MethodInvokingFactoryBean. We now use Spring's LocalSessionFactoryBean,
which makes this approach unusable since the LocalSessionFactoryBean gets
initialized before the MethodInvokingFactoryBean gets to do its stuff (since
there are no dependencies in that direction). We now have a standard AOP
approach where get-methods returning i18n-enabled objects are matched and
advised by the I18nTranslationInterceptor in dhis-i18n-db. I have also
tested and made the code in I18nService and TranslationService more compact,
although there was nothing wrong in there.

On Tue, Nov 10, 2009 at 12:47 PM, <noreply@xxxxxxxxxxxxx> wrote:

> ------------------------------------------------------------
> revno: 1018
> committer: Lars Helge Oeverland larshelge@xxxxxxxxx
> branch nick: trunk
> timestamp: Tue 2009-11-10 12:45:58 +0100
> message:
>  Fixed the database internationalisation. Applying translations through
> AOP.
> removed:
>
>  dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/hibernate/HibernateI18nPostLoadEventListener.java
> added:
>
>  dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/interceptor/I18nTranslationInterceptor.java
> modified:
>
>  dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitService.java
>
>  dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/DefaultI18nService.java
>  dhis-2/dhis-i18n/dhis-i18n-db/src/main/resources/META-INF/dhis/beans.xml
>
>  dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/beans.xml
>
>  dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataelement/DataElementServiceTest.java
>
>  dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/indicator/IndicatorServiceTest.java
>
>
> --
> lp:dhis2
> https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk<https://code.launchpad.net/%7Edhis2-devs-core/dhis2/trunk>
>
> Your team DHIS 2 developers is subscribed to branch lp:dhis2.
> To unsubscribe from this branch go to
> https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription<https://code.launchpad.net/%7Edhis2-devs-core/dhis2/trunk/+edit-subscription>
> .
>
> === modified file
> 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitService.java'
> ---
> dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitService.java
>   2009-10-01 09:24:46 +0000
> +++
> dhis-2/dhis-api/src/main/java/org/hisp/dhis/organisationunit/OrganisationUnitService.java
>   2009-11-10 11:45:58 +0000
> @@ -217,6 +217,12 @@
>      */
>     int getNumberOfOrganisationalLevels();
>
> +    /**
> +     * Set parent of OrganisationUnit.
> +     *
> +     * @param organisationUnitId the identifier of the child
> OrganisationUnit.
> +     * @param parentId the identifier of the parent OrganisationUnit.
> +     */
>     void updateOrganisationUnitParent( int organisationUnitId, int parentId
> );
>
>     //
> -------------------------------------------------------------------------
>
> === modified file
> 'dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/DefaultI18nService.java'
> ---
> dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/DefaultI18nService.java
>      2009-11-10 01:44:26 +0000
> +++
> dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/DefaultI18nService.java
>      2009-11-10 11:45:58 +0000
> @@ -30,8 +30,8 @@
>  import static org.hisp.dhis.system.util.ReflectionUtils.getClassName;
>  import static org.hisp.dhis.system.util.ReflectionUtils.getId;
>  import static org.hisp.dhis.system.util.ReflectionUtils.getProperty;
> +import static org.hisp.dhis.system.util.ReflectionUtils.isCollection;
>  import static org.hisp.dhis.system.util.ReflectionUtils.setProperty;
> -import static org.hisp.dhis.system.util.ReflectionUtils.isCollection;
>
>  import java.util.ArrayList;
>  import java.util.Collection;
> @@ -143,11 +143,16 @@
>
>     private void internationaliseCollection( Collection<?> intObjects )
>     {
> +        if ( intObjects == null || intObjects.size() == 0 )
> +        {
> +            return;
> +        }
> +
>         I18nObject i18nObject = isI18nObject( intObjects.iterator().next()
> );
>
>         Locale locale = localeManager.getCurrentLocale();
>
> -        if ( i18nObject != null && locale != null && intObjects != null )
> +        if ( i18nObject != null && locale != null )
>         {
>             Collection<Translation> allTranslations =
> translationService.getTranslations( i18nObject.getClassName(), locale );
>
> @@ -443,7 +448,7 @@
>                 }
>             }
>
> -            log.warn( "Object not enabled for i18n: " + object );
> +            log.debug( "Object not enabled for i18n: " + object );
>         }
>
>         return null;
>
> === removed file
> 'dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/hibernate/HibernateI18nPostLoadEventListener.java'
> ---
> dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/hibernate/HibernateI18nPostLoadEventListener.java
>    2009-03-09 22:33:48 +0000
> +++
> dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/hibernate/HibernateI18nPostLoadEventListener.java
>    1970-01-01 00:00:00 +0000
> @@ -1,50 +0,0 @@
> -package org.hisp.dhis.i18n.hibernate;
> -
> -/*
> - * Copyright (c) 2004-2007, University of Oslo
> - * All rights reserved.
> - *
> - * Redistribution and use in source and binary forms, with or without
> - * modification, are permitted provided that the following conditions are
> met:
> - * * Redistributions of source code must retain the above copyright
> notice, this
> - *   list of conditions and the following disclaimer.
> - * * Redistributions in binary form must reproduce the above copyright
> notice,
> - *   this list of conditions and the following disclaimer in the
> documentation
> - *   and/or other materials provided with the distribution.
> - * * Neither the name of the HISP project nor the names of its
> contributors may
> - *   be used to endorse or promote products derived from this software
> without
> - *   specific prior written permission.
> - *
> - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> IS" AND
> - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> IMPLIED
> - * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> - * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> LIABLE FOR
> - * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> DAMAGES
> - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> SERVICES;
> - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON
> - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> OF THIS
> - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> - */
> -
> -import org.hibernate.event.PostLoadEventListener;
> -import org.hibernate.event.PostLoadEvent;
> -import org.hisp.dhis.i18n.I18nService;
> -
> -/**
> - * @author Oyvind Brucker
> - */
> -public class HibernateI18nPostLoadEventListener implements
> PostLoadEventListener
> -{
> -    private I18nService i18nService;
> -
> -    public void setI18nService( I18nService i18nService )
> -    {
> -        this.i18nService = i18nService;
> -    }
> -
> -    public void onPostLoad( PostLoadEvent postLoadEvent )
> -    {
> -        i18nService.internationalise( postLoadEvent.getEntity() );
> -    }
> -}
>
> === added file
> 'dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/interceptor/I18nTranslationInterceptor.java'
> ---
> dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/interceptor/I18nTranslationInterceptor.java
>  1970-01-01 00:00:00 +0000
> +++
> dhis-2/dhis-i18n/dhis-i18n-db/src/main/java/org/hisp/dhis/i18n/interceptor/I18nTranslationInterceptor.java
>  2009-11-10 11:45:58 +0000
> @@ -0,0 +1,53 @@
> +package org.hisp.dhis.i18n.interceptor;
> +
> +/*
> + * Copyright (c) 2004-2007, University of Oslo
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are
> met:
> + * * Redistributions of source code must retain the above copyright
> notice, this
> + *   list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> notice,
> + *   this list of conditions and the following disclaimer in the
> documentation
> + *   and/or other materials provided with the distribution.
> + * * Neither the name of the HISP project nor the names of its
> contributors may
> + *   be used to endorse or promote products derived from this software
> without
> + *   specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> IS" AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> LIABLE FOR
> + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
> SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON
> + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> OF THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +import org.hisp.dhis.common.IdentifiableObject;
> +import org.hisp.dhis.i18n.I18nService;
> +import org.hisp.dhis.system.util.TimeUtils;
> +
> +import org.aspectj.lang.JoinPoint;
> +
> +/**
> + * @author Lars Helge Overland
> + * @version $Id$
> + */
> +public class I18nTranslationInterceptor
> +{
> +    private I18nService i18nService;
> +
> +    public void setI18nService( I18nService i18nService )
> +    {
> +        this.i18nService = i18nService;
> +    }
> +
> +    public void intercept( JoinPoint joinPoint, Object object )
> +    {
> +        i18nService.internationalise( object );
> +    }
> +}
>
> === modified file
> 'dhis-2/dhis-i18n/dhis-i18n-db/src/main/resources/META-INF/dhis/beans.xml'
> ---
> dhis-2/dhis-i18n/dhis-i18n-db/src/main/resources/META-INF/dhis/beans.xml
>  2009-11-09 22:14:51 +0000
> +++
> dhis-2/dhis-i18n/dhis-i18n-db/src/main/resources/META-INF/dhis/beans.xml
>  2009-11-10 11:45:58 +0000
> @@ -8,33 +8,6 @@
>     <property name="systemLocale" value="en_GB"/>
>   </bean>
>
> -  <!-- Event listener definitions -->
> -  <!--
> -  <bean
> id="org.hisp.dhis.i18n.hibernate.HibernateI18nPostLoadEventListener"
> -
>  class="org.hisp.dhis.i18n.hibernate.HibernateI18nPostLoadEventListener">
> -    <property name="i18nService">
> -      <ref bean="org.hisp.dhis.i18n.I18nService"/>
> -    </property>
> -  </bean>
> -  -->
> -  <!-- Event listener registrations -->
> -  <!--
> -  <bean id="addI18nPostLoadEventListener"
> -
>  class="org.springframework.beans.factory.config.MethodInvokingFactoryBean">
> -    <property name="targetObject">
> -      <ref bean="org.hisp.dhis.hibernate.HibernateConfigurationProvider"/>
> -    </property>
> -    <property name="targetMethod">
> -      <value>setEventListener</value>
> -    </property>
> -    <property name="arguments">
> -      <list>
> -        <value>post-load</value>
> -        <ref
> local="org.hisp.dhis.i18n.hibernate.HibernateI18nPostLoadEventListener"/>
> -      </list>
> -    </property>
> -  </bean>
> -  -->
>   <!-- I18n object definitions -->
>
>   <bean id="I18nDataElement"
> @@ -179,16 +152,6 @@
>     </property>
>   </bean>
>
> -  <bean id="I18nDataElementCategoryOption"
> -    class="org.hisp.dhis.i18n.I18nObject">
> -    <property name="className" value="DataElementCategoryOption"/>
> -    <property name="propertyNames">
> -      <list>
> -        <value>name</value>
> -      </list>
> -    </property>
> -  </bean>
> -
>   <!-- I18nService -->
>
>   <bean id="org.hisp.dhis.i18n.I18nService"
> @@ -214,7 +177,6 @@
>         <ref bean="I18nOrganisationUnitGroupSet"/>
>         <ref bean="I18nDataSet"/>
>         <ref bean="I18nDataElementCategory"/>
> -        <ref bean="I18nDataElementCategoryOption"/>
>       </list>
>     </property>
>   </bean>
> @@ -240,11 +202,11 @@
>       ref="org.hisp.dhis.i18n.I18nService"/>
>   </bean>
>   -->
> -  <!--
> +
>   <bean id="i18nTranslationInterceptor"
> -    class="org.hisp.dhis.i18n.interceptor.I18nObjectInterceptor">
> +    class="org.hisp.dhis.i18n.interceptor.I18nTranslationInterceptor">
>     <property name="i18nService"
>       ref="org.hisp.dhis.i18n.I18nService"/>
>   </bean>
> -  -->
> +
>  </beans>
>
> === modified file
> 'dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/beans.xml'
> ---
> dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/beans.xml
>   2009-11-07 14:09:00 +0000
> +++
> dhis-2/dhis-services/dhis-service-core/src/main/resources/META-INF/dhis/beans.xml
>   2009-11-10 11:45:58 +0000
> @@ -602,6 +602,15 @@
>       <aop:around pointcut="execution( *
> org.hisp.dhis.validation.ValidationRuleService.validate(..) )"
> method="intercept"/>
>     </aop:aspect>
>
> +    <aop:aspect ref="i18nTranslationInterceptor">
> +      <aop:after-returning pointcut="execution( *
> org.hisp.dhis.dataelement.DataElementService.get*(..) )" method="intercept"
> returning="object"/>
> +      <aop:after-returning pointcut="execution( *
> org.hisp.dhis.dataelement.DataElementCategoryService.get*(..) )"
> method="intercept" returning="object"/>
> +      <aop:after-returning pointcut="execution( *
> org.hisp.dhis.indicator.IndicatorService.get*(..) )" method="intercept"
> returning="object"/>
> +      <aop:after-returning pointcut="execution( *
> org.hisp.dhis.datadictionary.DataDictionaryService.get*(..) )"
> method="intercept" returning="object"/>
> +      <aop:after-returning pointcut="execution( *
> org.hisp.dhis.dataset.DataSetService.get*(..) )" method="intercept"
> returning="object"/>
> +      <aop:after-returning pointcut="execution( *
> org.hisp.dhis.organisationunit.OrganisationUnitService.get*(..) )"
> method="intercept" returning="object"/>
> +    </aop:aspect>
> +
>   </aop:config>
>
>  </beans>
>
> === modified file
> 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataelement/DataElementServiceTest.java'
> ---
> dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataelement/DataElementServiceTest.java
>  2009-11-07 14:09:00 +0000
> +++
> dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/dataelement/DataElementServiceTest.java
>  2009-11-10 11:45:58 +0000
> @@ -90,23 +90,11 @@
>         DataElement dataElementA = createDataElement( 'A' );
>         DataElement dataElementB = createDataElement( 'B' );
>         DataElement dataElementC = createDataElement( 'C' );
> -        DataElement dataElementD = createDataElement( 'A' );
>
>         int idA = dataElementService.addDataElement( dataElementA );
>         int idB = dataElementService.addDataElement( dataElementB );
>         int idC = dataElementService.addDataElement( dataElementC );
>
> -        try
> -        {
> -            // Should give unique constraint violation
> -            dataElementService.addDataElement( dataElementD );
> -            fail();
> -        }
> -        catch ( Exception e )
> -        {
> -            // Expected
> -        }
> -
>         dataElementA = dataElementService.getDataElement( idA );
>         assertNotNull( dataElementA );
>         assertEquals( idA, dataElementA.getId() );
> @@ -562,23 +550,11 @@
>         DataElementGroup dataElementGroupA = new DataElementGroup(
> "DataElementGroupA" );
>         DataElementGroup dataElementGroupB = new DataElementGroup(
> "DataElementGroupB" );
>         DataElementGroup dataElementGroupC = new DataElementGroup(
> "DataElementGroupC" );
> -        DataElementGroup dataElementGroupD = new DataElementGroup(
> "DataElementGroupA" );
>
>         int idA = dataElementService.addDataElementGroup( dataElementGroupA
> );
>         int idB = dataElementService.addDataElementGroup( dataElementGroupB
> );
>         int idC = dataElementService.addDataElementGroup( dataElementGroupC
> );
>
> -        try
> -        {
> -            // Should give unique constraint violation
> -            dataElementService.addDataElementGroup( dataElementGroupD );
> -            fail();
> -        }
> -        catch ( Exception e )
> -        {
> -            // Expected
> -        }
> -
>         dataElementGroupA = dataElementService.getDataElementGroup( idA );
>         assertNotNull( dataElementGroupA );
>         assertEquals( idA, dataElementGroupA.getId() );
>
> === modified file
> 'dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/indicator/IndicatorServiceTest.java'
> ---
> dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/indicator/IndicatorServiceTest.java
>      2009-06-10 22:25:07 +0000
> +++
> dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/indicator/IndicatorServiceTest.java
>      2009-11-10 11:45:58 +0000
> @@ -31,7 +31,6 @@
>  import static junit.framework.Assert.assertNotNull;
>  import static junit.framework.Assert.assertNull;
>  import static junit.framework.Assert.assertTrue;
> -import static junit.framework.Assert.fail;
>
>  import java.util.Collection;
>  import java.util.HashSet;
> @@ -83,20 +82,10 @@
>     {
>         IndicatorType typeA = new IndicatorType( "IndicatorTypeA", 100 );
>         IndicatorType typeB = new IndicatorType( "IndicatorTypeB", 1 );
> -        IndicatorType typeC = new IndicatorType( "IndicatorTypeA", 100 );
>
>         int idA = indicatorService.addIndicatorType( typeA );
>         int idB = indicatorService.addIndicatorType( typeB );
>
> -        try
> -        {
> -            indicatorService.addIndicatorType( typeC );
> -            fail( "Expected unique constraint exception" );
> -        }
> -        catch ( Exception ex )
> -        {
> -        }
> -
>         typeA = indicatorService.getIndicatorType( idA );
>         assertNotNull( typeA );
>         assertEquals( idA, typeA.getId() );
> @@ -194,20 +183,10 @@
>     {
>         IndicatorGroup groupA = new IndicatorGroup( "IndicatorGroupA" );
>         IndicatorGroup groupB = new IndicatorGroup( "IndicatorGroupB" );
> -        IndicatorGroup groupC = new IndicatorGroup( "IndicatorGroupA" );
>
>         int idA = indicatorService.addIndicatorGroup( groupA );
>         int idB = indicatorService.addIndicatorGroup( groupB );
>
> -        try
> -        {
> -            indicatorService.addIndicatorGroup( groupC );
> -            fail( "Expected unique constraint exception" );
> -        }
> -        catch ( Exception ex )
> -        {
> -        }
> -
>         groupA = indicatorService.getIndicatorGroup( idA );
>         assertNotNull( groupA );
>         assertEquals( idA, groupA.getId() );
> @@ -375,20 +354,10 @@
>
>         Indicator indicatorA = createIndicator( 'A', type );
>         Indicator indicatorB = createIndicator( 'B', type );
> -        Indicator indicatorC = createIndicator( 'A', type );
>
>         int idA = indicatorService.addIndicator( indicatorA );
>         int idB = indicatorService.addIndicator( indicatorB );
>
> -        try
> -        {
> -            indicatorService.addIndicator( indicatorC );
> -            fail( "Expected unique constraint exception" );
> -        }
> -        catch ( Exception ex )
> -        {
> -        }
> -
>         indicatorA = indicatorService.getIndicator( idA );
>         assertNotNull( indicatorA );
>         assertEq( 'A', indicatorA );
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~dhis2-devs<https://launchpad.net/%7Edhis2-devs>
> Post to     : dhis2-devs@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~dhis2-devs<https://launchpad.net/%7Edhis2-devs>
> More help   : https://help.launchpad.net/ListHelp
>
>
Follow ups

Re: [Branch ~dhis2-devs-core/dhis2/trunk] Rev 1018: Fixed the database internationalisation. Applying translations through AOP.
From: Ola Hodne Titlestad, 2009-11-10
References

[Branch ~dhis2-devs-core/dhis2/trunk] Rev 1018: Fixed the database internationalisation. Applying translations through AOP.
From: noreply, 2009-11-10