Skip to content
Snippets Groups Projects
Commit 48de0614 authored by Alok Joshi's avatar Alok Joshi
Browse files

validate metadata is not empty after patch

parent 623f8e67
No related branches found
No related tags found
2 merge requests!744Upgraded packages to mitigated vulns in netty, guava, snakeyaml,!639Extend patch API to support data block modification
Hide whitespace changes
Inline Side-by-side
storage-core/src/main/java/org/opengroup/osdu/storage/service/PatchRecordsServiceImpl.java
+ 67
− 14
View file @ 48de0614
......@@ -14,7 +14,10 @@
package org.opengroup.osdu.storage.service;
import com.fasterxml.jackson.annotation.JsonAutoDetect;
import com.fasterxml.jackson.annotation.PropertyAccessor;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.github.fge.jsonpatch.JsonPatch;
......@@ -42,6 +45,7 @@ import org.opengroup.osdu.storage.validation.api.PatchInputValidator;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import org.springframework.util.CollectionUtils;
import java.util.ArrayList;
import java.util.Iterator;
......@@ -98,6 +102,7 @@ public class PatchRecordsServiceImpl implements PatchRecordsService {
@Override
public PatchRecordsResponse patchRecords(List<String> recordIds, JsonPatch jsonPatch, String user, Optional<CollaborationContext> collaborationContext) {
List<String> successfulRecordIds = new ArrayList<>();
List<String> failedRecordIds = new ArrayList<>();
List<String> notFoundRecordIds = new ArrayList<>();
List<String> errors = new ArrayList<>();
......@@ -113,20 +118,23 @@ public class PatchRecordsServiceImpl implements PatchRecordsService {
if(dataUpdate) {
MultiRecordInfo multiRecordInfo = batchService.getMultipleRecords(new MultiRecordIds(recordIds, attributes), collaborationContext);
notFoundRecordIds = multiRecordInfo.getInvalidRecords();
recordIds.removeAll(notFoundRecordIds);
List<Record> recordsToPersist = new ArrayList<>();
for(Record validRecord : multiRecordInfo.getRecords()) {
try {
JsonNode patched = jsonPatch.apply(objectMapper.convertValue(validRecord, JsonNode.class));
Record patchedRecord = objectMapper.treeToValue(patched, Record.class);
recordsToPersist.add(patchedRecord);
if(isEmptyAclOrLegal(patchedRecord)) {
failedRecordIds.add(validRecord.getId());
errors.add("Patch operation for record: " + validRecord.getId() + " aborted. Potentially empty value of legaltags or acl/owners or acl/viewers");
} else {
recordsToPersist.add(patchedRecord);
successfulRecordIds.add(validRecord.getId());
}
} catch (JsonPatchException e) {
recordIds.remove(validRecord.getId());
failedRecordIds.add(validRecord.getId());
errors.add("Json patch error for record: "+validRecord.getId());
} catch (JsonProcessingException e) {
recordIds.remove(validRecord.getId());
failedRecordIds.add(validRecord.getId());
errors.add("Json processing error for record: "+validRecord.getId());
}
......@@ -153,19 +161,28 @@ public class PatchRecordsServiceImpl implements PatchRecordsService {
long currentTime = System.currentTimeMillis();
for(String recordId : recordIds) {
RecordMetadata metadata = existingRecords.get(CollaborationUtil.getIdWithNamespace(recordId, collaborationContext));
if(metadata == null) {
notFoundRecordIds.add(recordId);
recordIds.remove(recordId);
} else {
metadata.setModifyTime(currentTime);
metadata.setModifyUser(user);
recordMetadataToBePatched.add(metadata);
try {
if (checkIfResultingAclOrLegalTagsAreEmpty(metadata, jsonPatch)) {
failedRecordIds.add(recordId);
errors.add("Patch operation for record: " + recordId + " aborted. Potentially empty value of legaltags or acl/owners or acl/viewers");
} else {
if(metadata == null) {
notFoundRecordIds.add(recordId);
} else {
metadata.setModifyTime(currentTime);
metadata.setModifyUser(user);
recordMetadataToBePatched.add(metadata);
successfulRecordIds.add(recordId);
}
}
} catch (AppException e) {
failedRecordIds.add(recordId);
errors.add("Patch operation for record: " + recordId + " failed with error: " + e.getMessage());
}
}
if(!recordMetadataToBePatched.isEmpty()) {
Map<String, String> recordIdPatchError = persistenceService.patchRecordsMetadata(recordMetadataToBePatched, jsonPatch, collaborationContext);
for(String currentRecordId : recordIdPatchError.keySet()) {
recordIds.remove(recordIdPatchError.remove(currentRecordId));
errors.add(recordIdPatchError.get(currentRecordId));
}
}
......@@ -173,10 +190,10 @@ public class PatchRecordsServiceImpl implements PatchRecordsService {
PatchRecordsResponse recordsResponse = PatchRecordsResponse.builder()
.notFoundRecordIds(notFoundRecordIds)
.recordIds(recordIds)
.recordIds(successfulRecordIds)
.failedRecordIds(failedRecordIds)
.errors(errors)
.recordCount(recordIds.size()).build();
.recordCount(successfulRecordIds.size()).build();
auditCreateOrUpdateRecords(recordsResponse);
......@@ -242,4 +259,40 @@ public class PatchRecordsServiceImpl implements PatchRecordsService {
}
return false;
}
private boolean checkIfResultingAclOrLegalTagsAreEmpty(RecordMetadata recordMetadata, JsonPatch jsonPatch) {
try {
objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
objectMapper.setVisibility(PropertyAccessor.GETTER, JsonAutoDetect.Visibility.NONE);
objectMapper.setVisibility(PropertyAccessor.SETTER, JsonAutoDetect.Visibility.NONE);
objectMapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
JsonNode patched = jsonPatch.apply(objectMapper.convertValue(recordMetadata, JsonNode.class));
RecordMetadata patchedRecordMetadata = objectMapper.treeToValue(patched, RecordMetadata.class);
return isEmptyAclOrLegal(patchedRecordMetadata);
} catch (JsonPatchException e) {
throw new AppException(HttpStatus.SC_BAD_REQUEST, "Bad input", "JsonPatchException during patch operation");
} catch (JsonProcessingException e) {
throw new AppException(HttpStatus.SC_BAD_REQUEST, "Bad input", "JsonProcessingException during patch operation");
}
}
private boolean isEmptyAclOrLegal(RecordMetadata recordMetadata) {
return recordMetadata.getAcl() == null ||
recordMetadata.getAcl().getViewers() == null ||
recordMetadata.getAcl().getOwners() == null ||
recordMetadata.getLegal() == null ||
recordMetadata.getAcl().getOwners().length == 0 ||
recordMetadata.getAcl().getViewers().length == 0 ||
CollectionUtils.isEmpty(recordMetadata.getLegal().getLegaltags());
}
private boolean isEmptyAclOrLegal(Record record) {
return record.getAcl() == null ||
record.getAcl().getViewers() == null ||
record.getAcl().getOwners() == null ||
record.getLegal() == null ||
record.getAcl().getOwners().length == 0 ||
record.getAcl().getViewers().length == 0 ||
CollectionUtils.isEmpty(record.getLegal().getLegaltags());
}
}
storage-core/src/test/java/org/opengroup/osdu/storage/service/PatchRecordsServiceImplTest.java
+ 119
− 0
View file @ 48de0614
......@@ -13,6 +13,7 @@ import org.opengroup.osdu.core.common.model.entitlements.Acl;
import org.opengroup.osdu.core.common.model.http.CollaborationContext;
import org.opengroup.osdu.core.common.model.http.DpsHeaders;
import org.opengroup.osdu.core.common.model.indexer.OperationType;
import org.opengroup.osdu.core.common.model.legal.Legal;
import org.opengroup.osdu.core.common.model.storage.MultiRecordIds;
import org.opengroup.osdu.core.common.model.storage.MultiRecordInfo;
import org.opengroup.osdu.core.common.model.storage.Record;
......@@ -30,6 +31,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
......@@ -40,7 +42,9 @@ import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
......@@ -180,6 +184,115 @@ public class PatchRecordsServiceImplTest {
verify(auditLogger).createOrUpdateRecordsSuccess(result.getRecordIds());
}
@Test
public void shouldPatchRecordDataPartially_ifResultHasEmptyAclForOneRecord() throws IOException{
ReflectionTestUtils.setField(sut, "isOpaEnabled", false);
JsonPatch jsonPatch = JsonPatch.fromJson(mapper.readTree("[{\"op\":\"remove\", \"path\":\"/acl/viewers/0\"}, {\"op\":\"add\", \"path\":\"/data\", \"value\":{\"Hello\" : \"world\"}}]"));
MultiRecordInfo multiRecordInfo = getMultiRecordInfo();
String[] record1ViewersAcl = new String[]{"viewer1@company.com"};
multiRecordInfo.getRecords().get(0).getAcl().setViewers(record1ViewersAcl);
when(batchService.getMultipleRecords(any(MultiRecordIds.class), eq(COLLABORATION_CONTEXT))).thenReturn(multiRecordInfo);
PatchRecordsResponse result = sut.patchRecords(recordIds, jsonPatch, USER, COLLABORATION_CONTEXT);
verifyValidatorsWereInvocated(jsonPatch);
assertEquals(1, result.getFailedRecordIds().size());
assertEquals(RECORD_ID1, result.getFailedRecordIds().get(0));
assertEquals(1, result.getErrors().size());
assertEquals("Patch operation for record: " + RECORD_ID1 + " aborted. Potentially empty value of legaltags or acl/owners or acl/viewers", result.getErrors().get(0));
assertEquals(RECORD_ID2, result.getRecordIds().get(0));
assertThat(result.getRecordCount(), is(1));
Record patchedRecord2 = getRecord(RECORD_ID2);
String[] record2ViewersAcl = new String[]{"viewer2@company.com"};
patchedRecord2.getAcl().setViewers(record2ViewersAcl);
patchedRecord2.setData(Collections.singletonMap("Hello", "world"));
verify(ingestionService).createUpdateRecords(false, Arrays.asList(patchedRecord2), USER, COLLABORATION_CONTEXT);
verify(auditLogger).createOrUpdateRecordsSuccess(result.getRecordIds());
}
@Test
public void shouldNotPatchData_ifResultHasEmptyLegaltagsForAllRecords() throws IOException{
ReflectionTestUtils.setField(sut, "isOpaEnabled", false);
JsonPatch jsonPatch = JsonPatch.fromJson(mapper.readTree("[{\"op\":\"remove\", \"path\":\"/legal/legaltags/0\"}, {\"op\":\"add\", \"path\":\"/data\", \"value\":{\"Hello\" : \"world\"}}]"));
MultiRecordInfo multiRecordInfo = getMultiRecordInfo();
when(batchService.getMultipleRecords(any(MultiRecordIds.class), eq(COLLABORATION_CONTEXT))).thenReturn(multiRecordInfo);
PatchRecordsResponse result = sut.patchRecords(recordIds, jsonPatch, USER, COLLABORATION_CONTEXT);
verifyValidatorsWereInvocated(jsonPatch);
assertEquals(2, result.getFailedRecordIds().size());
assertEquals(RECORD_ID1, result.getFailedRecordIds().get(0));
assertEquals(RECORD_ID2, result.getFailedRecordIds().get(1));
assertEquals(2, result.getErrors().size());
assertEquals("Patch operation for record: " + RECORD_ID1 + " aborted. Potentially empty value of legaltags or acl/owners or acl/viewers", result.getErrors().get(0));
assertEquals("Patch operation for record: " + RECORD_ID2 + " aborted. Potentially empty value of legaltags or acl/owners or acl/viewers", result.getErrors().get(1));
assertTrue(result.getRecordIds().isEmpty());
verify(ingestionService, never()).createUpdateRecords(eq(false), anyList(), eq(USER), eq(COLLABORATION_CONTEXT));
verify(auditLogger, never()).createOrUpdateRecordsSuccess(result.getRecordIds());
}
@Test
public void shouldPatchRecordMetadataPartially_ifResultHasEmptyAclForOneRecord() throws IOException {
ReflectionTestUtils.setField(sut, "isOpaEnabled", false);
JsonPatch jsonPatch = JsonPatch.fromJson(mapper.readTree("[{\"op\":\"remove\", \"path\":\"/acl/viewers/0\"}]"));
Map<String, RecordMetadata> existingRecords = getExistingRecordsMetadata();
String[] record1ViewersAcl = new String[]{"viewer1@company.com"};
existingRecords.get(RECORD_ID1).setAcl(new Acl(record1ViewersAcl, OWNERS));
when(recordRepository.get(recordIds, COLLABORATION_CONTEXT)).thenReturn(existingRecords);
when(entitlementsAndCacheService.hasOwnerAccess(headers, OWNERS)).thenReturn(true);
List<RecordMetadata> recordMetadataToBePatched = new ArrayList<>();
recordMetadataToBePatched.add(existingRecords.get(RECORD_ID2));
PatchRecordsResponse result = sut.patchRecords(recordIds, jsonPatch, USER, COLLABORATION_CONTEXT);
verifyValidatorsWereInvocated(jsonPatch);
verify(patchInputValidator).validateLegalTags(jsonPatch);
verify(entitlementsAndCacheService, times(2)).hasOwnerAccess(headers, OWNERS);
assertEquals(Collections.emptyList(), result.getNotFoundRecordIds());
assertEquals(1, result.getFailedRecordIds().size());
assertEquals(RECORD_ID1, result.getFailedRecordIds().get(0));
assertEquals(1, result.getErrors().size());
assertEquals("Patch operation for record: " + RECORD_ID1 + " aborted. Potentially empty value of legaltags or acl/owners or acl/viewers", result.getErrors().get(0));
assertEquals(RECORD_ID2, result.getRecordIds().get(0));
assertThat(result.getRecordCount(), is(1));
verify(persistenceService).patchRecordsMetadata(recordMetadataToBePatched, jsonPatch, COLLABORATION_CONTEXT);
verify(auditLogger).createOrUpdateRecordsSuccess(result.getRecordIds());
}
@Test
public void shouldNotPatchMetadata_ifResultHasEmptyLegaltagsForAllRecords() throws IOException {
ReflectionTestUtils.setField(sut, "isOpaEnabled", false);
JsonPatch jsonPatch = JsonPatch.fromJson(mapper.readTree("[{\"op\":\"remove\", \"path\":\"/legal/legaltags/0\"}]"));
Map<String, RecordMetadata> existingRecords = getExistingRecordsMetadata();
when(recordRepository.get(recordIds, COLLABORATION_CONTEXT)).thenReturn(existingRecords);
when(entitlementsAndCacheService.hasOwnerAccess(headers, OWNERS)).thenReturn(true);
PatchRecordsResponse result = sut.patchRecords(recordIds, jsonPatch, USER, COLLABORATION_CONTEXT);
verifyValidatorsWereInvocated(jsonPatch);
verify(patchInputValidator).validateLegalTags(jsonPatch);
verify(entitlementsAndCacheService, times(2)).hasOwnerAccess(headers, OWNERS);
assertEquals(Collections.emptyList(), result.getNotFoundRecordIds());
assertEquals(2, result.getFailedRecordIds().size());
assertTrue(result.getFailedRecordIds().contains(RECORD_ID1));
assertTrue(result.getFailedRecordIds().contains(RECORD_ID2));
assertEquals(2, result.getErrors().size());
assertTrue(result.getErrors().contains("Patch operation for record: " + RECORD_ID1 + " aborted. Potentially empty value of legaltags or acl/owners or acl/viewers"));
assertTrue(result.getRecordIds().isEmpty());
assertThat(result.getRecordCount(), is(0));
verify(persistenceService, never()).patchRecordsMetadata(anyList(), eq(jsonPatch), eq(COLLABORATION_CONTEXT));
verify(auditLogger, never()).createOrUpdateRecordsSuccess(result.getRecordIds());
}
@Test
public void shouldFailRecordDataPatch_ifValueInvalid() throws IOException {
JsonPatch inValidJsonPatch = JsonPatch.fromJson(mapper.readTree("[{\"op\":\"add\", \"path\":\"/data\", \"value\":\"inValidDataFormat\"}]"));
......@@ -244,6 +357,9 @@ public class PatchRecordsServiceImplTest {
Record record = new Record();
record.setId(recordId);
record.setAcl(new Acl(VIEWERS, OWNERS));
Legal legal = new Legal();
legal.setLegaltags(new HashSet<>(Arrays.asList("legaltag")));
record.setLegal(legal);
return record;
}
......@@ -261,6 +377,9 @@ public class PatchRecordsServiceImplTest {
RecordMetadata recordMetadata = new RecordMetadata();
recordMetadata.setId(recordId);
recordMetadata.setAcl(new Acl(VIEWERS, OWNERS));
Legal legal = new Legal();
legal.setLegaltags(new HashSet<>(Arrays.asList("legaltag")));
recordMetadata.setLegal(legal);
return recordMetadata;
}
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment