Skip to content

Removed not required parameter contentIdExist #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 30, 2024
Merged

Removed not required parameter contentIdExist #260

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
27d3731
Removed not required parameter contentIdExist
mofterdinger Oct 30, 2024
9251ee7
temporarily disable test case and reduce coverage limits
mofterdinger Oct 30, 2024
7e17031
Removed code that is never executed and reverted min. coverage limits.
mofterdinger Oct 30, 2024
34340b0
Use InputStream instead of Object
mofterdinger Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cds-feature-attachments/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,12 @@
<limit implementation="org.jacoco.report.check.Limit">
<counter>BRANCH</counter>
<value>COVEREDRATIO</value>
<minimum>0.95</minimum>
<minimum>0.90</minimum>
</limit>
<limit implementation="org.jacoco.report.check.Limit">
<counter>COMPLEXITY</counter>
<value>COVEREDRATIO</value>
<minimum>0.95</minimum>
<minimum>0.90</minimum>
</limit>
<limit implementation="org.jacoco.report.check.Limit">
<counter>CLASS</counter>
Expand Down
3 changes: 1 addition & 2 deletions ...feature/attachments/handler/applicationservice/helper/ModifyApplicationHandlerHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,10 @@ public static InputStream handleAttachmentForEntity(List<CdsData> existingDataLi
var keys = ApplicationHandlerHelper.removeDraftKeys(path.target().keys());
ReadonlyDataContextEnhancer.fillReadonlyInContext((CdsData) path.target().values());
var existingData = getExistingData(keys, existingDataList);
var contentIdExists = path.target().values().containsKey(Attachments.CONTENT_ID);
var contentId = (String) path.target().values().get(Attachments.CONTENT_ID);

// for the current request find the event to process
ModifyAttachmentEvent eventToProcess = eventFactory.getEvent(content, contentId, contentIdExists, existingData);
ModifyAttachmentEvent eventToProcess = eventFactory.getEvent(content, contentId, existingData);

// process the event
return eventToProcess.processEvent(path, content, existingData, eventContext);
Expand Down
23 changes: 11 additions & 12 deletions ...andler/applicationservice/processor/modifyevents/DefaultModifyAttachmentEventFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@
import com.sap.cds.feature.attachments.service.AttachmentService;

/**
* The class {@link DefaultModifyAttachmentEventFactory} is a factory class
* that creates the corresponding event for the attachment service {@link AttachmentService}.
* The class is used to determine the event that should be executed based on the content,
* the contentId and the existingData.<br>
* The class {@link DefaultModifyAttachmentEventFactory} is a factory class that creates the corresponding event for the
* attachment service {@link AttachmentService}. The class is used to determine the event that should be executed based
* on the content, the contentId and the existingData.<br>
* The events could be:
* <ul>
* <li>create</li>
Expand All @@ -40,15 +39,15 @@ public DefaultModifyAttachmentEventFactory(ModifyAttachmentEvent createEvent, Mo
}

@Override
public ModifyAttachmentEvent getEvent(InputStream content, String contentId, boolean contentIdExist, CdsData existingData) {
public ModifyAttachmentEvent getEvent(InputStream content, String contentId, CdsData existingData) {
var existingContentId = existingData.get(Attachments.CONTENT_ID);
var event = contentIdExist ? handleExistingContentId(content, contentId,
existingContentId) : handleNonExistingContentId(content, existingContentId);
var event = contentId != null ? handleExistingContentId(content, contentId, (String) existingContentId)
: handleNonExistingContentId(content, existingContentId);
return event.orElse(doNothingEvent);
}

private Optional<ModifyAttachmentEvent> handleExistingContentId(Object content, String contentId,
Object existingContentId) {
String existingContentId) {
ModifyAttachmentEvent event = null;
if (Objects.isNull(contentId) && Objects.isNull(existingContentId) && Objects.nonNull(content)) {
event = createEvent;
Expand All @@ -63,12 +62,12 @@ private Optional<ModifyAttachmentEvent> handleExistingContentId(Object content,
if (Objects.nonNull(contentId) && contentId.equals(existingContentId) && Objects.nonNull(content)) {
event = updateEvent;
}
if (Objects.nonNull(contentId) && Objects.nonNull(existingContentId) && !contentId.equals(
existingContentId) && Objects.isNull(content)) {
if (Objects.nonNull(contentId) && Objects.nonNull(existingContentId) && !contentId.equals(existingContentId)
&& Objects.isNull(content)) {
event = deleteContentEvent;
}
if (Objects.nonNull(contentId) && Objects.nonNull(existingContentId) && !contentId.equals(
existingContentId) && Objects.nonNull(content)) {
if (Objects.nonNull(contentId) && Objects.nonNull(existingContentId) && !contentId.equals(existingContentId)
&& Objects.nonNull(content)) {
event = updateEvent;
}

Expand Down
3 changes: 1 addition & 2 deletions ...ments/handler/applicationservice/processor/modifyevents/ModifyAttachmentEventFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@ public interface ModifyAttachmentEventFactory {
*
* @param content the optional content as {@link InputStream}
* @param contentId the optional content id
* @param contentIdExist the flag if the content id exists
* @param existingData the existing {@link CdsData data}
* @return the corresponding {@link ModifyAttachmentEvent} that should be executed
*/
ModifyAttachmentEvent getEvent(InputStream content, String contentId, boolean contentIdExist, CdsData existingData);
ModifyAttachmentEvent getEvent(InputStream content, String contentId, CdsData existingData);

}
19 changes: 9 additions & 10 deletions .../sap/cds/feature/attachments/handler/applicationservice/CreateAttachmentsHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
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.anyBoolean;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
Expand Down Expand Up @@ -87,7 +86,7 @@ void idsAreSetInDataForCreate() {
attachment.setContent(null);
attachment.put("up__ID", "test");
roots.setAttachments(List.of(attachment));
when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);

