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.

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

allow changes to file metadata via API #6962 #6968

Merged
merged 7 commits into from
Jul 1, 2020
Merged

allow changes to file metadata via API #6962 #6968

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
809643a
allow changes to file metadata via API #6962
pdurbin Jun 5, 2020
508e95c
allow file metadata changes if passing existing path/name #6962
pdurbin Jun 8, 2020
9954220
allow changes if dir exists and label passed, formatting #6962
pdurbin Jun 9, 2020
e5ec9bc
avoid dir1 + file1 == dir + 1file1 (add "/") #6962
pdurbin Jun 9, 2020
491caff
eliminate code we don't need, handled elsewhere #6962
pdurbin Jun 9, 2020
a34d7c1
ensure passing "" as directoryLabel doesn't cause problems #6962
pdurbin Jun 10, 2020
2debed2
simply conflict-checking code #6962
pdurbin Jun 15, 2020
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
30 changes: 23 additions & 7 deletions src/main/java/edu/harvard/iq/dataverse/api/Files.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,16 +392,32 @@ public Response updateFileMetadata(@FormDataParam("jsonData") String jsonData,
javax.json.JsonObject jsonObject = jsonReader.readObject();
String label = jsonObject.getString("label", null);
String directoryLabel = jsonObject.getString("directoryLabel", null);
String path = "";
if (directoryLabel != null) {
path = directoryLabel + "/";
// If the user is trying to change the label/directoryLabel or not.
boolean labelChange = true;
if (label == null && directoryLabel == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Last suggested change - we can remove this check since it's handled by the code below (if both are null, they get populated with their old values). Simplifies the code for minimal "cost".

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. Removed in 491caff.

labelChange = false;
}
String oldLabel = df.getFileMetadata().getLabel();
String oldDirectoryLabel = df.getFileMetadata().getDirectoryLabel();
String oldPathPlusName = oldDirectoryLabel + "/" + oldLabel;
if (directoryLabel == null) {
directoryLabel = oldDirectoryLabel;
}
if (label == null) {
label = df.getFileMetadata().getLabel();
label = oldLabel;
}
String incomingPathPlusName = directoryLabel + "/" + label;
if (oldPathPlusName.equals(incomingPathPlusName)) {
labelChange = false;
}
if (IngestUtil.conflictsWithExistingFilenames(label, directoryLabel, fmdList, df)) {
String incomingPathPlusFileName = path + label;
return error(BAD_REQUEST, BundleUtil.getStringFromBundle("files.api.metadata.update.duplicateFile", Arrays.asList(incomingPathPlusFileName)));
if (labelChange && IngestUtil.conflictsWithExistingFilenames(label, directoryLabel, fmdList, df)) {
String pathPlusFilename = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is just for the error message, but why not just use "incomingPathPlusName" here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because if we do (I just tried), we get "null/" as part of the output. Tests fail with messages like this:

Expected: Filename already exists at README.md
  Actual: Filename already exists at null/README.md

That is to say, that code (below) is there to make sure there's no "null/" in the output, like above.

String pathPlusFilename = "";
if (directoryLabel != null) {
    pathPlusFilename = directoryLabel + "/" + label;
} else {
    pathPlusFilename = label;
}

if (directoryLabel != null) {
pathPlusFilename = directoryLabel + "/" + label;
} else {
pathPlusFilename = label;
}
return error(BAD_REQUEST, BundleUtil.getStringFromBundle("files.api.metadata.update.duplicateFile", Arrays.asList(pathPlusFilename)));
}

optionalFileParams.addOptionalParams(upFmd);
Expand Down
156 changes: 156 additions & 0 deletions src/test/java/edu/harvard/iq/dataverse/api/DuplicateFilesIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,4 +263,160 @@ public void moveFileToDirectoryContainingSameFileName() throws IOException {

}

/**
* In this test we make sure that other changes in the absence of label and
* directoryLabel go through, such as changing a file description.
*/
@Test
public void modifyFileDescription() throws IOException {

Response createUser = UtilIT.createRandomUser();
createUser.prettyPrint();
createUser.then().assertThat()
.statusCode(OK.getStatusCode());
String username = UtilIT.getUsernameFromResponse(createUser);
String apiToken = UtilIT.getApiTokenFromResponse(createUser);

Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken);
createDataverseResponse.prettyPrint();
createDataverseResponse.then().assertThat()
.statusCode(CREATED.getStatusCode());

String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse);

Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken);
createDataset.prettyPrint();
createDataset.then().assertThat()
.statusCode(CREATED.getStatusCode());

Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset);

Path pathtoReadme1 = Paths.get(Files.createTempDirectory(null) + File.separator + "README.md");
Files.write(pathtoReadme1, "File 1".getBytes());
System.out.println("README: " + pathtoReadme1);

Response uploadReadme1 = UtilIT.uploadFileViaNative(datasetId.toString(), pathtoReadme1.toString(), apiToken);
uploadReadme1.prettyPrint();
uploadReadme1.then().assertThat()
.statusCode(OK.getStatusCode())
.body("data.files[0].label", equalTo("README.md"));

