revision-diff.txt (4.07 KB)
Sorry, had to revert this commit… The error was not in the
if ( categories.contains( category ) )
clause, the problem was that the equals method on DataElementCategory was badly implemented leading to “contains” always returning false.
Even if it is not strickly needed in this case, this check is a general pattern we have for all the *Association converters. For the detailed meta data export one might want to export only selected data elements and groups. In that case one cannot export all dataelement-dataelementgroup-associations, only those which are included in that export…
Lars
···
On Wed, Dec 16, 2009 at 11:45 AM, noreply@launchpad.net wrote:
revno: 1232
committer: Bob Jolliffe bobj@bobj-laptop
branch nick: trunk
timestamp: Wed 2009-12-16 10:41:19 +0000
message:
Fixed error in dxf export of categoryComboCategoryAssociations. Simplified the convertor.
modified:
dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/CategoryComboCategoryAssociationConverter.java
dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/DataElementCategoryOptionComboConverter.java
–
lp:dhis2
https://code.launchpad.net/~dhis2-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.
=== modified file ‘dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/CategoryComboCategoryAssociationConverter.java’
— dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/CategoryComboCategoryAssociationConverter.java 2009-11-02 09:20:10 +0000
+++ dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/CategoryComboCategoryAssociationConverter.java 2009-12-16 10:41:19 +0000
@@ -29,6 +29,8 @@
import java.util.Collection;
import java.util.Map;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.amplecode.quick.BatchHandler;
import org.amplecode.staxwax.reader.XMLReader;
@@ -52,6 +54,8 @@
public class CategoryComboCategoryAssociationConverter
extends AbstractGroupMemberConverter implements XMLConverter{
private static final Log log = LogFactory.getLog(CategoryComboCategoryAssociationConverter.class);
public static final String COLLECTION_NAME = "categoryComboCategoryAssociations"; public static final String ELEMENT_NAME = "categoryComboCategoryAssociation";@@ -108,9 +112,7 @@
{ Collection<DataElementCategoryCombo> categoryCombos = categoryService.getDataElementCategoryCombos( params.getCategoryCombos() );
Collection<DataElementCategory> categories = categoryService.getDataElementCategories( params.getCategories() );if ( categoryCombos != null && categoryCombos.size() > 0 && categories != null && categories.size() > 0 )
if ( categoryCombos != null && categoryCombos.size() > 0 ) { writer.openElement( COLLECTION_NAME );@@ -122,8 +124,6 @@
for ( DataElementCategory category : categoryCombo.getCategories() ) {
if ( categories.contains( category ) ){ writer.openElement( ELEMENT_NAME ); writer.writeElement( FIELD_CATEGORY_COMBO, String.valueOf( categoryCombo.getId() ) );@@ -131,7 +131,6 @@
writer.writeElement( FIELD_SORT_ORDER, String.valueOf( sortOrder++ ) ); writer.closeElement();
} } } }=== modified file ‘dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/DataElementCategoryOptionComboConverter.java’
— dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/DataElementCategoryOptionComboConverter.java 2009-10-18 22:44:41 +0000
+++ dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/DataElementCategoryOptionComboConverter.java 2009-12-16 10:41:19 +0000
@@ -29,6 +29,9 @@
import java.util.Collection;
import java.util.Map;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.amplecode.staxwax.reader.XMLReader;
import org.amplecode.staxwax.writer.XMLWriter;
@@ -51,6 +54,9 @@
public class DataElementCategoryOptionComboConverter
extends AbstractDataElementCategoryOptionComboConverter implements XMLConverter{
private static final Log log = LogFactory.getLog(DataElementCategoryOptionComboConverter.class);
public static final String COLLECTION_NAME = "categoryOptionCombos"; public static final String ELEMENT_NAME = "categoryOptionCombo";@@ -133,7 +139,7 @@
// ------------------------------------------------------------- writer.openElement( SUB_COLLECTION_NAME );
for ( DataElementCategoryOption categoryOption : categoryOptionCombo.getCategoryOptions() ) { writer.openElement( SUB_COLLECTION_ELEMENT_NAME );
Mailing list: https://launchpad.net/~dhis2-devs
Post to : dhis2-devs@lists.launchpad.net
Unsubscribe : https://launchpad.net/~dhis2-devs
More help : https://help.launchpad.net/ListHelp
OK. Fixing the equals method was the right thing to do. I could see contains() was not doing what it should, but took the lazy way out when I spotted the redundancy. It really is not required in this case. Whenever we export a category we will always want to export all the category options. Regardless of any other constraint. So this is not acting as any kind of check - just burning cycles (computational masturbation 
If I was better with hibernate I would rather do a cascade save on things like this - would be more robust though I understand the performance concerns. Should we not emulate that anyway?
We add categoryoptions to the category and get the batchhandler for the category to take care of also persisting it’s options and conversely with writing out category elements. Well that won’t really be an issue for these elements in dxfv2 because we are not explicitly writing out the associations anyway.
Jo these will be issues for you to consider.
Bob.
···
2009/12/18 Lars Helge Øverland larshelge@gmail.com
Sorry, had to revert this commit… The error was not in the
if ( categories.contains( category ) )
clause, the problem was that the equals method on DataElementCategory was badly implemented leading to “contains” always returning false.
Even if it is not strickly needed in this case, this check is a general pattern we have for all the *Association converters. For the detailed meta data export one might want to export only selected data elements and groups. In that case one cannot export all dataelement-dataelementgroup-associations, only those which are included in that export…
Lars
On Wed, Dec 16, 2009 at 11:45 AM, noreply@launchpad.net wrote:
revno: 1232
committer: Bob Jolliffe bobj@bobj-laptop
branch nick: trunk
timestamp: Wed 2009-12-16 10:41:19 +0000
message:
Fixed error in dxf export of categoryComboCategoryAssociations. Simplified the convertor.
modified:
dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/CategoryComboCategoryAssociationConverter.java
dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/DataElementCategoryOptionComboConverter.java
–
lp:dhis2
https://code.launchpad.net/~dhis2-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.
=== modified file ‘dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/CategoryComboCategoryAssociationConverter.java’
— dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/CategoryComboCategoryAssociationConverter.java 2009-11-02 09:20:10 +0000
+++ dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/CategoryComboCategoryAssociationConverter.java 2009-12-16 10:41:19 +0000
@@ -29,6 +29,8 @@
import java.util.Collection;
import java.util.Map;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.amplecode.quick.BatchHandler;
import org.amplecode.staxwax.reader.XMLReader;
@@ -52,6 +54,8 @@
public class CategoryComboCategoryAssociationConverter
extends AbstractGroupMemberConverter implements XMLConverter{
private static final Log log = LogFactory.getLog(CategoryComboCategoryAssociationConverter.class);
public static final String COLLECTION_NAME = "categoryComboCategoryAssociations"; public static final String ELEMENT_NAME = "categoryComboCategoryAssociation";@@ -108,9 +112,7 @@
{ Collection<DataElementCategoryCombo> categoryCombos = categoryService.getDataElementCategoryCombos( params.getCategoryCombos() );
Collection<DataElementCategory> categories = categoryService.getDataElementCategories( params.getCategories() );if ( categoryCombos != null && categoryCombos.size() > 0 && categories != null && categories.size() > 0 )
if ( categoryCombos != null && categoryCombos.size() > 0 ) { writer.openElement( COLLECTION_NAME );@@ -122,8 +124,6 @@
for ( DataElementCategory category : categoryCombo.getCategories() ) {
if ( categories.contains( category ) ){ writer.openElement( ELEMENT_NAME ); writer.writeElement( FIELD_CATEGORY_COMBO, String.valueOf( categoryCombo.getId() ) );@@ -131,7 +131,6 @@
writer.writeElement( FIELD_SORT_ORDER, String.valueOf( sortOrder++ ) ); writer.closeElement();
} } } }=== modified file ‘dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/DataElementCategoryOptionComboConverter.java’
— dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/DataElementCategoryOptionComboConverter.java 2009-10-18 22:44:41 +0000
+++ dhis-2/dhis-services/dhis-service-importexport/src/main/java/org/hisp/dhis/importexport/dxf/converter/DataElementCategoryOptionComboConverter.java 2009-12-16 10:41:19 +0000
@@ -29,6 +29,9 @@
import java.util.Collection;
import java.util.Map;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.amplecode.staxwax.reader.XMLReader;
import org.amplecode.staxwax.writer.XMLWriter;
@@ -51,6 +54,9 @@
public class DataElementCategoryOptionComboConverter
extends AbstractDataElementCategoryOptionComboConverter implements XMLConverter{
private static final Log log = LogFactory.getLog(DataElementCategoryOptionComboConverter.class);
public static final String COLLECTION_NAME = "categoryOptionCombos"; public static final String ELEMENT_NAME = "categoryOptionCombo";@@ -133,7 +139,7 @@
// ------------------------------------------------------------- writer.openElement( SUB_COLLECTION_NAME );
for ( DataElementCategoryOption categoryOption : categoryOptionCombo.getCategoryOptions() ) { writer.openElement( SUB_COLLECTION_ELEMENT_NAME );
Mailing list: https://launchpad.net/~dhis2-devs
Post to : dhis2-devs@lists.launchpad.net
Unsubscribe : https://launchpad.net/~dhis2-devs
More help : https://help.launchpad.net/ListHelp
Mailing list: https://launchpad.net/~dhis2-devs
Post to : dhis2-devs@lists.launchpad.net
Unsubscribe : https://launchpad.net/~dhis2-devs
More help : https://help.launchpad.net/ListHelp
OK. Fixing the equals method was the right thing to do. I could see contains() was not doing what it should, but took the lazy way out when I spotted the redundancy. It really is not required in this case. Whenever we export a category we will always want to export all the category options. Regardless of any other constraint. So this is not acting as any kind of check - just burning cycles (computational masturbation
I agree, but is it necessary to export all category options when exporting a category, including those which don’t belong to it? Anyway this is more of a hypothetical discussion, I don’t have strong opinions here…
If I was better with hibernate I would rather do a cascade save on things like this - would be more robust though I understand the performance concerns. Should we not emulate that anyway?
For the category stuff we might want to use Hibernate as it is generally not so many of them, meaning performance won’t be a big issue. Yes we might emulate it - its just that its complex, I have already spent lots of hours banging my head trying to make it work and I am not sure if it makes sense to do that all over again…
We add categoryoptions to the category and get the batchhandler for the category to take care of also persisting it’s options and conversely with writing out category elements. Well that won’t really be an issue for these elements in dxfv2 because we are not explicitly writing out the associations anyway.
Jo these will be issues for you to consider.
Sure, that’s fine with me. Its just that I don’t want to start going too far into Hibernate-domain with the batchhandler stuff…
Lars
···
2009/12/18 Bob Jolliffe bobjolliffe@gmail.com