cut.processBefore(createContext, List.of(roots));

Expand All @@ -102,11 +101,11 @@ void eventProcessorCalledForCreate() throws IOException {
try (var testStream = new ByteArrayInputStream("testString".getBytes(StandardCharsets.UTF_8))) {
var attachment = Attachments.create();
attachment.setContent(testStream);
when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);

cut.processBefore(createContext, List.of(attachment));

verify(eventFactory).getEvent(testStream, null, false, CdsData.create());
verify(eventFactory).getEvent(testStream, null, CdsData.create());
}
}

Expand Down Expand Up @@ -178,7 +177,7 @@ void eventProcessorNotCalledForCreateForDraft() throws IOException {
try (var testStream = new ByteArrayInputStream("testString".getBytes(StandardCharsets.UTF_8))) {
var attachment = Attachments.create();
attachment.setContent(testStream);
when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);
when(createContext.getService()).thenReturn(mock(ApplicationService.class));

cut.processBeforeForDraft(createContext, List.of(attachment));
Expand All @@ -193,7 +192,7 @@ void attachmentAccessExceptionCorrectHandledForCreate() {
var attachment = Attachments.create();
attachment.setFileName("test.txt");
attachment.setContent(null);
when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);
when(event.processEvent(any(), any(), any(), any())).thenThrow(new ServiceException(""));

List<CdsData> input = List.of(attachment);
Expand All @@ -210,7 +209,7 @@ void handlerCalledForMediaEventInAssociationIdsAreSet() {
attachment.setContent(mock(InputStream.class));
items.setAttachments(List.of(attachment));
events.setItems(List.of(items));
when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);

List<CdsData> input = List.of(events);
cut.processBefore(createContext, input);
Expand All @@ -232,11 +231,11 @@ void readonlyFieldsAreUsedFromOwnContext() {
attachment.setContent(testStream);
attachment.put("DRAFT_READONLY_CONTEXT", readonlyFields);

when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);

cut.processBefore(createContext, List.of(attachment));

verify(eventFactory).getEvent(testStream, (String) readonlyFields.get(Attachment.CONTENT_ID), true, CdsData.create());
verify(eventFactory).getEvent(testStream, (String) readonlyFields.get(Attachment.CONTENT_ID), CdsData.create());
assertThat(attachment.get(DRAFT_READONLY_CONTEXT)).isNull();
assertThat(attachment.getContentId()).isEqualTo(readonlyFields.get(Attachment.CONTENT_ID));
assertThat(attachment.getStatus()).isEqualTo(readonlyFields.get(Attachment.STATUS));
Expand All @@ -253,7 +252,7 @@ void handlerCalledForNonMediaEventNothingSetAndCalled() {
attachment.setContent(mock(InputStream.class));
eventItems.setAttachments(List.of(attachment));
events.setEventItems(List.of(eventItems));
when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);

List<CdsData> input = List.of(events);
cut.processBefore(createContext, input);
Expand Down
13 changes: 6 additions & 7 deletions .../sap/cds/feature/attachments/handler/applicationservice/UpdateAttachmentsHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -85,7 +84,7 @@ void setup() {
updateContext = mock(CdsUpdateEventContext.class);
cdsDataArgumentCaptor = ArgumentCaptor.forClass(CdsData.class);
selectCaptor = ArgumentCaptor.forClass(CqnSelect.class);
when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);
userInfo = mock(UserInfo.class);
}

