Merge branch 'master' of github.com:openmrs/openmrs-module-webservice…

…s.rest into RESTWS-920 # Conflicts: # omod-2.5/pom.xml
openmrs · Sep 20, 2024 · a1f0521 · a1f0521
2 parents ceff5c5 + eb0b5d7
commit a1f0521
Show file tree

Hide file tree

Showing 27 changed files with 817 additions and 109 deletions.
diff --git a/...java/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/ObsResource1_8.java b/...java/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/ObsResource1_8.java
@@ -26,6 +26,7 @@
 import org.openmrs.ConceptNumeric;
 import org.openmrs.Drug;
 import org.openmrs.Encounter;
+import org.openmrs.Location;
 import org.openmrs.Obs;
 import org.openmrs.Patient;
 import org.openmrs.api.APIException;
@@ -35,6 +36,7 @@
 import org.openmrs.module.webservices.rest.web.ConversionUtil;
 import org.openmrs.module.webservices.rest.web.RequestContext;
 import org.openmrs.module.webservices.rest.web.RestConstants;
+import org.openmrs.module.webservices.rest.web.RestUtil;
 import org.openmrs.module.webservices.rest.web.annotation.PropertyGetter;
 import org.openmrs.module.webservices.rest.web.annotation.PropertySetter;
 import org.openmrs.module.webservices.rest.web.annotation.Resource;