Integer idOfReadme1 = JsonPath.from(uploadReadme1.getBody().asString()).getInt("data.files[0].dataFile.id");
System.out.println("id: " + idOfReadme1);

JsonObjectBuilder updateFileMetadata = Json.createObjectBuilder()
.add("description", "This file is awesome.");
Response updateFileMetadataResponse = UtilIT.updateFileMetadata(String.valueOf(idOfReadme1), updateFileMetadata.build().toString(), apiToken);
updateFileMetadataResponse.prettyPrint();
updateFileMetadataResponse.then().statusCode(OK.getStatusCode());

}

/**
* In this test we make sure that that if you keep the label the same, you
* are still able to change other metadata such as file description.
*/
@Test
public void modifyFileDescriptionSameLabel() throws IOException {

Response createUser = UtilIT.createRandomUser();
createUser.prettyPrint();
createUser.then().assertThat()
.statusCode(OK.getStatusCode());
String username = UtilIT.getUsernameFromResponse(createUser);
String apiToken = UtilIT.getApiTokenFromResponse(createUser);

Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken);
createDataverseResponse.prettyPrint();
createDataverseResponse.then().assertThat()
.statusCode(CREATED.getStatusCode());

String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse);

Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken);
createDataset.prettyPrint();
createDataset.then().assertThat()
.statusCode(CREATED.getStatusCode());

Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset);

Path pathtoReadme1 = Paths.get(Files.createTempDirectory(null) + File.separator + "README.md");
Files.write(pathtoReadme1, "File 1".getBytes());
System.out.println("README: " + pathtoReadme1);

JsonObjectBuilder json1 = Json.createObjectBuilder()
.add("directoryLabel", "code");

Response uploadReadme1 = UtilIT.uploadFileViaNative(datasetId.toString(), pathtoReadme1.toString(), json1.build(), apiToken);
uploadReadme1.prettyPrint();
uploadReadme1.then().assertThat()
.statusCode(OK.getStatusCode())
.body("data.files[0].label", equalTo("README.md"));

Integer idOfReadme1 = JsonPath.from(uploadReadme1.getBody().asString()).getInt("data.files[0].dataFile.id");
System.out.println("id: " + idOfReadme1);

JsonObjectBuilder updateFileMetadata = Json.createObjectBuilder()
.add("label", "README.md")
.add("directoryLabel", "code")
.add("description", "This file is awesome.");
Response updateFileMetadataResponse = UtilIT.updateFileMetadata(String.valueOf(idOfReadme1), updateFileMetadata.build().toString(), apiToken);
updateFileMetadataResponse.prettyPrint();
updateFileMetadataResponse.then().statusCode(OK.getStatusCode());

}

/**
* What if the directory for the file exists and you pass the filename
* (label) while changing the description? This should be allowed.
*/
@Test
public void existingDirectoryPassLabelChangeDescription() throws IOException {

Response createUser = UtilIT.createRandomUser();
createUser.prettyPrint();
createUser.then().assertThat()
.statusCode(OK.getStatusCode());
String username = UtilIT.getUsernameFromResponse(createUser);
String apiToken = UtilIT.getApiTokenFromResponse(createUser);

Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken);
createDataverseResponse.prettyPrint();
createDataverseResponse.then().assertThat()
.statusCode(CREATED.getStatusCode());

String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse);

Response createDataset = UtilIT.createRandomDatasetViaNativeApi(dataverseAlias, apiToken);
createDataset.prettyPrint();
createDataset.then().assertThat()
.statusCode(CREATED.getStatusCode());

Integer datasetId = UtilIT.getDatasetIdFromResponse(createDataset);

Path pathToFile = Paths.get(Files.createTempDirectory(null) + File.separator + "label");
Files.write(pathToFile, "File 1".getBytes());
System.out.println("file: " + pathToFile);

JsonObjectBuilder json1 = Json.createObjectBuilder()
.add("directory", "code");

Response uploadFile = UtilIT.uploadFileViaNative(datasetId.toString(), pathToFile.toString(), json1.build(), apiToken);
uploadFile.prettyPrint();
uploadFile.then().assertThat()
.statusCode(OK.getStatusCode())
.body("data.files[0].label", equalTo("label"));

Integer idOfFile = JsonPath.from(uploadFile.getBody().asString()).getInt("data.files[0].dataFile.id");
System.out.println("id: " + idOfFile);

JsonObjectBuilder updateFileMetadata = Json.createObjectBuilder()
.add("label", "label")
.add("description", "This file is awesome.");
Response updateFileMetadataResponse = UtilIT.updateFileMetadata(String.valueOf(idOfFile), updateFileMetadata.build().toString(), apiToken);
updateFileMetadataResponse.prettyPrint();
updateFileMetadataResponse.then().statusCode(OK.getStatusCode());

}

}