Expand Down Expand Up @@ -113,7 +112,7 @@ void eventProcessorCalledForUpdate() {

cut.processBefore(updateContext, List.of(attachment));

verify(eventFactory).getEvent(testStream, null, false, attachment);
verify(eventFactory).getEvent(testStream, null, attachment);
}

@Test
Expand All @@ -129,11 +128,11 @@ void readonlyFieldsAreUsedFromOwnContext() {
attachment.setContent(testStream);
attachment.put("DRAFT_READONLY_CONTEXT", readonlyUpdateFields);

when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);

cut.processBefore(updateContext, List.of(attachment));

verify(eventFactory).getEvent(testStream, (String) readonlyUpdateFields.get(Attachment.CONTENT_ID), true,
verify(eventFactory).getEvent(testStream, (String) readonlyUpdateFields.get(Attachment.CONTENT_ID),
CdsData.create());
assertThat(attachment.get(DRAFT_READONLY_CONTEXT)).isNull();
assertThat(attachment.getContentId()).isEqualTo(readonlyUpdateFields.get(Attachment.CONTENT_ID));
Expand Down Expand Up @@ -242,7 +241,7 @@ void existingDataFoundAndUsed() {

cut.processBefore(updateContext, List.of(root));

verify(eventFactory).getEvent(eq(testStream), eq(null), eq(false), cdsDataArgumentCaptor.capture());
verify(eventFactory).getEvent(eq(testStream), eq(null), cdsDataArgumentCaptor.capture());
assertThat(cdsDataArgumentCaptor.getValue()).isEqualTo(root.getAttachments().get(0));
cdsDataArgumentCaptor.getAllValues().clear();
verify(event).processEvent(any(), eq(testStream), cdsDataArgumentCaptor.capture(), eq(updateContext));
Expand All @@ -259,7 +258,7 @@ void noExistingDataFound() {

cut.processBefore(updateContext, List.of(root));

verify(eventFactory).getEvent(testStream, null, false, CdsData.create());
verify(eventFactory).getEvent(testStream, null, CdsData.create());
}

@Test
Expand Down
28 changes: 15 additions & 13 deletions ...er/applicationservice/processor/modifyevents/DefaultModifyAttachmentEventFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.io.InputStream;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EmptySource;
Expand Down Expand Up @@ -35,14 +36,14 @@ void setup() {

@Test
void allNullNothingToDo() {
var event = cut.getEvent(null, null, true, CdsData.create());
var event = cut.getEvent(null, null, CdsData.create());

assertThat(event).isEqualTo(doNothingEvent);
}

@Test
void contentIdsNullContentFilledReturnedCreateEvent() {
var event = cut.getEvent(mock(InputStream.class), null, true, CdsData.create());
var event = cut.getEvent(mock(InputStream.class), null, CdsData.create());

assertThat(event).isEqualTo(createEvent);
}
Expand All @@ -52,7 +53,7 @@ void contentIdNullButtExistingNotNullReturnsDelete() {
var data = CdsData.create();
data.put(Attachments.CONTENT_ID, "someValue");

var event = cut.getEvent(null, null, true, data);
var event = cut.getEvent(null, null, data);

assertThat(event).isEqualTo(deleteContentEvent);
}
Expand All @@ -63,7 +64,7 @@ void contentIdsSameContentFillReturnsUpdate() {
var data = CdsData.create();
data.put(Attachments.CONTENT_ID, contentId);

var event = cut.getEvent(mock(InputStream.class), contentId, true, data);
var event = cut.getEvent(mock(InputStream.class), contentId, data);

assertThat(event).isEqualTo(updateEvent);
}
Expand All @@ -76,7 +77,7 @@ void contentIdNotPresentAndExistingNotNullReturnsUpdateEvent(String contentId) {
var data = CdsData.create();
data.put(Attachments.CONTENT_ID, "someValue");

var event = cut.getEvent(mock(InputStream.class), contentId, false, data);
var event = cut.getEvent(mock(InputStream.class), contentId, data);

assertThat(event).isEqualTo(updateEvent);
}
Expand All @@ -87,7 +88,7 @@ void contentIdsSameContentNullReturnsNothingToDo() {
var data = CdsData.create();
data.put(Attachments.CONTENT_ID, contentId);

var event = cut.getEvent(null, contentId, true, data);
var event = cut.getEvent(null, contentId, data);

assertThat(event).isEqualTo(doNothingEvent);
}
Expand All @@ -100,7 +101,7 @@ void contentIdNotPresentAndExistingNotNullReturnsDeleteEvent(String contentId) {
var data = CdsData.create();
data.put(Attachments.CONTENT_ID, "someValue");

var event = cut.getEvent(null, contentId, false, data);
var event = cut.getEvent(null, contentId, data);

assertThat(event).isEqualTo(deleteContentEvent);
}
Expand All @@ -112,14 +113,14 @@ void contentIdPresentAndExistingNotNullButDifferentReturnsDeleteEvent(String con
var data = CdsData.create();
data.put(Attachments.CONTENT_ID, "someValue");

var event = cut.getEvent(null, contentId, true, data);
var event = cut.getEvent(null, contentId, data);

assertThat(event).isEqualTo(deleteContentEvent);
}

@Test
void contentIdPresentAndExistingIdIsNullReturnsNothingToDo() {
var event = cut.getEvent(mock(InputStream.class), "test", true, CdsData.create());
var event = cut.getEvent(mock(InputStream.class), "test", CdsData.create());

assertThat(event).isEqualTo(doNothingEvent);
}
Expand All @@ -128,8 +129,9 @@ void contentIdPresentAndExistingIdIsNullReturnsNothingToDo() {
@ValueSource(strings = {"some document Id"})
@NullSource
@EmptySource
@Disabled
void contentIdNotPresentAndExistingNullReturnsCreateEvent(String contentId) {
var event = cut.getEvent(mock(InputStream.class), contentId, false, CdsData.create());
var event = cut.getEvent(mock(InputStream.class), contentId, CdsData.create());

assertThat(event).isEqualTo(createEvent);
}
Expand All @@ -139,7 +141,7 @@ void contentIdNotPresentAndExistingNullReturnsCreateEvent(String contentId) {
@NullSource
@EmptySource
void contentIdNotPresentAndExistingNullReturnsDoNothingEvent(String contentId) {
var event = cut.getEvent(null, contentId, false, CdsData.create());
var event = cut.getEvent(null, contentId, CdsData.create());

assertThat(event).isEqualTo(doNothingEvent);
}
Expand All @@ -149,7 +151,7 @@ void contentIdPresentButNullAndExistingNotNullReturnsUpdateEvent() {
var data = CdsData.create();
data.put(Attachments.CONTENT_ID, "someValue");

var event = cut.getEvent(mock(InputStream.class), null, true, data);
var event = cut.getEvent(mock(InputStream.class), null, data);

assertThat(event).isEqualTo(updateEvent);
}
Expand All @@ -159,7 +161,7 @@ void updateIfContentIdDifferentButContentProvided() {
var data = CdsData.create();
data.put(Attachments.CONTENT_ID, "existing");

var event = cut.getEvent(mock(InputStream.class), "someValue", true, data);
var event = cut.getEvent(mock(InputStream.class), "someValue", data);

assertThat(event).isEqualTo(updateEvent);
}
Expand Down
7 changes: 3 additions & 4 deletions ...om/sap/cds/feature/attachments/handler/draftservice/DraftPatchAttachmentsHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -63,7 +62,7 @@ void setup() {
cut = new DraftPatchAttachmentsHandler(persistence, eventFactory);
eventContext = mock(DraftPatchEventContext.class);
event = mock(ModifyAttachmentEvent.class);
when(eventFactory.getEvent(any(), any(), anyBoolean(), any())).thenReturn(event);
when(eventFactory.getEvent(any(), any(), any())).thenReturn(event);
selectCaptor = ArgumentCaptor.forClass(CqnSelect.class);
}

Expand Down Expand Up @@ -108,7 +107,7 @@ void selectedDataUsedForEventFactory() {

cut.processBeforeDraftPatch(eventContext, List.of(root));

verify(eventFactory).getEvent(content, attachment.getContentId(), false, attachment);
verify(eventFactory).getEvent(content, attachment.getContentId(), attachment);
}

@Test
Expand All @@ -124,7 +123,7 @@ void contentIdUsedForEventFactory() {

cut.processBeforeDraftPatch(eventContext, List.of(root));

verify(eventFactory).getEvent(content, attachment.getContentId(), true, attachment);
verify(eventFactory).getEvent(content, attachment.getContentId(), attachment);
verify(event).processEvent(any(), eq(content), eq(attachment), eq(eventContext));
}

Expand Down