@@ -315,6 +317,7 @@ public Object getValue(Obs obs) throws ConversionException {
 					return Context.getLocationService().getLocation(new Integer(obs.getValueText()));
 				}
 				catch (NumberFormatException e) {
+					// TDOO; we really shouldn't be supporting two ways of storing a location obs, should only use the location id
 					return Context.getLocationService().getLocationByUuid(obs.getValueText());
 				}
 			} else {
@@ -394,23 +397,28 @@ public static void setConcept(Obs obs, Object value) {
 	@PropertySetter("value")
 	public static void setValue(Obs obs, Object value) throws ParseException, ConversionException, IOException {
 		if (value != null) {
+
+			// special case for complex obs
 			if (obs.isComplex()) {
 				byte[] bytes = DatatypeConverter.parseBase64Binary(value.toString());
-				
+
 				ComplexData complexData = new ComplexData(obs.getUuid() + ".raw", new ByteArrayInputStream(bytes));
 				obs.setComplexData(complexData);
-			} else if (obs.getConcept().getDatatype().isCoded()) {
-				// setValueAsString is not implemented for coded obs (in core)
-
-				//We want clients to be able to fetch a coded value in one rest call
-				//and set the returned payload as the obs value
+				return;
+			}
+
+			// special case for data type coded (setValueAsString is not implemented for coded obs (in core))
+			if (obs.getConcept().getDatatype().isCoded()) {
+				// We want clients to be able to fetch a coded value in one rest call
+				// and set the returned payload as the obs value
+				// (ie support setting based on posting the entire REST rep or just the concept uuid)
 				if (value instanceof Map) {
 					Object uuid = ((Map) value).get(RestConstants.PROPERTY_UUID);
 					if (uuid != null) {
 						value = uuid.toString();
 					}
 				}
-				
+
 				Concept valueCoded = (Concept) ConversionUtil.convert(value, Concept.class);
 				if (valueCoded == null) {
 					//try checking if this this is value drug
@@ -421,58 +429,86 @@ public static void setValue(Obs obs, Object value) throws ParseException, Conver
 					} else {
 						throw new ObjectNotFoundException(obs.getConcept().getName().getName() + ":" + value.toString());
 					}
-					
+
 				} else {
 					obs.setValueCoded(valueCoded);
 				}
-
-			} else {
-				if (obs.getConcept().isNumeric()) {
-					//get the actual persistent object rather than the hibernate proxy
-					ConceptNumeric concept = Context.getConceptService().getConceptNumeric(obs.getConcept().getId());
-					String units = concept.getUnits();
-					if (StringUtils.isNotBlank(units)) {
-						String originalValue = value.toString().trim();
-						if (originalValue.endsWith(units))
-							value = originalValue.substring(0, originalValue.indexOf(units)).trim();
-						else {
-							//check that that this value has no invalid units
-							try {
-								Double.parseDouble(originalValue);
-							}
-							catch (NumberFormatException e) {
-								throw new APIException(originalValue + " has invalid units", e);
-							}
+				return;
+			}
+
+			// special case for Location
+			String potentialLocationUuid = null;
+
+			// if this is a representation of an object, get the uuid property as potential location uuid
+			if (value instanceof Map) {
+				Object uuid = ((Map) value).get(RestConstants.PROPERTY_UUID);
+				if (uuid != null) {
+					potentialLocationUuid = uuid.toString();
+				}
+			}
+			else {
+				// otherwise, we will test if the value itself is a location uuid
+				potentialLocationUuid = value.toString();
+			}
+
+			// if there is a potential uuid, see if there is a matching location, and,if so, set the value text as the primary key
+			if (RestUtil.isValidUuid(potentialLocationUuid)) {
+				Location location = Context.getLocationService().getLocationByUuid(potentialLocationUuid);
+				if (location != null) {
+					obs.setValueText(location.getLocationId().toString());
+					obs.setComment("org.openmrs.Location");
+					return;
+				}
+			}
+
+			// handle all other types using obs.setValueAsString after special conversions for numeric and boolean
+			if (obs.getConcept().isNumeric()) {
+				//get the actual persistent object rather than the hibernate proxy
+				ConceptNumeric concept = Context.getConceptService().getConceptNumeric(obs.getConcept().getId());
+				String units = concept.getUnits();
+				if (StringUtils.isNotBlank(units)) {
+					String originalValue = value.toString().trim();
+					if (originalValue.endsWith(units))
+						value = originalValue.substring(0, originalValue.indexOf(units)).trim();
+					else {
+						//check that this value has no invalid units
+						try {
+							Double.parseDouble(originalValue);
+						}
+						catch (NumberFormatException e) {
+							throw new APIException(originalValue + " has invalid units", e);
 						}
 					}
-				} else if (obs.getConcept().getDatatype().isBoolean()) {
-					if (value instanceof Concept) {
-						value = ((Concept) value).getUuid();
+				}
+			} else if (obs.getConcept().getDatatype().isBoolean()) {
+				if (value instanceof Concept) {
+					value = ((Concept) value).getUuid();
+				}
+				if (value.equals(Context.getConceptService().getTrueConcept().getUuid())) {
+					value = true;
+				} else if (value.equals(Context.getConceptService().getFalseConcept().getUuid())) {
+					value = false;
+				} else if (!value.getClass().isAssignableFrom(Boolean.class)) {
+					List<String> trueValues = Arrays.asList("true", "1", "on", "yes");
+					List<String> falseValues = Arrays.asList("false", "0", "off", "no");
+
+					String val = value.toString().trim().toLowerCase();
+					if (trueValues.contains(val)) {
+						value = Boolean.TRUE;
+					} else if (falseValues.contains(val)) {
+						value = Boolean.FALSE;
 					}
-					if (value.equals(Context.getConceptService().getTrueConcept().getUuid())) {
-						value = true;
-					} else if (value.equals(Context.getConceptService().getFalseConcept().getUuid())) {
-						value = false;
-					} else if (!value.getClass().isAssignableFrom(Boolean.class)) {
-						List<String> trueValues = Arrays.asList("true", "1", "on", "yes");
-						List<String> falseValues = Arrays.asList("false", "0", "off", "no");
-
-						String val = value.toString().trim().toLowerCase();
-						if (trueValues.contains(val)) {
-							value = Boolean.TRUE;
-						} else if (falseValues.contains(val)) {
-							value = Boolean.FALSE;
-						}
-
-						if (!(Boolean.TRUE.equals(value) || Boolean.FALSE.equals(value))) {
-							throw new ConversionException("Unexpected value: " + value + " set as the value of boolean. "
-							        + trueValues + falseValues + ", ConceptService.getTrueConcept or "
-							        + ", ConceptService.getFalseConcept expected");
-						}
+
+					if (!(Boolean.TRUE.equals(value) || Boolean.FALSE.equals(value))) {
+						throw new ConversionException("Unexpected value: " + value + " set as the value of boolean. "
+								+ trueValues + falseValues + ", ConceptService.getTrueConcept or "
+								+ ", ConceptService.getFalseConcept expected");
 					}
 				}
-				obs.setValueAsString(value.toString());
 			}
+			obs.setValueAsString(value.toString());
+
+
 		} else {
 			throw new APIException("The value for an observation cannot be null");
 		}

diff --git a/...rs/module/webservices/rest/web/v1_0/resource/openmrs1_8/PatientIdentifierResource1_8.java b/...rs/module/webservices/rest/web/v1_0/resource/openmrs1_8/PatientIdentifierResource1_8.java
@@ -231,10 +231,10 @@ public PatientIdentifier save(PatientIdentifier delegate) {
 		}
 
 		if (needToAdd) {
+			service().savePatientIdentifier(delegate);
 			delegate.getPatient().addIdentifier(delegate);
 		}
-
-		service().savePatientIdentifier(delegate);
+
 		return delegate;
 
 	}

diff --git a/...nmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/PersonAttributeResource1_8.java b/...nmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/PersonAttributeResource1_8.java
@@ -15,7 +15,7 @@
 import io.swagger.models.properties.RefProperty;
 import io.swagger.models.properties.StringProperty;
 import org.openmrs.Attributable;
-import org.openmrs.Concept;
+import org.openmrs.Location;
 import org.openmrs.Person;
 import org.openmrs.PersonAttribute;
 import org.openmrs.PersonAttributeType;
@@ -24,6 +24,7 @@
 import org.openmrs.module.webservices.rest.web.ConversionUtil;
 import org.openmrs.module.webservices.rest.web.RequestContext;
 import org.openmrs.module.webservices.rest.web.RestConstants;
+import org.openmrs.module.webservices.rest.web.RestUtil;
 import org.openmrs.module.webservices.rest.web.annotation.PropertyGetter;
 import org.openmrs.module.webservices.rest.web.annotation.PropertySetter;
 import org.openmrs.module.webservices.rest.web.annotation.Resource;
@@ -37,6 +38,7 @@
 import org.openmrs.module.webservices.rest.web.response.ConversionException;
 import org.openmrs.module.webservices.rest.web.response.ResponseException;
 import org.openmrs.util.OpenmrsClassLoader;
+import java.util.UUID;
 
 /**
  * {@link Resource} for PersonAttributes, supporting standard CRUD operations
@@ -96,9 +98,17 @@ public void setHydratedObject(PersonAttribute personAttribute, String attributab
 			throw new APIException("Could not convert value to Attributable", e);
 		}
 	}
-	
+
 	@PropertySetter("value")
 	public void setValue(PersonAttribute personAttribute, String value) {
+		if (RestUtil.isValidUuid(value)) {
+			Location location = Context.getLocationService().getLocationByUuid(value);
+			if (location != null) {
+				personAttribute.setValue(location.getUuid());
+				return;
+			}
+		}
+
 		PersonAttributeType attributeType = personAttribute.getAttributeType();
 		if (attributeType == null) {
 			personAttribute.setValue(value);
@@ -297,10 +307,10 @@ public String getDisplayString(PersonAttribute pa) {
 		String value = pa.toString();
 		return value == null ? "" : value;
 	}
-	
+
 	/**
 	 * Gets the hydrated object of person attribute.
-	 * 
+	 *
 	 * @param pa the person attribute.
 	 * @return an object containing the hydrated object.
 	 */
@@ -310,8 +320,7 @@ public Object getValue(PersonAttribute pa) {
 		if (value == null) {
 			return null;
 		}
-		
+
 		return ConversionUtil.convertToRepresentation(value, Representation.REF);
 	}
-
 }
diff --git a/...g/openmrs/module/webservices/rest/web/v1_0/search/openmrs1_8/ConceptSearchHandler1_8.java b/...g/openmrs/module/webservices/rest/web/v1_0/search/openmrs1_8/ConceptSearchHandler1_8.java
@@ -20,6 +20,7 @@
 import org.openmrs.api.ConceptService;
 import org.openmrs.module.webservices.rest.web.RequestContext;
 import org.openmrs.module.webservices.rest.web.RestConstants;
+import org.openmrs.module.webservices.rest.web.RestUtil;
 import org.openmrs.module.webservices.rest.web.resource.api.PageableResult;
 import org.openmrs.module.webservices.rest.web.resource.api.SearchConfig;
 import org.openmrs.module.webservices.rest.web.resource.api.SearchHandler;
@@ -89,7 +90,7 @@ public PageableResult search(RequestContext context) throws ResponseException {
 					continue;
 				}
 				// handle UUIDs
-				if (isValidUuid(conceptReference)) {
+				if (RestUtil.isValidUuid(conceptReference)) {
 					Concept concept = conceptService.getConceptByUuid(conceptReference);
 					if (concept != null) {
 						concepts.add(concept);
@@ -185,8 +186,4 @@ public PageableResult search(RequestContext context) throws ResponseException {
 			return new NeedsPaging<Concept>(conceptsByMapping, context);
 		}
 	}
-
-	private static boolean isValidUuid(String uuid) {
-		return uuid != null && (uuid.length() == 36 || uuid.length() == 38 || uuid.indexOf(' ') < 0 || uuid.indexOf('.') < 0);
-	}
 }
diff --git a/...bservices/rest/web/v1_0/controller/openmrs1_8/PatientIdentifierTypeController1_8Test.java b/...bservices/rest/web/v1_0/controller/openmrs1_8/PatientIdentifierTypeController1_8Test.java
@@ -168,4 +168,21 @@ public void shouldReturnTheAuditInfoForTheFullRepresentation() throws Exception
 
 		assertNotNull(PropertyUtils.getProperty(result, "auditInfo"));
 	}
+
+	@Test
+	public void shouldReturnCustomRepresentationForAuditInfo() throws Exception {
+		SimpleObject result = deserialize(
+				handle(newGetRequest(getURI() + "/" + getUuid(),
+						new Parameter("v", "custom:(auditInfo:(creator:(uuid),dateCreated))"))));
+
+		assertNotNull(PropertyUtils.getProperty(result, "auditInfo"));
+		assertNotNull(PropertyUtils.getProperty(result, "auditInfo.creator"));
+		assertNotNull(PropertyUtils.getProperty(result, "auditInfo.creator.uuid"));
+		assertNotNull(PropertyUtils.getProperty(result, "auditInfo.dateCreated"));
+
+		assertNull(PropertyUtils.getProperty(result, "name"));
+		assertNull(PropertyUtils.getProperty(result, "links"));
+		assertNull(PropertyUtils.getProperty(result, "auditInfo.creator.display"));
+		assertNull(PropertyUtils.getProperty(result, "auditInfo.creator.links"));
+	}
 }
diff --git a/.../module/webservices/rest/web/v1_0/resource/openmrs1_8/PersonAttributeResource1_8Test.java b/.../module/webservices/rest/web/v1_0/resource/openmrs1_8/PersonAttributeResource1_8Test.java
@@ -34,10 +34,11 @@
 import static org.hamcrest.Matchers.equalToIgnoringCase;
 
 public class PersonAttributeResource1_8Test extends BaseModuleWebContextSensitiveTest {
-
-	public static final String PERSON_ATTRIBUTE_JSON = "{" + "    \"value\": \"Bangalore\"," + "    \"attributeType\": {"
-	        + "        \"uuid\": \"54fc8400-1683-4d71-a1ac-98d40836ff7c\"" + "    }" + "}";
-
+
+	public static final String PERSON_ATTRIBUTE_JSON = "{\"value\": \"Bangalore\", \"uuid\": \"592349ed-d012-4552-a274-d5d8e73b9401\", \"attributeType\": { \"uuid\": \"54fc8400-1683-4d71-a1ac-98d40836ff7c\" }}";
+
+	public static final String PERSON_ATTRIBUTE_JSON_WITHOUT_UUID = "{\"value\": \"Bangalore\", \"uuid\": \"\", \"attributeType\": { \"uuid\": \"54fc8400-1683-4d71-a1ac-98d40836ff7c\" }}";
+
 	private SimpleObject personAttributeSimpleObject = new SimpleObject();
 
 	private PersonAttributeResource1_8 resource;
@@ -65,6 +66,14 @@ public void shouldCreatePersonAttribute() throws Exception {
 		Assert.assertEquals("Bangalore", created.get("value"));
 	}
 
+	@Test
+	public void getValue_shouldFallBackToHydratedObjectWhenUuidIsEmpty() throws Exception {
+		personAttributeSimpleObject.putAll(new ObjectMapper().readValue(PERSON_ATTRIBUTE_JSON_WITHOUT_UUID, HashMap.class));
+		SimpleObject created = (SimpleObject) resource.create("da7f524f-27ce-4bb2-86d6-6d1d05312bd5",
+				personAttributeSimpleObject, new RequestContext());
+		Assert.assertEquals("Bangalore", created.get("value"));
+	}
+
 	@Test
 	public void getDisplayString_shouldGetDisplayStringForConcept() {
 		// arrange
@@ -138,11 +147,9 @@ public void setValue_shouldSetProperAttributableIdIfFound() {
 
 		PersonAttribute attribute = new PersonAttribute(type, null);
 		attribute.setAttributeType(type);
-
 		Assert.assertNull(attribute.getValue());
 
 		resource.setValue(attribute, location.getUuid());
-
 		Assert.assertEquals(location.getUuid(), attribute.getValue());
 	}
 

diff --git a/...module/webservices/rest/web/v1_0/controller/openmrs1_9/ConceptReferenceController1_9.java b/...module/webservices/rest/web/v1_0/controller/openmrs1_9/ConceptReferenceController1_9.java
@@ -61,7 +61,7 @@ public Object search(HttpServletRequest request, HttpServletResponse response) {
 					continue;
 				}
 				// handle UUIDs
-				if (isValidUuid(conceptReference)) {
+				if (RestUtil.isValidUuid(conceptReference)) {
 					Concept concept = conceptService.getConceptByUuid(conceptReference);
 					if (concept != null) {
 						addResult(results, conceptReference, concept, requestContext.getRepresentation());
@@ -95,8 +95,5 @@ private void addResult(SimpleObject results, String conceptReference, Concept co
 				ConversionUtil.convertToRepresentation(concept, rep == null ? new DefaultRepresentation() : rep));
 	}
 
-	private static boolean isValidUuid(String uuid) {
-		return uuid != null && (uuid.length() == 36 || uuid.length() == 38 || uuid.indexOf(' ') < 0
-				|| uuid.indexOf('.') < 0);
-	}
+
 }