[AJ-1279] Expose update attribute data type API (#479)

* Expose update attribute API

* Add test for successful update

* Use optionals (#487)

* Replace mapper.writeValueAsString with JSON strings in tests

* Revert "Replace mapper.writeValueAsString with JSON strings in tests"

This reverts commit dde2b92.

---------

Co-authored-by: Josh Ladieu <[email protected]>

service/src/main/java/org/databiosphere/workspacedataservice/controller/RecordController.java

@@ -31,6 +31,7 @@
 import org.springframework.web.bind.annotation.RequestParam;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.multipart.MultipartFile;
+import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody;
 
 @RestController
@@ -176,13 +177,29 @@ public ResponseEntity<AttributeSchema> updateAttribute(
       @PathVariable("type") RecordType recordType,
       @PathVariable("attribute") String attribute,
       @RequestBody AttributeSchema newAttributeSchema) {
-    String newAttributeName = newAttributeSchema.name();
-    recordOrchestratorService.renameAttribute(
-        instanceId, version, recordType, attribute, newAttributeName);
+    Optional<String> optionalNewAttributeName = Optional.ofNullable(newAttributeSchema.name());
+    Optional<String> optionalNewDataType = Optional.ofNullable(newAttributeSchema.datatype());
+
+    if (optionalNewAttributeName.isEmpty() && optionalNewDataType.isEmpty()) {
+      throw new ResponseStatusException(
+          HttpStatus.BAD_REQUEST, "At least one of name or datatype is required");
+    }
+
+    optionalNewAttributeName.ifPresent(
+        newAttributeName ->
+            recordOrchestratorService.renameAttribute(
+                instanceId, version, recordType, attribute, newAttributeName));
+
+    String finalAttributeName = optionalNewAttributeName.orElse(attribute);
+
+    optionalNewDataType.ifPresent(
+        newDataType ->
+            recordOrchestratorService.updateAttributeDataType(
+                instanceId, version, recordType, finalAttributeName, newDataType));
 
     RecordTypeSchema recordTypeSchema =
         recordOrchestratorService.describeRecordType(instanceId, version, recordType);
-    AttributeSchema attributeSchema = recordTypeSchema.getAttributeSchema(newAttributeName);
+    AttributeSchema attributeSchema = recordTypeSchema.getAttributeSchema(finalAttributeName);
     return new ResponseEntity<>(attributeSchema, HttpStatus.OK);
   }
 

service/src/main/resources/static/swagger/openapi-docs.yaml

@@ -503,7 +503,7 @@ paths:
         204:
           description: Success
         400:
-          description: Attribute is the primary key for record type
+          description: Update is invalid
           content:
             'application/json':
               schema:
@@ -515,7 +515,7 @@ paths:
               schema:
                 $ref: '#/components/schemas/ErrorResponse'
         409:
-          description: Rename conflicts with another attribute
+          description: Update cannot be applied to current data in the record type
           content:
             'application/json':
               schema:
@@ -712,6 +712,12 @@ components:
           type: string
         image:
           type: string
+    AttributeDataType:
+      type: string
+      enum: [ BOOLEAN, NUMBER, DATE, DATE_TIME, STRING, JSON, RELATION, FILE, ARRAY_OF_BOOLEAN, ARRAY_OF_STRING, ARRAY_OF_NUMBER, ARRAY_OF_DATE, ARRAY_OF_DATE_TIME, ARRAY_OF_RELATION, ARRAY_OF_FILE ]
+      description: |
+        Datatype of attribute. The enum of data types is in flux and will change. Please
+        comment at https://docs.google.com/document/d/1d352ZoN5kEYWPjy0NqqWGxdf7HEu5VEdrLmiAv7dMmQ/edit#heading=h.naxag0augkgf.
     AttributeSchema:
       type: object
       required:
@@ -722,22 +728,18 @@ components:
           type: string
           description: name of this attribute.
         datatype:
-          type: string
-          enum: [ BOOLEAN, NUMBER, DATE, DATE_TIME, STRING, JSON, RELATION, FILE, ARRAY_OF_BOOLEAN, ARRAY_OF_STRING, ARRAY_OF_NUMBER, ARRAY_OF_DATE, ARRAY_OF_DATE_TIME, ARRAY_OF_RELATION, ARRAY_OF_FILE ]
-          description: |
-            Datatype of this attribute. The enum of datatypes is in flux and will change. Please
-            comment at https://docs.google.com/document/d/1d352ZoN5kEYWPjy0NqqWGxdf7HEu5VEdrLmiAv7dMmQ/edit#heading=h.naxag0augkgf.
+          $ref: '#/components/schemas/AttributeDataType'
         relatesTo:
           type: string
           description: Name of type to which this attribute relates. Only present if this is a relation attribute.
     AttributeSchemaUpdate:
       type: object
-      required:
-        - name
       properties:
         name:
           type: string
           description: new name of this attribute.
+        datatype:
+          $ref: '#/components/schemas/AttributeDataType'
     BackupJob:
       x-all-of-name: BackupJob
       allOf:

service/src/test/java/org/databiosphere/workspacedataservice/controller/RecordControllerMockMvcTest.java

@@ -2096,6 +2096,104 @@ void renameAttributeConflict() throws Exception {
         .andExpect(status().isConflict());
   }
 
+  @Test
+  @Transactional
+  void updateAttributeDataType() throws Exception {
+    // Arrange
+    String recordType = "recordType";
+    createSomeRecords(recordType, 3);
+    // createSomeRecords puts floats in attr2.
+    String attributeToUpdate = "attr2";
+
+    MvcResult initialGetSchemaResult =
+        mockMvc
+            .perform(get("/{instanceId}/types/{v}/{type}", instanceId, versionId, recordType))
+            .andReturn();
+    RecordTypeSchema initialRecordTypeSchema =
+        mapper.readValue(
+            initialGetSchemaResult.getResponse().getContentAsString(), RecordTypeSchema.class);
+    AttributeSchema initialAttributeSchema =
+        initialRecordTypeSchema.getAttributeSchema(attributeToUpdate);
+    assertEquals("NUMBER", initialAttributeSchema.datatype());
+
+    // Act
+    MvcResult updateAttributeDataTypeResult =
+        mockMvc
+            .perform(
+                patch(
+                        "/{instanceId}/types/{v}/{type}/{attribute}",
+                        instanceId,
+                        versionId,
+                        recordType,
+                        attributeToUpdate)
+                    .contentType(MediaType.APPLICATION_JSON)
+                    .content(mapper.writeValueAsString(new AttributeSchema(null, "STRING"))))
+            .andExpect(status().isOk())
+            .andReturn();
+
+    // Assert
+    AttributeSchema updatedAttributeSchema =
+        mapper.readValue(
+            updateAttributeDataTypeResult.getResponse().getContentAsString(),
+            AttributeSchema.class);
+    assertEquals("STRING", updatedAttributeSchema.datatype());
+
+    MvcResult finalGetSchemaResult =
+        mockMvc
+            .perform(get("/{instanceId}/types/{v}/{type}", instanceId, versionId, recordType))
+            .andReturn();
+    RecordTypeSchema finalRecordTypeSchema =
+        mapper.readValue(
+            finalGetSchemaResult.getResponse().getContentAsString(), RecordTypeSchema.class);
+    AttributeSchema finalAttributeSchema =
+        finalRecordTypeSchema.getAttributeSchema(attributeToUpdate);
+    assertEquals("STRING", finalAttributeSchema.datatype());
+  }
+
+  @Test
+  @Transactional
+  void updateAttributeDataTypePrimaryKey() throws Exception {
+    String recordType = "recordType";
+    createSomeRecords(recordType, 3);
+    // sys_name is the default name for the primary key column used by createSomeRecords.
+    String attributeToUpdate = "sys_name";
+
+    mockMvc
+        .perform(
+            patch(
+                    "/{instanceId}/types/{v}/{type}/{attribute}",
+                    instanceId,
+                    versionId,
+                    recordType,
+                    attributeToUpdate)
+                .contentType(MediaType.APPLICATION_JSON)
+                .content(mapper.writeValueAsString(new AttributeSchema(null, "NUMBER", null))))
+        .andExpect(status().isBadRequest());
+  }
+
+  @Test
+  @Transactional
+  void updateAttributeDataTypeInvalidDataType() throws Exception {
+    String recordType = "recordType";
+    createSomeRecords(recordType, 3);
+    // createSomeRecords creates records with attributes including "attr1".
+    String attributeToUpdate = "attr1";
+
+    mockMvc
+        .perform(
+            patch(
+                    "/{instanceId}/types/{v}/{type}/{attribute}",
+                    instanceId,
+                    versionId,
+                    recordType,
+                    attributeToUpdate)
+                .contentType(MediaType.APPLICATION_JSON)
+                .content(
+                    mapper.writeValueAsString(
+                        new AttributeSchema(null, "INVALID_DATA_TYPE", null))))
+        .andExpect(status().isBadRequest());
+  }
+
   @Test
   @Transactional
   void deleteAttribute() throws Exception {