Skip to content
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.

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

Remove encoded characters from file name #781

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@

import static com.github.onsdigital.logging.v2.event.SimpleEvent.info;
import static com.github.onsdigital.zebedee.configuration.CMSFeatureFlags.cmsFeatureFlags;
import static com.github.onsdigital.zebedee.configuration.Configuration.getDatasetWhitelist;
import static com.github.onsdigital.zebedee.json.EventType.APPROVAL_FAILED;
import static com.github.onsdigital.zebedee.logging.CMSLogEvent.error;

Expand Down Expand Up @@ -165,7 +166,11 @@ private boolean doApproval() throws Exception {
eventLog.compressedZipFiles();

if (Configuration.isUploadNewEndpointEnabled() && DatasetWhitelistChecker.isURIWhitelisted(collectionReader)) {
uploadNewEndpoint(collection, collectionReader);
List<String> reviewedList = collection.getReviewed().uris().stream().filter(i -> i.contains("datasets")).collect(Collectors.toList());
String whitelist = getDatasetWhitelist();
List<String> allowed = Arrays.asList(whitelist.split(","));
reviewedList = reviewedList.stream().filter(j -> allowed.stream().anyMatch(j::contains)).collect(Collectors.toList());
uploadWhitelistedFiles(collectionReader, reviewedList, collection.getDescription().getId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will probably need a null check on the reviewedList to ensure no null pointer exceptions

}

approveCollection();
Expand Down Expand Up @@ -352,22 +357,17 @@ private void validate() {
info().data("collectionId", collection.getDescription().getId()).log("approval task: validation successful");
}

protected void uploadNewEndpoint(Collection collection, CollectionReader collectionReader)
protected void uploadWhitelistedFiles(CollectionReader collectionReader, List<String> urisToCheck, String description)
throws ZebedeeException, IOException {
uploadWhitelistedFiles(collection, collectionReader);
}

protected void uploadWhitelistedFiles(Collection collection, CollectionReader collectionReader)
throws ZebedeeException, IOException {
String correctDatasetVersion = findCorrectDatasetVersion(collectionReader.getReviewed().listUris());
for (String uri : collectionReader.getReviewed().listUris()) {
for (String uri : urisToCheck) {
if (uri.endsWith(".csv") || uri.endsWith(".xlsx") || uri.endsWith(".xls") || uri.endsWith(".csdb")) {
String fileName = uri.substring(1);
Resource myFile = collectionReader.getResource(fileName);
if (DatasetWhitelistChecker.isWhitelisted(myFile.getName())) {
info().data("filename", fileName).data("collectionId", collection.getDescription().getId())
info().data("filename", fileName).data("collectionId", description)
.log("File is whitelisted");
uploadFile(myFile, fileName, collection.getDescription().getId(), correctDatasetVersion);
String correctDatasetVersion = findCorrectDatasetVersion(urisToCheck, fileName);
uploadFile(myFile, fileName, description, correctDatasetVersion);
}
}
}
Expand Down Expand Up @@ -526,14 +526,14 @@ protected String incrementDatasetVersionByOne(String datasetVersion) {
return letter + Integer.toString(number);
}

protected String findCorrectDatasetVersion(List<String> listOfUris) {
protected String findCorrectDatasetVersion(List<String> listOfUris, String fileName) {
if (listOfUris == null) {
throw new IllegalArgumentException("input array can't be null");
}
String datasetVersion = "";
for (String uri : listOfUris) {
if (uri.endsWith(".csv") || uri.endsWith(".xlsx") || uri.endsWith(".xls") || uri.endsWith(".csdb")) {
if (uri.contains("previous")) {
if (uri.contains("previous") && uri.contains(extractFileName(fileName))) {
datasetVersion = extractDatasetVersion(uri);
return datasetVersion;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,46 +332,6 @@ public void shouldApproveCmdSuccessfully() throws Exception {
}
}

// new unit tests

@Test
public void testUploadNewEndpoint_HappyPath() throws ZebedeeException, IOException {
// Given
Collection collection = Mockito.mock(Collection.class);
CollectionReader collectionReader = Mockito.mock(CollectionReader.class);
System.setProperty("ENABLE_UPLOAD_NEW_ENDPOINT", "true");

when(collection.getDescription()).thenReturn(collectionDescription);
when(collectionReader.getReviewed()).thenReturn(contentReader);
when(collectionWriter.getReviewed()).thenReturn(contentWriter);
when(collection.getReviewed()).thenReturn(content);

// When
task.uploadNewEndpoint(collection, collectionReader);

// Then
verify(task, times(1)).uploadWhitelistedFiles(collection, collectionReader);
}

@Test
public void testUploadNewEndpoint_UnHappyPath() throws ZebedeeException, IOException {
// Given
Collection collection = Mockito.mock(Collection.class);
CollectionReader collectionReader = Mockito.mock(CollectionReader.class);
System.setProperty("ENABLE_UPLOAD_NEW_ENDPOINT", "false");

when(collection.getDescription()).thenReturn(collectionDescription);
when(collectionReader.getReviewed()).thenReturn(contentReader);
when(collectionWriter.getReviewed()).thenReturn(contentWriter);
when(collection.getReviewed()).thenReturn(content);

// When
task.uploadNewEndpoint(collection, collectionReader);

// Then
verify(task, times(1)).uploadWhitelistedFiles(collection, collectionReader);
}

@Test
public void testCreateUploadParams() {
String resumableFilename = "test_file.csv";
Expand Down Expand Up @@ -574,7 +534,7 @@ public void testIncrementDatasetVersionByOne_HappyPath() {

@Test
public void testfindCorrectDatasetVersion_nullInput() {
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> task.findCorrectDatasetVersion(null));
IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> task.findCorrectDatasetVersion(null, ""));
assertThat(ex.getMessage(), equalTo("input array can't be null"));
}

Expand All @@ -587,7 +547,7 @@ public void testfindCorrectDatasetVersion_correctInput() {
listOfUris.add("/economy/grossdomesticproductgdp/datasets/mycollectionpagedltest1/augusttoseptember2024/previous/v1/diop.csv");

String expected = "v1";
String actual = task.findCorrectDatasetVersion(listOfUris);
String actual = task.findCorrectDatasetVersion(listOfUris, "diop.csv");
assertEquals(expected, actual);
}

Expand All @@ -600,7 +560,7 @@ public void testfindCorrectDatasetVersion_anotherCorrectInput_previousVersionExi
listOfUris.add("/economy/grossdomesticproductgdp/datasets/mycollectionpagedltest1/current/previous/v123/diop.csv");

String expected = "v123";
String actual = task.findCorrectDatasetVersion(listOfUris);
String actual = task.findCorrectDatasetVersion(listOfUris, "diop.csv");
assertEquals(expected, actual);
}

Expand All @@ -612,7 +572,7 @@ public void testfindCorrectDatasetVersion_anotherCorrectInput_previousVersionDoe
listOfUris.add("/economy/grossdomesticproductgdp/datasets/mycollectionpagedltest1/september2024/diop.xlsx");

String expected = "";
String actual = task.findCorrectDatasetVersion(listOfUris);
String actual = task.findCorrectDatasetVersion(listOfUris, "diop.csv");
assertEquals(expected, actual);
}

Expand All @@ -621,8 +581,7 @@ public void testfindCorrectDatasetVersion_emptyList() {
List<String> listOfUris = new ArrayList<>();

String expected = "";
String actual = task.findCorrectDatasetVersion(listOfUris);
String actual = task.findCorrectDatasetVersion(listOfUris, "diop.csv");
assertEquals(expected, actual);
}

}