dhis2-devs team mailing list archive
-
dhis2-devs team
-
Mailing list archive
-
Message #34773
[Branch ~dhis2-devs-core/dhis2/trunk] Rev 17834: various importer fixes, properly handle cases where the object references are not readable by cur...
------------------------------------------------------------
revno: 17834
committer: Morten Olav Hansen <mortenoh@xxxxxxxxx>
branch nick: dhis2
timestamp: Tue 2014-12-30 13:06:44 +0100
message:
various importer fixes, properly handle cases where the object references are not readable by current user
modified:
dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/IdentifiableObjectManager.java
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/acl/DefaultAclService.java
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/DefaultIdentifiableObjectManager.java
dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/DefaultObjectBridge.java
dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/importers/DefaultIdentifiableObjectImporter.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-api/src/main/java/org/hisp/dhis/common/IdentifiableObjectManager.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/IdentifiableObjectManager.java 2014-12-30 09:38:00 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/IdentifiableObjectManager.java 2014-12-30 12:06:44 +0000
@@ -103,8 +103,12 @@
<T extends IdentifiableObject> Map<String, T> getIdMap( Class<T> clazz, IdentifiableProperty property );
+ <T extends IdentifiableObject> Map<String, T> getIdMapNoAcl( Class<T> clazz, IdentifiableProperty property );
+
<T extends NameableObject> Map<String, T> getIdMap( Class<T> clazz, NameableProperty property );
+ <T extends NameableObject> Map<String, T> getIdMapNoAcl( Class<T> clazz, NameableProperty property );
+
<T extends IdentifiableObject> T getObject( Class<T> clazz, IdentifiableProperty property, String id );
IdentifiableObject getObject( String uid, String simpleClassName );
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/acl/DefaultAclService.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/acl/DefaultAclService.java 2014-09-26 06:56:04 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/acl/DefaultAclService.java 2014-12-30 12:06:44 +0000
@@ -30,6 +30,7 @@
import org.hisp.dhis.common.IdentifiableObject;
import org.hisp.dhis.dashboard.Dashboard;
+import org.hisp.dhis.period.Period;
import org.hisp.dhis.schema.AuthorityType;
import org.hisp.dhis.schema.Schema;
import org.hisp.dhis.schema.SchemaService;
@@ -122,6 +123,11 @@
@Override
public boolean canRead( User user, IdentifiableObject object )
{
+ if ( object == null || Period.class.isInstance( object ) )
+ {
+ return true;
+ }
+
Schema schema = schemaService.getSchema( object.getClass() );
if ( schema == null )
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/DefaultIdentifiableObjectManager.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/DefaultIdentifiableObjectManager.java 2014-12-30 09:38:00 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/common/DefaultIdentifiableObjectManager.java 2014-12-30 12:06:44 +0000
@@ -630,6 +630,56 @@
@Override
@SuppressWarnings( "unchecked" )
+ public <T extends IdentifiableObject> Map<String, T> getIdMapNoAcl( Class<T> clazz, IdentifiableProperty property )
+ {
+ Map<String, T> map = new HashMap<>();
+
+ GenericIdentifiableObjectStore<T> store = (GenericIdentifiableObjectStore<T>) getIdentifiableObjectStore( clazz );
+
+ if ( store == null )
+ {
+ return map;
+ }
+
+ Collection<T> objects = store.getAllNoAcl();
+
+ for ( T object : objects )
+ {
+ if ( IdentifiableProperty.ID.equals( property ) )
+ {
+ if ( object.getId() > 0 )
+ {
+ map.put( String.valueOf( object.getId() ), object );
+ }
+ }
+ else if ( IdentifiableProperty.UID.equals( property ) )
+ {
+ if ( object.getUid() != null )
+ {
+ map.put( object.getUid(), object );
+ }
+ }
+ else if ( IdentifiableProperty.CODE.equals( property ) )
+ {
+ if ( object.getCode() != null )
+ {
+ map.put( object.getCode(), object );
+ }
+ }
+ else if ( IdentifiableProperty.NAME.equals( property ) )
+ {
+ if ( object.getName() != null )
+ {
+ map.put( object.getName(), object );
+ }
+ }
+ }
+
+ return map;
+ }
+
+ @Override
+ @SuppressWarnings( "unchecked" )
public <T extends NameableObject> Map<String, T> getIdMap( Class<T> clazz, NameableProperty property )
{
Map<String, T> map = new HashMap<>();
@@ -654,6 +704,30 @@
@Override
@SuppressWarnings( "unchecked" )
+ public <T extends NameableObject> Map<String, T> getIdMapNoAcl( Class<T> clazz, NameableProperty property )
+ {
+ Map<String, T> map = new HashMap<>();
+
+ GenericNameableObjectStore<T> store = (GenericNameableObjectStore<T>) getNameableObjectStore( clazz );
+
+ Collection<T> objects = store.getAllNoAcl();
+
+ for ( T object : objects )
+ {
+ if ( property == NameableProperty.SHORT_NAME )
+ {
+ if ( object.getShortName() != null )
+ {
+ map.put( object.getShortName(), object );
+ }
+ }
+ }
+
+ return map;
+ }
+
+ @Override
+ @SuppressWarnings( "unchecked" )
public <T extends IdentifiableObject> T getObject( Class<T> clazz, IdentifiableProperty property, String id )
{
GenericIdentifiableObjectStore<T> store = (GenericIdentifiableObjectStore<T>) getIdentifiableObjectStore( clazz );
=== modified file 'dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/DefaultObjectBridge.java'
--- dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/DefaultObjectBridge.java 2014-12-29 14:05:40 +0000
+++ dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/DefaultObjectBridge.java 2014-12-30 12:06:44 +0000
@@ -182,7 +182,7 @@
if ( preheatCache && IdentifiableObject.class.isAssignableFrom( clazz ) )
{
- map = new HashSet<>( manager.getAll( (Class<IdentifiableObject>) clazz ) );
+ map = new HashSet<>( manager.getAllNoAcl( (Class<IdentifiableObject>) clazz ) );
}
masterMap.put( clazz, map );
@@ -195,7 +195,7 @@
if ( preheatCache && IdentifiableObject.class.isAssignableFrom( clazz ) )
{
- map = (Map<String, IdentifiableObject>) manager.getIdMap( (Class<? extends IdentifiableObject>) clazz, property );
+ map = (Map<String, IdentifiableObject>) manager.getIdMapNoAcl( (Class<? extends IdentifiableObject>) clazz, property );
}
if ( !preheatCache || map != null )
=== modified file 'dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/importers/DefaultIdentifiableObjectImporter.java'
--- dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/importers/DefaultIdentifiableObjectImporter.java 2014-12-29 15:51:39 +0000
+++ dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/importers/DefaultIdentifiableObjectImporter.java 2014-12-30 12:06:44 +0000
@@ -43,7 +43,6 @@
import org.hisp.dhis.common.IdentifiableObjectManager;
import org.hisp.dhis.common.NameableObject;
import org.hisp.dhis.dashboard.DashboardItem;
-import org.hisp.dhis.dataelement.DataElement;
import org.hisp.dhis.dataelement.DataElementCategoryDimension;
import org.hisp.dhis.dataelement.DataElementCategoryService;
import org.hisp.dhis.dataelement.DataElementOperand;
@@ -289,7 +288,7 @@
// object.setUser( user );
object.setUser( null );
- NonIdentifiableObjects nonIdentifiableObjects = new NonIdentifiableObjects();
+ NonIdentifiableObjects nonIdentifiableObjects = new NonIdentifiableObjects( user );
nonIdentifiableObjects.extract( object );
UserCredentials userCredentials = null;
@@ -310,14 +309,14 @@
Map<Field, Object> fields = detachFields( object );
Map<Field, Collection<Object>> collectionFields = detachCollectionFields( object );
- reattachFields( object, fields );
+ reattachFields( object, fields, user );
log.debug( "Trying to save new object => " + ImportUtils.getDisplayName( object ) + " (" + object.getClass().getSimpleName() + ")" +
"" );
objectBridge.saveObject( object );
updatePeriodTypes( object );
- reattachCollectionFields( object, collectionFields );
+ reattachCollectionFields( object, collectionFields, user );
objectBridge.updateObject( object );
@@ -335,7 +334,7 @@
sessionFactory.getCurrentSession().save( userCredentials );
- reattachCollectionFields( userCredentials, collectionFieldsUserCredentials );
+ reattachCollectionFields( userCredentials, collectionFieldsUserCredentials, user );
sessionFactory.getCurrentSession().saveOrUpdate( userCredentials );
@@ -385,7 +384,7 @@
return true;
}
- NonIdentifiableObjects nonIdentifiableObjects = new NonIdentifiableObjects();
+ NonIdentifiableObjects nonIdentifiableObjects = new NonIdentifiableObjects( user );
nonIdentifiableObjects.extract( object );
nonIdentifiableObjects.delete( persistedObject );
@@ -407,13 +406,13 @@
Map<Field, Object> fields = detachFields( object );
Map<Field, Collection<Object>> collectionFields = detachCollectionFields( object );
- reattachFields( object, fields );
+ reattachFields( object, fields, user );
persistedObject.mergeWith( object );
updatePeriodTypes( persistedObject );
- reattachCollectionFields( persistedObject, collectionFields );
+ reattachCollectionFields( persistedObject, collectionFields, user );
log.debug( "Starting update of object " + ImportUtils.getDisplayName( persistedObject ) + " (" + persistedObject.getClass()
.getSimpleName() + ")" );
@@ -432,7 +431,7 @@
}
((User) persistedObject).getUserCredentials().mergeWith( userCredentials );
- reattachCollectionFields( ((User) persistedObject).getUserCredentials(), collectionFieldsUserCredentials );
+ reattachCollectionFields( ((User) persistedObject).getUserCredentials(), collectionFieldsUserCredentials, user );
sessionFactory.getCurrentSession().saveOrUpdate( ((User) persistedObject).getUserCredentials() );
}
@@ -707,7 +706,7 @@
return fieldMap;
}
- private void reattachFields( Object object, Map<Field, Object> fields )
+ private void reattachFields( Object object, Map<Field, Object> fields, User user )
{
for ( Field field : fields.keySet() )
{
@@ -721,6 +720,14 @@
reportReferenceError( object, idObject );
}
}
+ else
+ {
+ if ( !aclService.canRead( user, reference ) )
+ {
+ reportReadAccessError( object, reference );
+ reference = null;
+ }
+ }
if ( !options.isDryRun() )
{
@@ -749,7 +756,7 @@
return collectionFields;
}
- private void reattachCollectionFields( final Object idObject, Map<Field, Collection<Object>> collectionFields )
+ private void reattachCollectionFields( final Object idObject, Map<Field, Collection<Object>> collectionFields, User user )
{
for ( Field field : collectionFields.keySet() )
{
@@ -758,11 +765,18 @@
for ( Object object : collection )
{
- IdentifiableObject ref = findObjectByReference( (IdentifiableObject) object );
+ IdentifiableObject reference = findObjectByReference( (IdentifiableObject) object );
- if ( ref != null )
+ if ( reference != null )
{
- objects.add( ref );
+ if ( !aclService.canRead( user, reference ) )
+ {
+ reportReadAccessError( idObject, reference );
+ }
+ else
+ {
+ objects.add( reference );
+ }
}
else
{
@@ -829,6 +843,20 @@
summaryType.getImportConflicts().add( importConflict );
}
+ private void reportReadAccessError( Object object, Object reference )
+ {
+ String objectName = object != null ? object.getClass().getSimpleName() : "null";
+ String referenceName = reference != null ? reference.getClass().getSimpleName() : "null";
+
+ String logMsg = "User does not have read access to " + identifiableObjectToString( reference ) + " (" + referenceName + ")" +
+ " on object " + identifiableObjectToString( object ) + " (" + objectName + ").";
+
+ log.debug( logMsg );
+
+ ImportConflict importConflict = new ImportConflict( ImportUtils.getDisplayName( object ), logMsg );
+ summaryType.getImportConflicts().add( importConflict );
+ }
+
//-------------------------------------------------------------------------------------------------------
// Internal state
//-------------------------------------------------------------------------------------------------------
@@ -856,6 +884,13 @@
private List<DataElementCategoryDimension> categoryDimensions = new ArrayList<>();
+ private User user;
+
+ public NonIdentifiableObjects( User user )
+ {
+ this.user = user;
+ }
+
public void extract( T object )
{
attributeValues = extractAttributeValues( object );
@@ -910,7 +945,7 @@
if ( dataEntryForm != null )
{
Map<Field, Collection<Object>> identifiableObjectCollections = detachCollectionFields( dataEntryForm );
- reattachCollectionFields( dataEntryForm, identifiableObjectCollections );
+ reattachCollectionFields( dataEntryForm, identifiableObjectCollections, user );
dataEntryForm.setId( 0 );
dataEntryFormService.addDataEntryForm( dataEntryForm );
@@ -1015,10 +1050,10 @@
for ( DataElementCategoryDimension categoryDimension : categoryDimensions )
{
Map<Field, Object> detachFields = detachFields( categoryDimension );
- reattachFields( categoryDimension, detachFields );
+ reattachFields( categoryDimension, detachFields, user );
Map<Field, Collection<Object>> detachCollectionFields = detachCollectionFields( categoryDimension );
- reattachCollectionFields( categoryDimension, detachCollectionFields );
+ reattachCollectionFields( categoryDimension, detachCollectionFields, user );
if ( !options.isDryRun() )
{
@@ -1052,7 +1087,7 @@
if ( expression != null )
{
Map<Field, Collection<Object>> identifiableObjectCollections = detachCollectionFields( expression );
- reattachCollectionFields( expression, identifiableObjectCollections );
+ reattachCollectionFields( expression, identifiableObjectCollections, user );
expression.setId( 0 );
expressionService.addExpression( expression );
@@ -1073,7 +1108,7 @@
for ( DataElementOperand dataElementOperand : dataElementOperands )
{
Map<Field, Object> identifiableObjects = detachFields( dataElementOperand );
- reattachFields( dataElementOperand, identifiableObjects );
+ reattachFields( dataElementOperand, identifiableObjects, user );
dataElementOperand.setId( 0 );
dataElementOperandService.addDataElementOperand( dataElementOperand );
@@ -1187,7 +1222,7 @@
for ( ProgramTrackedEntityAttribute programTrackedEntityAttribute : programTrackedEntityAttributes )
{
Map<Field, Object> identifiableObjects = detachFields( programTrackedEntityAttribute );
- reattachFields( programTrackedEntityAttribute, identifiableObjects );
+ reattachFields( programTrackedEntityAttribute, identifiableObjects, user );
sessionFactory.getCurrentSession().save( programTrackedEntityAttribute );
}
@@ -1235,7 +1270,7 @@
for ( ProgramStageDataElement programStageDataElement : programStageDataElements )
{
Map<Field, Object> identifiableObjects = detachFields( programStageDataElement );
- reattachFields( programStageDataElement, identifiableObjects );
+ reattachFields( programStageDataElement, identifiableObjects, user );
if ( ProgramStage.class.isAssignableFrom( object.getClass() ) )
{
