Do not generated UUIDs for key fields, runtime will do that. (#496)

Author: mofterdinger

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/CreateAttachmentsHandler.java

@@ -7,20 +7,16 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.UUID;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.sap.cds.CdsData;
-import com.sap.cds.CdsDataProcessor;
 import com.sap.cds.feature.attachments.handler.applicationservice.helper.ModifyApplicationHandlerHelper;
 import com.sap.cds.feature.attachments.handler.applicationservice.helper.ReadonlyDataContextEnhancer;
 import com.sap.cds.feature.attachments.handler.applicationservice.helper.ThreadDataStorageReader;
 import com.sap.cds.feature.attachments.handler.applicationservice.modifyevents.ModifyAttachmentEventFactory;
 import com.sap.cds.feature.attachments.handler.common.ApplicationHandlerHelper;
-import com.sap.cds.reflect.CdsBaseType;
-import com.sap.cds.reflect.CdsEntity;
 import com.sap.cds.services.cds.ApplicationService;
 import com.sap.cds.services.cds.CdsCreateEventContext;
 import com.sap.cds.services.handler.EventHandler;
@@ -40,7 +36,6 @@ public class CreateAttachmentsHandler implements EventHandler {
 
 	private final ModifyAttachmentEventFactory eventFactory;
 	private final ThreadDataStorageReader storageReader;
-	private final CdsDataProcessor processor = CdsDataProcessor.create();
 
 	public CreateAttachmentsHandler(ModifyAttachmentEventFactory eventFactory, ThreadDataStorageReader storageReader) {
 		this.eventFactory = requireNonNull(eventFactory, "eventFactory must not be null");
@@ -56,20 +51,10 @@ void processBeforeForDraft(CdsCreateEventContext context, List<CdsData> data) {
 	@Before
 	@HandlerOrder(HandlerOrder.LATE)
 	void processBefore(CdsCreateEventContext context, List<CdsData> data) {
-		if (ApplicationHandlerHelper.noContentFieldInData(context.getTarget(), data)) {
-			return;
+		if (ApplicationHandlerHelper.containsContentField(context.getTarget(), data)) {
+			logger.debug("Processing before {} event for entity {}", context.getEvent(), context.getTarget());
+			ModifyApplicationHandlerHelper.handleAttachmentForEntities(context.getTarget(), data, new ArrayList<>(),
+					eventFactory, context);
 		}
-
-		logger.debug("Processing before create event for entity {}", context.getTarget().getName());
-		setKeysInData(context.getTarget(), data);
-		ModifyApplicationHandlerHelper.handleAttachmentForEntities(context.getTarget(), data, new ArrayList<>(),
-				eventFactory, context);
-	}
-
-	private void setKeysInData(CdsEntity entity, List<CdsData> data) {
-		processor.addGenerator(
-				(path, element, type) -> element.isKey() && element.getType().isSimpleType(CdsBaseType.UUID),
-				(path, element, isNull) -> UUID.randomUUID().toString()).process(data, entity);
 	}
-
 }

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/DeleteAttachmentsHandler.java

@@ -44,7 +44,7 @@ public DeleteAttachmentsHandler(AttachmentsReader attachmentsReader, MarkAsDelet
 	@Before
 	@HandlerOrder(HandlerOrder.LATE)
 	void processBefore(CdsDeleteEventContext context) {
-		logger.debug("Processing before delete event for entity {}", context.getTarget().getName());
+		logger.debug("Processing before {} event for entity {}", context.getEvent(), context.getTarget());
 
 		List<Attachments> attachments = attachmentsReader.readAttachments(context.getModel(), context.getTarget(),
 				context.getCqn());

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/ReadAttachmentsHandler.java

@@ -71,7 +71,7 @@ public ReadAttachmentsHandler(AttachmentService attachmentService, AttachmentSta
 	@Before
 	@HandlerOrder(HandlerOrder.EARLY)
 	void processBefore(CdsReadEventContext context) {
-		logger.debug("Processing before read event for entity {}", context.getTarget().getName());
+		logger.debug("Processing before {} for entity {}.", context.getEvent(), context.getTarget());
 
 		CdsModel cdsModel = context.getModel();
 		List<String> fieldNames = getAttachmentAssociations(cdsModel, context.getTarget(), "", new ArrayList<>());
@@ -84,27 +84,26 @@ void processBefore(CdsReadEventContext context) {
 	@After
 	@HandlerOrder(HandlerOrder.EARLY)
 	void processAfter(CdsReadEventContext context, List<CdsData> data) {
-		if (ApplicationHandlerHelper.noContentFieldInData(context.getTarget(), data)) {
-			return;
+		if (ApplicationHandlerHelper.containsContentField(context.getTarget(), data)) {
+			logger.debug("Processing after {} event for entity {}", context.getEvent(), context.getTarget());
+
+			Converter converter = (path, element, value) -> {
+				Attachments attachment = Attachments.of(path.target().values());
+				InputStream content = attachment.getContent();
+				boolean contentExists = nonNull(content);
+				if (nonNull(attachment.getContentId()) || contentExists) {
+					verifyStatus(path, attachment, contentExists);
+					Supplier<InputStream> supplier = contentExists ? () -> content
+							: () -> attachmentService.readAttachment(attachment.getContentId());
+					return new LazyProxyInputStream(supplier, statusValidator, attachment.getStatus());
+				} else {
+					return value;
+				}
+			};
+
+			CdsDataProcessor.create().addConverter(ApplicationHandlerHelper.MEDIA_CONTENT_FILTER, converter)
+					.process(data, context.getTarget());
 		}
-		logger.debug("Processing after read event for entity {}", context.getTarget().getQualifiedName());
-
-		Converter converter = (path, element, value) -> {
-			Attachments attachment = Attachments.of(path.target().values());
-			InputStream content = attachment.getContent();
-			boolean contentExists = nonNull(content);
-			if (nonNull(attachment.getContentId()) || contentExists) {
-				verifyStatus(path, attachment, contentExists);
-				Supplier<InputStream> supplier = contentExists ? () -> content
-						: () -> attachmentService.readAttachment(attachment.getContentId());
-				return new LazyProxyInputStream(supplier, statusValidator, attachment.getStatus());
-			} else {
-				return value;
-			}
-		};
-
-		CdsDataProcessor.create().addConverter(ApplicationHandlerHelper.MEDIA_CONTENT_FILTER, converter).process(data,
-				context.getTarget());
 	}
 
 	private List<String> getAttachmentAssociations(CdsModel model, CdsEntity entity, String associationName,

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandler.java

@@ -70,21 +70,19 @@ void processBefore(CdsUpdateEventContext context, List<CdsData> data) {
 		CdsEntity target = context.getTarget();
 		boolean associationsAreUnchanged = associationsAreUnchanged(target, data);
 
-		if (ApplicationHandlerHelper.noContentFieldInData(target, data) && associationsAreUnchanged) {
-			return;
-		}
-
-		logger.debug("Processing before update event for entity {}", target.getName());
+		if (ApplicationHandlerHelper.containsContentField(target, data) || !associationsAreUnchanged) {
+			logger.debug("Processing before {} event for entity {}", context.getEvent(), target);
 
-		CqnSelect select = CqnUtils.toSelect(context.getCqn(), context.getTarget());
-		List<Attachments> attachments = attachmentsReader.readAttachments(context.getModel(), target, select);
+			CqnSelect select = CqnUtils.toSelect(context.getCqn(), context.getTarget());
+			List<Attachments> attachments = attachmentsReader.readAttachments(context.getModel(), target, select);
 
-		List<Attachments> condensedAttachments = ApplicationHandlerHelper.condenseAttachments(attachments, target);
-		ModifyApplicationHandlerHelper.handleAttachmentForEntities(target, data, condensedAttachments, eventFactory,
-				context);
+			List<Attachments> condensedAttachments = ApplicationHandlerHelper.condenseAttachments(attachments, target);
+			ModifyApplicationHandlerHelper.handleAttachmentForEntities(target, data, condensedAttachments, eventFactory,
+					context);
 
-		if (!associationsAreUnchanged) {
-			deleteRemovedAttachments(attachments, data, target, context.getUserInfo());
+			if (!associationsAreUnchanged) {
+				deleteRemovedAttachments(attachments, data, target, context.getUserInfo());
+			}
 		}
 	}
 

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/common/ApplicationHandlerHelper.java

@@ -41,11 +41,11 @@ public final class ApplicationHandlerHelper {
 	 * @param data   The data to check
 	 * @return <code>true</code> if the data contains a content field, <code>false</code> otherwise
 	 */
-	public static boolean noContentFieldInData(CdsEntity entity, List<? extends CdsData> data) {
+	public static boolean containsContentField(CdsEntity entity, List<? extends CdsData> data) {
 		AtomicBoolean isIncluded = new AtomicBoolean();
-		Validator validator = (path, element, value) -> isIncluded.set(true);
-		CdsDataProcessor.create().addValidator(MEDIA_CONTENT_FILTER, validator).process(data, entity);
-		return !isIncluded.get();
+		CdsDataProcessor.create().addValidator(MEDIA_CONTENT_FILTER, (path, element, value) -> isIncluded.set(true))
+				.process(data, entity);
+		return isIncluded.get();
 	}
 
 	/**

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/draftservice/DraftCancelAttachmentsHandler.java

@@ -59,7 +59,7 @@ public DraftCancelAttachmentsHandler(AttachmentsReader attachmentsReader,
 	@HandlerOrder(HandlerOrder.LATE)
 	void processBeforeDraftCancel(DraftCancelEventContext context) {
 		if (isWhereEmpty(context)) {
-			logger.debug("Processing before draft cancel event for entity {}", context.getTarget().getName());
+			logger.debug("Processing before {} event for entity {}", context.getEvent(), context.getTarget());
 
 			CdsEntity activeEntity = DraftUtils.getActiveEntity(context.getTarget());
 			CdsEntity draftEntity = DraftUtils.getDraftEntity(context.getTarget());

cds-feature-attachments/src/main/java/com/sap/cds/feature/attachments/handler/draftservice/DraftPatchAttachmentsHandler.java

@@ -51,7 +51,7 @@ public DraftPatchAttachmentsHandler(PersistenceService persistence, ModifyAttach
 	@Before
 	@HandlerOrder(HandlerOrder.LATE)
 	void processBeforeDraftPatch(DraftPatchEventContext context, List<? extends CdsData> data) {
-		logger.debug("Processing before draft patch event for entity {}", context.getTarget().getName());
+		logger.debug("Processing before {} event for entity {}", context.getEvent(), context.getTarget());
 
 		Converter converter = (path, element, value) -> {
 			CdsEntity draftEntity = DraftUtils.getDraftEntity(path.target().entity());

cds-feature-attachments/src/test/java/com/sap/cds/feature/attachments/handler/applicationservice/CreateAttachmentsHandlerTest.java

@@ -3,6 +3,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
@@ -90,8 +91,7 @@ void idsAreSetInDataForCreate() {
 
 		cut.processBefore(createContext, List.of(roots));
 
-		assertThat(roots.getId()).isNotEmpty();
-		assertThat(attachment.getId()).isNotEmpty();
+		verify(eventFactory).getEvent(null, null, Attachments.create());
 	}
 
 	@Test
@@ -205,16 +205,16 @@ void handlerCalledForMediaEventInAssociationIdsAreSet() {
 		events.setContent("test");
 		var items = Items.create();
 		var attachment = Attachments.create();
-		attachment.setContent(mock(InputStream.class));
+		var content = mock(InputStream.class);
+		attachment.setContent(content);
 		items.setAttachments(List.of(attachment));
 		events.setItems(List.of(items));
 		when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);
 
 		List<CdsData> input = List.of(events);
 		cut.processBefore(createContext, input);
 
-		assertThat(events.getId1()).isNotEmpty();
-		assertThat(events.getId2()).isNull();
+		verify(eventFactory).getEvent(eq(content), any(), any());
 	}
 
 	@Test