dhis2-devs team mailing list archive
-
dhis2-devs team
-
Mailing list archive
-
Message #20804
[Branch ~dhis2-devs-core/dhis2/trunk] Rev 9674: Analytics, improved validation and error responses. Improved testing. Improved meta-data part of ...
------------------------------------------------------------
revno: 9674
committer: Lars Helge Øverland <larshelge@xxxxxxxxx>
branch nick: dhis2
timestamp: Thu 2013-01-31 12:59:22 +0200
message:
Analytics, improved validation and error responses. Improved testing. Improved meta-data part of response.
modified:
dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryParams.java
dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultAnalyticsService.java
dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTableManager.java
dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/DataQueryParamsTest.java
dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/data/AnalyticsServiceTest.java
dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/api/controller/AnalyticsController.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-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryParams.java'
--- dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryParams.java 2013-01-30 13:46:01 +0000
+++ dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/DataQueryParams.java 2013-01-31 10:59:22 +0000
@@ -65,14 +65,15 @@
public static final String LEVEL_PREFIX = "uidlevel";
private static final String DIMENSION_NAME_SEP = ":";
- private static final String OPTION_SEP = ",";
+ private static final String OPTION_SEP = ";";
public static final String DIMENSION_SEP = "-";
+
+ public static final List<String> DATA_DIMS = Arrays.asList( INDICATOR_DIM_ID, DATAELEMENT_DIM_ID, DATASET_DIM_ID );
+ public static final List<String> FIXED_DIMS = Arrays.asList( DATA_X_DIM_ID, INDICATOR_DIM_ID, DATAELEMENT_DIM_ID, DATASET_DIM_ID, PERIOD_DIM_ID, ORGUNIT_DIM_ID );
private static final DimensionOption[] DIM_OPT_ARR = new DimensionOption[0];
private static final DimensionOption[][] DIM_OPT_2D_ARR = new DimensionOption[0][];
- private static final List<String> DATA_DIMS = Arrays.asList( INDICATOR_DIM_ID, DATAELEMENT_DIM_ID, DATASET_DIM_ID );
-
private List<Dimension> dimensions = new ArrayList<Dimension>();
private List<Dimension> filters = new ArrayList<Dimension>();
@@ -240,36 +241,15 @@
}
/**
- * Returns the first dimension which has no dimension options.
- */
- public Dimension getEmptyDimension()
- {
- for ( Dimension dim : dimensions )
- {
- if ( dim.getOptions() == null || dim.getOptions().isEmpty() )
- {
- return dim;
- }
- }
-
- for ( Dimension filter : filters )
- {
- if ( filter == null || filter.getOptions().isEmpty() )
- {
- return filter;
- }
- }
-
- return null;
- }
-
- /**
* Indicates whether periods are present as a dimension or as a filter. If
* not this object is in an illegal state.
*/
public boolean hasPeriods()
{
- return getDimensionOptions( PERIOD_DIM_ID ) != null || getFilterOptions( PERIOD_DIM_ID ) != null;
+ List<IdentifiableObject> dimOpts = getDimensionOptions( PERIOD_DIM_ID );
+ List<IdentifiableObject> filterOpts = getFilterOptions( PERIOD_DIM_ID );
+
+ return ( dimOpts != null && !dimOpts.isEmpty() ) || ( filterOpts != null && !filterOpts.isEmpty() );
}
/**
@@ -318,32 +298,6 @@
}
/**
- * Returns a mapping between the uid and name for all options in all dimensions.
- */
- public Map<String, String> getUidNameMap()
- {
- Map<String, String> map = new HashMap<String, String>();
-
- for ( Dimension dimension : dimensions )
- {
- for ( IdentifiableObject idObject : dimension.getOptions() )
- {
- map.put( idObject.getUid(), idObject.getDisplayName() );
- }
- }
-
- for ( Dimension filter : filters )
- {
- for ( IdentifiableObject idObject : filter.getOptions() )
- {
- map.put( idObject.getUid(), idObject.getDisplayName() );
- }
- }
-
- return map;
- }
-
- /**
* Generates all permutations of the dimension and filter options for this query.
* Ignores the data element, category option combo and indicator dimensions.
*/
=== modified file 'dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultAnalyticsService.java'
--- dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultAnalyticsService.java 2013-01-30 20:37:26 +0000
+++ dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/data/DefaultAnalyticsService.java 2013-01-31 10:59:22 +0000
@@ -34,6 +34,7 @@
import static org.hisp.dhis.analytics.DataQueryParams.DATASET_DIM_ID;
import static org.hisp.dhis.analytics.DataQueryParams.DATA_X_DIM_ID;
import static org.hisp.dhis.analytics.DataQueryParams.DIMENSION_SEP;
+import static org.hisp.dhis.analytics.DataQueryParams.FIXED_DIMS;
import static org.hisp.dhis.analytics.DataQueryParams.INDICATOR_DIM_ID;
import static org.hisp.dhis.analytics.DataQueryParams.ORGUNIT_DIM_ID;
import static org.hisp.dhis.analytics.DataQueryParams.PERIOD_DIM_ID;
@@ -98,7 +99,6 @@
//TODO completeness
//TODO make sure data x dims are successive
- //TODO disable all dim options? often leads to sequential scans
@Autowired
private AnalyticsManager analyticsManager;
@@ -141,7 +141,7 @@
// Headers and meta-data
// ---------------------------------------------------------------------
- grid.setMetaData( params.getUidNameMap() );
+ grid.setMetaData( getUidNameMap( params ) );
for ( Dimension col : params.getHeaderDimensions() )
{
@@ -346,6 +346,10 @@
// Supportive methods
// -------------------------------------------------------------------------
+ /**
+ * Returns a list of dimensions generated from the given dimension identifier
+ * and list of dimension options.
+ */
private List<Dimension> getDimension( String dimension, List<String> options, I18nFormat format )
{
if ( DATA_X_DIM_ID.equals( dimension ) )
@@ -400,19 +404,34 @@
dataDimensions.add( new Dimension( DATASET_DIM_ID, DimensionType.DATASET, dataSets ) );
}
+ if ( indicators.isEmpty() && dataElements.isEmpty() && dataSets.isEmpty() )
+ {
+ throw new IllegalQueryException( "Dimension dx is present in query without any valid dimension options" );
+ }
+
return dataDimensions;
}
- else if ( CATEGORYOPTIONCOMBO_DIM_ID.equals( dimension ) )
+
+ if ( CATEGORYOPTIONCOMBO_DIM_ID.equals( dimension ) )
{
return Arrays.asList( new Dimension( dimension, DimensionType.CATEGORY_OPTION_COMBO, new ArrayList<IdentifiableObject>() ) );
}
- else if ( ORGUNIT_DIM_ID.equals( dimension ) )
+
+ if ( ORGUNIT_DIM_ID.equals( dimension ) )
{
- return Arrays.asList( new Dimension( dimension, DimensionType.ORGANISATIONUNIT, asList( organisationUnitService.getOrganisationUnitsByUid( options ) ) ) );
+ List<IdentifiableObject> ous = asList( organisationUnitService.getOrganisationUnitsByUid( options ) );
+
+ if ( ous == null || ous.isEmpty() )
+ {
+ throw new IllegalQueryException( "Dimension ou is present in query without any valid dimension options" );
+ }
+
+ return Arrays.asList( new Dimension( dimension, DimensionType.ORGANISATIONUNIT, ous ) );
}
- else if ( PERIOD_DIM_ID.equals( dimension ) )
+
+ if ( PERIOD_DIM_ID.equals( dimension ) )
{
- List<IdentifiableObject> list = new ArrayList<IdentifiableObject>();
+ List<IdentifiableObject> periods = new ArrayList<IdentifiableObject>();
periods : for ( String isoPeriod : options )
{
@@ -421,19 +440,24 @@
if ( period != null )
{
period.setName( format != null ? format.formatPeriod( period ) : null );
- list.add( period );
+ periods.add( period );
continue periods;
}
if ( RelativePeriodEnum.contains( isoPeriod ) )
{
RelativePeriodEnum relativePeriod = RelativePeriodEnum.valueOf( isoPeriod );
- list.addAll( RelativePeriods.getRelativePeriodsFromEnum( relativePeriod, format, true ) );
+ periods.addAll( RelativePeriods.getRelativePeriodsFromEnum( relativePeriod, format, true ) );
continue periods;
}
}
- return Arrays.asList( new Dimension( dimension, DimensionType.PERIOD, list ) );
+ if ( periods.isEmpty() )
+ {
+ throw new IllegalQueryException( "Dimension pe is present in query without any valid dimension options" );
+ }
+
+ return Arrays.asList( new Dimension( dimension, DimensionType.PERIOD, periods ) );
}
OrganisationUnitGroupSet orgUnitGroupSet = organisationUnitGroupService.getOrganisationUnitGroupSet( dimension );
@@ -467,4 +491,49 @@
return params;
}
+
+ private Map<String, String> getUidNameMap( DataQueryParams params )
+ {
+ Map<String, String> map = new HashMap<String, String>();
+ map.putAll( getUidNameMap( params.getDimensions() ) );
+ map.putAll( getUidNameMap( params.getFilters() ) );
+ return map;
+ }
+
+ private Map<String, String> getUidNameMap( List<Dimension> dimensions )
+ {
+ Map<String, String> map = new HashMap<String, String>();
+
+ for ( Dimension dimension : dimensions )
+ {
+ List<IdentifiableObject> options = new ArrayList<IdentifiableObject>( dimension.getOptions() );
+
+ // -----------------------------------------------------------------
+ // If dimension is not fixed and has no options, insert all options
+ // -----------------------------------------------------------------
+
+ if ( !FIXED_DIMS.contains( dimension.getDimension() ) && ( options == null || options.isEmpty() ) )
+ {
+ if ( DimensionType.ORGANISATIONUNIT_GROUPSET.equals( dimension.getType() ) )
+ {
+ options = asList( organisationUnitGroupService.getOrganisationUnitGroupSet( dimension.getDimension() ).getOrganisationUnitGroups() );
+ }
+ else if ( DimensionType.DATAELEMENT_GROUPSET.equals( dimension.getType() ) )
+ {
+ options = asList( dataElementService.getDataElementGroupSet( dimension.getDimension() ).getMembers() );
+ }
+ }
+
+ // -----------------------------------------------------------------
+ // Insert UID and name into map
+ // -----------------------------------------------------------------
+
+ for ( IdentifiableObject idObject : options )
+ {
+ map.put( idObject.getUid(), idObject.getDisplayName() );
+ }
+ }
+
+ return map;
+ }
}
=== modified file 'dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTableManager.java'
--- dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTableManager.java 2013-01-25 16:38:21 +0000
+++ dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcCompletenessTableManager.java 2013-01-31 10:59:22 +0000
@@ -61,7 +61,7 @@
sqlCreate += col[0] + " " + col[1] + ",";
}
- sqlCreate += "date date)";
+ sqlCreate += "value date)";
log.info( "Create SQL: " + sqlCreate );
@@ -81,7 +81,7 @@
insert += col[0] + ",";
}
- insert += "date) ";
+ insert += "value) ";
String select = "select ";
@@ -93,7 +93,7 @@
select = select.replace( "organisationunitid", "sourceid" ); // Legacy fix TODO remove
select +=
- "cdr.date as date " +
+ "cdr.date as value " +
"from completedatasetregistration cdr " +
"left join _organisationunitgroupsetstructure ougs on cdr.sourceid=ougs.organisationunitid " +
"left join _orgunitstructure ous on cdr.sourceid=ous.organisationunitid " +
=== modified file 'dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/DataQueryParamsTest.java'
--- dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/DataQueryParamsTest.java 2013-01-28 13:31:44 +0000
+++ dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/DataQueryParamsTest.java 2013-01-31 10:59:22 +0000
@@ -33,15 +33,18 @@
import java.util.List;
import java.util.Map;
+import org.hisp.dhis.common.IdentifiableObject;
+import org.hisp.dhis.period.Period;
import org.junit.Test;
import static org.junit.Assert.*;
+import static org.hisp.dhis.analytics.DataQueryParams.*;
public class DataQueryParamsTest
{
@Test
public void testGetDimensionFromParam()
{
- assertEquals( DataQueryParams.DATAELEMENT_DIM_ID, DataQueryParams.getDimensionFromParam( "de:D348asd782j,kj78HnH6hgT,9ds9dS98s2" ) );
+ assertEquals( DataQueryParams.DATAELEMENT_DIM_ID, DataQueryParams.getDimensionFromParam( "de:D348asd782j;kj78HnH6hgT;9ds9dS98s2" ) );
}
@Test
@@ -49,16 +52,39 @@
{
List<String> expected = new ArrayList<String>( Arrays.asList( "D348asd782j", "kj78HnH6hgT", "9ds9dS98s2" ) );
- assertEquals( expected, DataQueryParams.getDimensionOptionsFromParam( "de:D348asd782j,kj78HnH6hgT,9ds9dS98s2" ) );
+ assertEquals( expected, DataQueryParams.getDimensionOptionsFromParam( "de:D348asd782j;kj78HnH6hgT;9ds9dS98s2" ) );
}
@Test
- public void test()
+ public void testGetMeasureCriteriaFromParam()
{
Map<MeasureFilter, Double> expected = new HashMap<MeasureFilter, Double>();
expected.put( MeasureFilter.GT, 100d );
expected.put( MeasureFilter.LT, 200d );
- assertEquals( expected, DataQueryParams.getMeasureCriteriaFromParam( "GT:100,LT:200" ) );
+ assertEquals( expected, DataQueryParams.getMeasureCriteriaFromParam( "GT:100;LT:200" ) );
+ }
+
+ @Test
+ public void testHasPeriods()
+ {
+ DataQueryParams params = new DataQueryParams();
+
+ assertFalse( params.hasPeriods() );
+
+ List<IdentifiableObject> periods = new ArrayList<IdentifiableObject>();
+
+ params.getDimensions().add( new Dimension( PERIOD_DIM_ID, DimensionType.PERIOD, periods ) );
+
+ assertFalse( params.hasPeriods() );
+
+ params.removeDimension( PERIOD_DIM_ID );
+
+ assertFalse( params.hasPeriods() );
+
+ periods.add( new Period() );
+ params.getDimensions().add( new Dimension( PERIOD_DIM_ID, DimensionType.PERIOD, periods ) );
+
+ assertTrue( params.hasPeriods() );
}
}
=== modified file 'dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/data/AnalyticsServiceTest.java'
--- dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/data/AnalyticsServiceTest.java 2013-01-30 13:46:01 +0000
+++ dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/data/AnalyticsServiceTest.java 2013-01-31 10:59:22 +0000
@@ -27,12 +27,15 @@
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
+import static org.junit.Assert.assertEquals;
+
import java.util.HashSet;
import java.util.Set;
import org.hisp.dhis.DhisSpringTest;
import org.hisp.dhis.analytics.AnalyticsService;
import org.hisp.dhis.analytics.DataQueryParams;
+import org.hisp.dhis.analytics.IllegalQueryException;
import org.hisp.dhis.dataelement.DataElement;
import org.hisp.dhis.dataelement.DataElementService;
import org.hisp.dhis.organisationunit.OrganisationUnit;
@@ -43,8 +46,6 @@
import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired;
-import static org.junit.Assert.*;
-
public class AnalyticsServiceTest
extends DhisSpringTest
{
@@ -120,23 +121,76 @@
}
@Test
- public void testGetFromUrl()
+ public void testGetFromUrlA()
{
Set<String> dimensionParams = new HashSet<String>();
- dimensionParams.add( "dx:" + BASE_UID + "A," + BASE_UID + "B," + BASE_UID + "C," + BASE_UID + "D" );
- dimensionParams.add( "pe:2012,2012S1,2012S2" );
- dimensionParams.add( ouGroupSetA.getUid() + ":" + ouGroupA.getUid() + "," + ouGroupB.getUid() + "," + ouGroupC.getUid() );
+ dimensionParams.add( "dx:" + BASE_UID + "A;" + BASE_UID + "B;" + BASE_UID + "C;" + BASE_UID + "D" );
+ dimensionParams.add( "pe:2012;2012S1;2012S2" );
+ dimensionParams.add( ouGroupSetA.getUid() + ":" + ouGroupA.getUid() + ";" + ouGroupB.getUid() + ";" + ouGroupC.getUid() );
Set<String> filterParams = new HashSet<String>();
- filterParams.add( "ou:" + BASE_UID + "A," + BASE_UID + "B," + BASE_UID + "C," + BASE_UID + "D," + BASE_UID + "E" );
+ filterParams.add( "ou:" + BASE_UID + "A;" + BASE_UID + "B;" + BASE_UID + "C;" + BASE_UID + "D;" + BASE_UID + "E" );
DataQueryParams params = analyticsService.getFromUrl( dimensionParams, filterParams, null, null, null );
assertEquals( 4, params.getDataElements().size() );
assertEquals( 3, params.getPeriods().size() );
assertEquals( 5, params.getFilterOrganisationUnits().size() );
-
- assertEquals( 4, params.getDataElements().size() );
assertEquals( 3, params.getDimensionOptions( ouGroupSetA.getUid() ).size() );
}
+
+ @Test
+ public void testGetFromUrlB()
+ {
+ Set<String> dimensionParams = new HashSet<String>();
+ dimensionParams.add( "dx:" + BASE_UID + "A;" + BASE_UID + "B;" + BASE_UID + "C;" + BASE_UID + "D" );
+
+ Set<String> filterParams = new HashSet<String>();
+ filterParams.add( "ou:" + BASE_UID + "A" );
+
+ DataQueryParams params = analyticsService.getFromUrl( dimensionParams, filterParams, null, null, null );
+
+ assertEquals( 4, params.getDataElements().size() );
+ assertEquals( 1, params.getFilterOrganisationUnits().size() );
+ }
+
+ @Test( expected = IllegalQueryException.class )
+ public void testGetFromUrlNoDx()
+ {
+ Set<String> dimensionParams = new HashSet<String>();
+ dimensionParams.add( "dx" );
+ dimensionParams.add( "pe:2012,2012S1,2012S2" );
+
+ analyticsService.getFromUrl( dimensionParams, null, null, null, null );
+ }
+
+ @Test( expected = IllegalQueryException.class )
+ public void testGetFromUrlNoPeriods()
+ {
+ Set<String> dimensionParams = new HashSet<String>();
+ dimensionParams.add( "dx:" + BASE_UID + "A;" + BASE_UID + "B;" + BASE_UID + "C;" + BASE_UID + "D" );
+ dimensionParams.add( "pe" );
+
+ analyticsService.getFromUrl( dimensionParams, null, null, null, null );
+ }
+
+ @Test( expected = IllegalQueryException.class )
+ public void testGetFromUrlNoOrganisationUnits()
+ {
+ Set<String> dimensionParams = new HashSet<String>();
+ dimensionParams.add( "dx:" + BASE_UID + "A;" + BASE_UID + "B;" + BASE_UID + "C;" + BASE_UID + "D" );
+ dimensionParams.add( "ou" );
+
+ analyticsService.getFromUrl( dimensionParams, null, null, null, null );
+ }
+
+ @Test( expected = IllegalQueryException.class )
+ public void testGetFromUrlInvalidDimension()
+ {
+ Set<String> dimensionParams = new HashSet<String>();
+ dimensionParams.add( "dx:" + BASE_UID + "A;" + BASE_UID + "B;" + BASE_UID + "C;" + BASE_UID + "D" );
+ dimensionParams.add( "yebo:2012,2012S1,2012S2" );
+
+ analyticsService.getFromUrl( dimensionParams, null, null, null, null );
+ }
}
=== modified file 'dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/api/controller/AnalyticsController.java'
--- dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/api/controller/AnalyticsController.java 2013-01-30 13:46:01 +0000
+++ dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/api/controller/AnalyticsController.java 2013-01-31 10:59:22 +0000
@@ -192,7 +192,7 @@
ContextUtils.conflictResponse( response, "Indicators cannot be specified as filter" );
return false;
}
-
+
//TODO check if any dimension occur more than once
return true;
