fix: exclude embedded classes from schemas endpoint (42)

dhis-2/dhis-services/dhis-service-schema/src/main/java/org/hisp/dhis/schema/DefaultSchemaService.java

@@ -119,6 +119,7 @@
 import org.hisp.dhis.schema.descriptors.LegendSchemaDescriptor;
 import org.hisp.dhis.schema.descriptors.LegendSetSchemaDescriptor;
 import org.hisp.dhis.schema.descriptors.MapSchemaDescriptor;
+import org.hisp.dhis.schema.descriptors.MapViewSchemaDescriptor;
 import org.hisp.dhis.schema.descriptors.MessageConversationSchemaDescriptor;
 import org.hisp.dhis.schema.descriptors.MetadataVersionSchemaDescriptor;
 import org.hisp.dhis.schema.descriptors.MinMaxDataElementSchemaDescriptor;
@@ -247,6 +248,7 @@ private void init() {
     register(new LegendSetSchemaDescriptor());
     register(new ExternalMapLayerSchemaDescriptor());
     register(new MapSchemaDescriptor());
+    register(new MapViewSchemaDescriptor());
     register(new MessageConversationSchemaDescriptor());
     register(new MetadataVersionSchemaDescriptor());
     register(new OptionSchemaDescriptor());
@@ -327,6 +329,8 @@ private void init() {
 
   private final Map<Class<?>, Schema> classSchemaMap = new HashMap<>();
 
+  private final List<Schema> nonEmbeddedSchemaMap = new ArrayList<>();
+
   private final Map<String, Schema> singularSchemaMap = new HashMap<>();
 
   private final Map<String, Schema> pluralSchemaMap = new HashMap<>();
@@ -391,6 +395,7 @@ public void handleContextRefresh(ContextRefreshedEvent contextRefreshedEvent) {
             Maps.newHashMap(propertyIntrospectorService.getPropertiesMap(schema.getKlass())));
       }
 
+      if (!schema.isEmbeddedObject()) nonEmbeddedSchemaMap.add(schema);
       classSchemaMap.put(schema.getKlass(), schema);
       singularSchemaMap.put(schema.getSingular(), schema);
       pluralSchemaMap.put(schema.getPlural(), schema);
@@ -482,16 +487,19 @@ public List<Schema> getSchemas() {
   }
 
   @Override
-  public List<Schema> getSortedSchemas() {
-    List<Schema> schemas = Lists.newArrayList(classSchemaMap.values());
-    schemas.sort(OrderComparator.INSTANCE);
+  public List<Schema> getNonEmbeddedSchemas() {
+    return nonEmbeddedSchemaMap;
+  }
 
-    return schemas;
+  @Override
+  public List<Schema> getSortedNonEmbeddedSchemas() {
+    nonEmbeddedSchemaMap.sort(OrderComparator.INSTANCE);
+    return nonEmbeddedSchemaMap;
   }
 
   @Override
   public List<Schema> getMetadataSchemas() {
-    List<Schema> schemas = getSchemas();
+    List<Schema> schemas = getNonEmbeddedSchemas();
 
     schemas.removeIf(schema -> !schema.isMetadata());
     schemas.sort(OrderComparator.INSTANCE);

dhis-2/dhis-services/dhis-service-schema/src/main/java/org/hisp/dhis/schema/SchemaService.java

@@ -86,12 +86,14 @@ public interface SchemaService {
    */
   List<Schema> getSchemas();
 
+  List<Schema> getNonEmbeddedSchemas();
+
   /**
    * Get all available schemas (which are generated with a schema descriptor).
    *
    * @return List of all available schemas
    */
-  List<Schema> getSortedSchemas();
+  List<Schema> getSortedNonEmbeddedSchemas();
 
   /**
    * Get all available schemas which have the metadata property set to true.

dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/metadata/export/MetadataExportWithDependenciesTest.java

@@ -154,7 +154,6 @@ void exportMetadataShouldNotContainMapView() throws IOException {
 
     MetadataExportParams exportParams = new MetadataExportParams();
     exportParams.addClass(org.hisp.dhis.mapping.Map.class);
-    exportParams.addClass(MapView.class);
     ObjectNode exported = metadataExportService.getMetadataAsObjectNode(exportParams);
 
     Map<Class<? extends IdentifiableObject>, List<IdentifiableObject>> metadata =

dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/MapControllerTest.java

@@ -82,6 +82,71 @@ void testPutJsonObject_NotFound() {
         PUT("/maps/xyz", "{'name':'My updated map'}").content(HttpStatus.NOT_FOUND));
   }
 
+  @Test
+  void testImportExportedMapWithLegendSet() {
+    String ouId =
+        assertStatus(
+            HttpStatus.CREATED,
+            POST(
+                "/organisationUnits/",
+                "{'name':'OU1','shortName':'OU1','openingDate':'2020-01-01'}"));
+    String indicatorTypeId =
+        assertStatus(
+            HttpStatus.CREATED,
+            POST("/indicatorTypes", "{'name':'Per cent','factor':100,'number':false}"));
+    String indicatorId =
+        assertStatus(
+            HttpStatus.CREATED,
+            POST(
+                "/indicators",
+                "{'name':'Ind1','shortName':'Ind1','indicatorType':{'id':'"
+                    + indicatorTypeId
+                    + "'},'numerator':'1','denominator':'1'}"));
+    String legendSetId = assertStatus(HttpStatus.CREATED, POST("/legendSets/", "{'name':'LS'}"));
+
+    String mapId =
+        assertStatus(
+            HttpStatus.CREATED,
+            POST(
+                "/maps/",
+                "{'name':'My map','mapViews':[{"
+                    + "'layer':'thematic1','renderingStrategy':'SINGLE',"
+                    + "'legendSet':{'id':'"
+                    + legendSetId
+                    + "'},"
+                    + "'columns':[{'dimension':'dx','items':[{'id':'"
+                    + indicatorId
+                    + "'}]}],"
+                    + "'rows':[{'dimension':'ou','items':[{'id':'"
+                    + ouId
+                    + "'}]}],"
+                    + "'filters':[{'dimension':'pe','items':[{'id':'THIS_YEAR'}]}]"
+                    + "}]}"));
+
+    JsonObject exported = GET("/maps/{uid}?fields=:owner", mapId).content();
+    System.out.println("EXPORTED=" + exported.toString());
+
+    System.out.println(
+        "METADATA_IMPORT="
+            + POST("/metadata", "{\"maps\":[" + exported.toString() + "]}")
+                .content(HttpStatus.OK)
+                .toString());
+
+    System.out.println("IMPORTED=" + GET("/maps/{uid}?fields=:owner", mapId).content().toString());
+    JsonObject importedView =
+        GET("/maps/{uid}", mapId).content().getArray("mapViews").get(0).as(JsonObject.class);
+    assertEquals(legendSetId, importedView.getObject("legendSet").getString("id").string());
+    assertEquals(
+        indicatorId,
+        importedView
+            .getArray("dataDimensionItems")
+            .get(0)
+            .as(JsonObject.class)
+            .getObject("indicator")
+            .getString("id")
+            .string());
+  }
+
   @Test
   void testGetWithMapViewAndOrgUnitField() {
     String attrId =

dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/MapImportPostgresIntegrationTest.java

@@ -0,0 +1,133 @@
+/*
+ * Copyright (c) 2004-2022, 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:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * 2. 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.
+ *
+ * 3. Neither the name of the copyright holder 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.
+ */
+package org.hisp.dhis.webapi.controller;
+
+import static org.hisp.dhis.http.HttpAssertions.assertStatus;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.hisp.dhis.http.HttpStatus;
+import org.hisp.dhis.jsontree.JsonObject;
+import org.hisp.dhis.test.webapi.PostgresControllerIntegrationTestBase;
+import org.hisp.dhis.test.webapi.json.domain.JsonImportSummary;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Reproduces the {@code TransientPropertyValueException: DataDimensionItem.indicator -> Indicator}
+ * that occurs when a {@link org.hisp.dhis.mapping.Map} is exported and re-imported through the
+ * {@code POST /metadata} import.
+ *
+ * <p>This test deliberately does <b>not</b> use {@code @Transactional}: the metadata import runs in
+ * its own transaction with a fresh Hibernate session, which is required to surface the bug (a
+ * {@code @Transactional} test shares the session, so the referenced {@code Indicator} is already
+ * managed and the bug is masked).
+ *
+ * @author vietnguyen
+ */
+class MapImportPostgresIntegrationTest extends PostgresControllerIntegrationTestBase {
+
+  private String mapId;
+
+  @AfterEach
+  void tearDownMap() {
+    // mapview has a FK to maplegendset, so the map (and its map views) must be removed before the
+    // test framework empties the database table-by-table.
+    if (mapId != null) {
+      DELETE("/maps/" + mapId);
+    }
+  }
+
+  @Test
+  void testReimportExportedMapWithIndicator() {
+    String ouId =
+        assertStatus(
+            HttpStatus.CREATED,
+            POST(
+                "/organisationUnits/",
+                "{'name':'OU1','shortName':'OU1','openingDate':'2020-01-01'}"));
+    String indicatorTypeId =
+        assertStatus(
+            HttpStatus.CREATED,
+            POST("/indicatorTypes", "{'name':'Per cent','factor':100,'number':false}"));
+    String indicatorId =
+        assertStatus(
+            HttpStatus.CREATED,
+            POST(
+                "/indicators",
+                "{'name':'Ind1','shortName':'Ind1','indicatorType':{'id':'"
+                    + indicatorTypeId
+                    + "'},'numerator':'1','denominator':'1'}"));
+    String legendSetId = assertStatus(HttpStatus.CREATED, POST("/legendSets/", "{'name':'LS'}"));
+
+    mapId =
+        assertStatus(
+            HttpStatus.CREATED,
+            POST(
+                "/maps/",
+                "{'name':'My map','mapViews':[{"
+                    + "'layer':'thematic1','renderingStrategy':'SINGLE',"
+                    + "'legendSet':{'id':'"
+                    + legendSetId
+                    + "'},"
+                    + "'columns':[{'dimension':'dx','items':[{'id':'"
+                    + indicatorId
+                    + "'}]}],"
+                    + "'rows':[{'dimension':'ou','items':[{'id':'"
+                    + ouId
+                    + "'}]}],"
+                    + "'filters':[{'dimension':'pe','items':[{'id':'THIS_YEAR'}]}]"
+                    + "}]}]}"));
+
+    JsonObject exported = GET("/maps/{uid}?fields=:owner", mapId).content();
+
+    // Re-import the exported map
+    JsonImportSummary summary =
+        POST("/metadata", "{\"maps\":[" + exported.toString() + "]}")
+            .content(HttpStatus.OK)
+            .get("response")
+            .as(JsonImportSummary.class);
+
+    // the import must not have failed with TransientPropertyValueException on DataDimensionItem
+    assertEquals("OK", summary.getStatus(), "metadata import failed: " + summary.getTypeReports());
+
+    JsonObject importedView =
+        GET("/maps/{uid}", mapId).content().getArray("mapViews").get(0).as(JsonObject.class);
+    assertEquals(legendSetId, importedView.getObject("legendSet").getString("id").string());
+    assertEquals(
+        indicatorId,
+        importedView
+            .getArray("dataDimensionItems")
+            .get(0)
+            .as(JsonObject.class)
+            .getObject("indicator")
+            .getString("id")
+            .string());
+  }
+}

dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/SchemaControllerTest.java

@@ -124,6 +124,33 @@
     }
   }
 
+  @Test
+  void testSchemasDoNotContainEmbeddedObjects() {
+    JsonList<JsonSchema> schemas =
+        GET("/schemas?fields=name,klass,embeddedObject")
+            .content(HttpStatus.OK)
+            .getList("schemas", JsonSchema.class);
+    assertFalse(schemas.isEmpty());
+    java.util.List<String> embedded =
+        schemas.stream()
+            .filter(JsonSchema::isEmbeddedObject)
+            .map(JsonSchema::getName)
+            .collect(java.util.stream.Collectors.toList());
+    assertTrue(embedded.isEmpty(), () -> "embedded schemas should not be returned: " + embedded);
+  }
+
+  @org.springframework.beans.factory.annotation.Autowired
+  private org.hisp.dhis.schema.SchemaService schemaService;
+
+  @Test
+  void testSchemasDoNotContainMapView() {
+    JsonList<JsonSchema> schemas =
+        GET("/schemas?fields=name").content(HttpStatus.OK).getList("schemas", JsonSchema.class);
+    assertFalse(
+        schemas.stream().anyMatch(schema -> "mapView".equals(schema.getName())),
+        "embedded mapView schema should not be returned");
+  }
+
   @Test
   void testAttributeWritable() {
     JsonSchema schema = GET("/schemas/attribute").content().as(JsonSchema.class);

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SchemaController.java

@@ -95,7 +95,7 @@ public class SchemaController {
   @GetMapping
   public ResponseEntity<JsonRoot> getSchemas(
       @RequestParam(defaultValue = "*") List<String> fields) {
-    List<Schema> schemas = schemaService.getSortedSchemas();
+    List<Schema> schemas = schemaService.getSortedNonEmbeddedSchemas();
     linkService.generateSchemaLinks(schemas);
 
     FieldFilterParams<Schema> params = FieldFilterParams.of(schemas, fields);