Merge pull request #483 from CMSgov/story-537-merged

QPPCT-537: Efficient DynamoDB querying

rest-api/src/main/java/gov/cms/qpp/conversion/api/controllers/v1/CpcFileControllerV1.java

@@ -3,6 +3,7 @@
 import gov.cms.qpp.conversion.api.model.Constants;
 import gov.cms.qpp.conversion.api.model.UnprocessedCpcFileData;
 import gov.cms.qpp.conversion.api.services.CpcFileService;
+
 import gov.cms.qpp.conversion.util.EnvironmentHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;

rest-api/src/main/java/gov/cms/qpp/conversion/api/helper/MetadataHelper.java

@@ -1,5 +1,6 @@
 package gov.cms.qpp.conversion.api.helper;
 
+import gov.cms.qpp.conversion.api.model.Constants;
 import gov.cms.qpp.conversion.api.model.Metadata;
 import gov.cms.qpp.conversion.decode.ClinicalDocumentDecoder;
 import gov.cms.qpp.conversion.model.Node;
@@ -9,13 +10,20 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
+import java.util.concurrent.ThreadLocalRandom;
 
 /**
  * Utilities for working with Metadata beans
  */
 public class MetadataHelper {
 
+	private static final ThreadLocalRandom RANDOM_HASH = ThreadLocalRandom.current();
+
+	/**
+	 * No need for constructor in this utility class
+	 */
 	private MetadataHelper() {
+		//empty
 	}
 
 	/**
@@ -35,7 +43,7 @@ public static Metadata generateMetadata(Node node, Outcome outcome) {
 			metadata.setApm(findApm(node));
 			metadata.setTin(findTin(node));
 			metadata.setNpi(findNpi(node));
-			metadata.setCpc(isCpc(node));
+			metadata.setCpc(deriveCpcHash(node));
 			metadata.setCpcProcessed(false);
 		}
 
@@ -44,20 +52,55 @@ public static Metadata generateMetadata(Node node, Outcome outcome) {
 		return metadata;
 	}
 
+	/**
+	 * Retrieves the random hash for the Cpc field if this is a CPC+ conversion.
+	 *
+	 * @return Cpc field randomly hashed or null if this isn't a CPC+ conversion
+	 */
+	private static String deriveCpcHash(Node node) {
+		String cpcHash = null;
+
+		if (isCpc(node)) {
+			cpcHash = Constants.CPC_DYNAMO_PARTITION_START + RANDOM_HASH.nextInt(Constants.CPC_DYNAMO_PARTITIONS);
+		}
+
+		return cpcHash;
+	}
+
+	/**
+	 * Retrieves the APM Entity Id from the given node
+	 *
+	 * @return Apm Entity ID value
+	 */
 	private static String findApm(Node node) {
 		return findValue(node, ClinicalDocumentDecoder.ENTITY_ID, TemplateId.CLINICAL_DOCUMENT);
 	}
 
+	/**
+	 * Retrieves the Taxpayer Identification Number from the given node
+	 *
+	 * @return TIN value
+	 */
 	private static String findTin(Node node) {
 		return findValue(node, ClinicalDocumentDecoder.TAX_PAYER_IDENTIFICATION_NUMBER,
 				TemplateId.CLINICAL_DOCUMENT);
 	}
 
+	/**
+	 * Retrieves the National Provider Identifier from the given node
+	 *
+	 * @return NPI value
+	 */
 	private static String findNpi(Node node) {
 		return findValue(node, ClinicalDocumentDecoder.NATIONAL_PROVIDER_IDENTIFIER,
 				TemplateId.CLINICAL_DOCUMENT);
 	}
 
+	/**
+	 * Retrieves the random hash for CPC Field
+	 *
+	 * @return Cpc field randomly hashed
+	 */
 	private static boolean isCpc(Node node) {
 		if (Program.isCpc(node)) {
 			return true;
@@ -87,6 +130,15 @@ private static String findValue(Node node, String key, TemplateId... possibleLoc
 		return found == null ? null : found.getValue(key);
 	}
 
+	/**
+	 * Finds all possible children within the given node for each {@link TemplateId} given
+	 * filtered by children with specific keys
+	 *
+	 * @param node Object to search through
+	 * @param key value to filter
+	 * @param possibleLocations areas which the child can exist
+	 * @return A child node with the correct value or null
+	 */
 	private static Node findPossibleChildNode(Node node, String key, TemplateId... possibleLocations) {
 		return Arrays.stream(possibleLocations)
 			.distinct()

rest-api/src/main/java/gov/cms/qpp/conversion/api/model/Constants.java

@@ -14,6 +14,10 @@ public class Constants {
 	public static final String USE_SYNC_EXECUTOR = "USE_SYNC_EXECUTOR";
 	public static final String NO_CPC_PLUS_API_ENV_VARIABLE = "NO_CPC_PLUS_API";
 	public static final String V1_API_ACCEPT = "application/vnd.qpp.cms.gov.v1+json";
+	public static final Integer CPC_DYNAMO_PARTITIONS = 32;
+	public static final String CPC_DYNAMO_PARTITION_START = "CPC_";
+	public static final String DYNAMO_CPC_ATTRIBUTE = "Cpc";
+	public static final String DYNAMO_CPC_PROCESSED_CREATE_DATE_ATTRIBUTE = "CpcProcessed_CreateDate";
 
 	/**
 	 * Library utility class so the constructor is private and empty.

rest-api/src/main/java/gov/cms/qpp/conversion/api/model/Metadata.java

@@ -1,12 +1,14 @@
 package gov.cms.qpp.conversion.api.model;
 
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBAttribute;
-import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBAutoGenerateStrategy;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBAutoGeneratedKey;
-import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBAutoGeneratedTimestamp;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBHashKey;
+import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBIgnore;
 import com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBTable;
 import com.amazonaws.services.dynamodbv2.datamodeling.encryption.DoNotEncrypt;
+
+import java.time.Instant;
+import java.time.format.DateTimeFormatter;
 import java.util.Date;
 import java.util.Objects;
 
@@ -15,24 +17,36 @@
  */
 @DynamoDBTable(tableName = "ConversionMetadata")
 public final class Metadata {
+	private static final int CPC_PROCESSED_CREATE_DATE_NUM_FIELDS = 2;
+	private static final int CPC_PROCESSED_INDEX = 0;
+	private static final int CPC_CREATE_DATE_INDEX = 1;
+
 	private String uuid;
 	private String tin;  //this field is encrypted
 	private String npi;
-	private Date createdDate;
 	private String apm;
 	private Long submissionYear;
 	private String submissionLocator;
 	private String qppLocator;
-	private String fileName;
+	private String fileName;  //this field is encrypted
 	private Boolean overallStatus;
 	private Boolean conversionStatus;
 	private Boolean validationStatus;
-	private Boolean cpc;
+	private String cpc;
 	private String conversionErrorLocator;
 	private String validationErrorLocator;
 	private String rawValidationErrorLocator;
+
+	private Date createdDate;
 	private Boolean cpcProcessed;
 
+	/**
+	 * Constructs a new {@code Metadata} with the {@code createdDate} filled in upon construction.
+	 */
+	public Metadata() {
+		createdDate = new Date();
+	}
+
 
 	/**
 	 * The UUID that uniquely identifies this item.
@@ -61,7 +75,6 @@ public void setUuid(String uuid) {
 	 */
 	@DoNotEncrypt
 	@DynamoDBAttribute(attributeName = "CreateDate")
-	@DynamoDBAutoGeneratedTimestamp(strategy = DynamoDBAutoGenerateStrategy.CREATE)
 	public Date getCreatedDate() {
 		Date returnedDate = null;
 
@@ -290,20 +303,26 @@ public void setValidationStatus(Boolean validationStatus) {
 	/**
 	 * Whether the conversion was for the CPC+ program.
 	 *
-	 * @return True for a CPC+ conversion, false otherwise.
+	 * This is set to a {@link String} that contains "CPC_" plus a number for DynamoDB partitioning of the GSI.
+	 * If this method returns {@code null}, this was not a CPC+ conversion.
+	 *
+	 * @return A {@link String} for a CPC+ conversion, null otherwise.
 	 */
 	@DoNotEncrypt
-	@DynamoDBAttribute(attributeName = "Cpc")
-	public Boolean getCpc() {
+	@DynamoDBAttribute(attributeName = Constants.DYNAMO_CPC_ATTRIBUTE)
+	public String getCpc() {
 		return cpc;
 	}
 
 	/**
 	 * Sets whether the conversion was for the CPC+ program.
 	 *
+	 * If not {@code null}, must be of the form "CPC_" plus a number.
+	 * Setting this to {@code null}, indicates this was not a CPC+ conversion.
+	 *
 	 * @param cpc A CPC+ conversion or not.
 	 */
-	public void setCpc(Boolean cpc) {
+	public void setCpc(String cpc) {
 		this.cpc = cpc;
 	}
 
@@ -376,10 +395,11 @@ public void setRawValidationErrorLocator(String rawValidationErrorLocator) {
 	/**
 	 * Whether the file was processed by the CPC+ team
 	 *
-	 * @return
+	 * Ignored when writing to DynamoDB because {@code CpcProcessed_CreateDate} holds the pertinent information.
+	 *
+	 * @return Whether the file was processed.
 	 */
-	@DoNotEncrypt
-	@DynamoDBAttribute(attributeName = "CpcProcessed")
+	@DynamoDBIgnore
 	public Boolean getCpcProcessed() {
 		return cpcProcessed;
 	}
@@ -393,6 +413,49 @@ public void setCpcProcessed(Boolean cpcProcessed) {
 		this.cpcProcessed = cpcProcessed;
 	}
 
+	/**
+	 * Returns an attribute that combines the CPC+ processed state and the date of creation.
+	 *
+	 * This is mostly useful in the CPC+ global secondary index.
+	 *
+	 * @return The combined attribute.
+	 */
+	@DoNotEncrypt
+	@DynamoDBAttribute(attributeName = Constants.DYNAMO_CPC_PROCESSED_CREATE_DATE_ATTRIBUTE)
+	public String getCpcProcessedCreateDate() {
+		String combination = null;
+
+		if (cpcProcessed != null) {
+			combination = cpcProcessed.toString() + "#" + DateTimeFormatter.ISO_INSTANT.format(createdDate.toInstant());
+		}
+
+		return combination;
+	}
+
+	/**
+	 * Sets the separate CPC+ processed flag and created date based on the argument
+	 *
+	 * Splits the the processed flag from the date by a {@code #} character.
+	 * The first field must be {@code true} or {@code false} which represents the CPC+ processed boolean.
+	 * The second field must be an ISO 8601 timestamp string.  For example, {@code 2017-12-08T18:32:54.846Z}.
+	 *
+	 * @param combination The combined attribute.
+	 */
+	public void setCpcProcessedCreateDate(String combination) {
+
+		String[] split = combination.split("#");
+
+		if (split.length < CPC_PROCESSED_CREATE_DATE_NUM_FIELDS) {
+			return;
+		}
+
+		String isProcessed = split[CPC_PROCESSED_INDEX];
+		String creationDate = split[CPC_CREATE_DATE_INDEX];
+
+		setCpcProcessed(Boolean.valueOf(isProcessed));
+		setCreatedDate(Date.from(Instant.parse(creationDate)));
+	}
+
 	/**
 	 * Determines the equality between this object and another.
 	 *

rest-api/src/main/java/gov/cms/qpp/conversion/api/model/UnprocessedCpcFileData.java

@@ -1,5 +1,7 @@
 package gov.cms.qpp.conversion.api.model;
 
+import com.google.common.base.MoreObjects;
+
 import java.util.Date;
 
 /**
@@ -70,4 +72,14 @@ public Boolean getValidationSuccess() {
 		return validationSuccess;
 	}
 
+	@Override
+	public String toString() {
+		return MoreObjects.toStringHelper(this)
+			.add("fileId", fileId)
+			.add("filename", "*could hold PII*")
+			.add("apm", apm)
+			.add("conversionDate", conversionDate)
+			.add("validationSuccess", validationSuccess)
+			.toString();
+	}
 }

rest-api/src/main/java/gov/cms/qpp/conversion/api/services/AuditService.java

@@ -3,8 +3,12 @@
 
 import gov.cms.qpp.conversion.Converter.ConversionReport;
 
+import gov.cms.qpp.conversion.api.model.Metadata;
 import java.util.concurrent.CompletableFuture;
 
+/**
+ * Service Interface for auditing {@link Metadata} by {@link ConversionReport}
+ */
 public interface AuditService {
 	/**
 	 * Report a successful conversion.

rest-api/src/main/java/gov/cms/qpp/conversion/api/services/AuditServiceImpl.java

@@ -18,6 +18,9 @@
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 
+/**
+ * Service for storing {@link Metadata} by {@link Converter.ConversionReport} outcome
+ */
 @Service
 public class AuditServiceImpl implements AuditService {
 	private static final Logger API_LOG = LoggerFactory.getLogger(Constants.API_LOG);
@@ -110,9 +113,9 @@ public CompletableFuture<Void> failValidation(Converter.ConversionReport convers
 	}
 
 	/**
-	 * Determines whether the no audit environment variable is set.
+	 * Determines if the No Audit Environment variable was passed
 	 *
-	 * @return Whether to do auditing or not.
+	 * @return the status of auditing
 	 */
 	private boolean noAudit() {
 		String noAudit = environment.getProperty(Constants.NO_AUDIT_ENV_VARIABLE);
@@ -126,11 +129,11 @@ private boolean noAudit() {
 	}
 
 	/**
-	 * Creates a baseline {@link Metadata} object.
+	 * Initializes {@link Metadata} from the {@link Converter.ConversionReport} and conversion outcome
 	 *
-	 * @param report The report from a conversion.
-	 * @param outcome The high level outcome of a conversion.
-	 * @return A new {@link Metadata}.
+	 * @param report Object containing metadata information
+	 * @param outcome Status of the conversion
+	 * @return Constructed metadata
 	 */
 	private Metadata initMetadata(Converter.ConversionReport report, MetadataHelper.Outcome outcome) {
 		Metadata metadata = MetadataHelper.generateMetadata(report.getDecoded(), outcome);

rest-api/src/main/java/gov/cms/qpp/conversion/api/services/CpcFileServiceImpl.java

@@ -3,13 +3,13 @@
 import gov.cms.qpp.conversion.api.exceptions.NoFileInDatabaseException;
 import gov.cms.qpp.conversion.api.model.Metadata;
 import gov.cms.qpp.conversion.api.model.UnprocessedCpcFileData;
-import java.io.IOException;
-import java.util.List;
-import java.util.stream.Collectors;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.core.io.InputStreamResource;
 import org.springframework.stereotype.Service;
 
+import java.util.List;
+import java.util.stream.Collectors;
+
 /**
  * Service for handling Cpc File meta data
  */
@@ -41,11 +41,10 @@ public List<UnprocessedCpcFileData> getUnprocessedCpcPlusFiles() {
 	 *
 	 * @param fileId {@link Metadata} identifier
 	 * @return file contents as a {@link String}
-	 * @throws IOException
 	 */
-	public InputStreamResource getFileById(String fileId) throws IOException {
+	public InputStreamResource getFileById(String fileId) {
 		Metadata metadata = dbService.getMetadataById(fileId);
-		if (metadata != null && metadata.getCpc() && !metadata.getCpcProcessed()) {
+		if (metadata != null && metadata.getCpc() != null && !metadata.getCpcProcessed()) {
 			return new InputStreamResource(storageService.getFileByLocationId(metadata.getSubmissionLocator()));
 		} else {
 			throw new NoFileInDatabaseException(FILE_NOT_FOUND);