Skip to content

Commit

Permalink
Per review feedback, made it impossible to supply the file sizes via …
Browse files Browse the repository at this point in the history
…the /addFiles API (i.e., we don't want to trust the users of the direct s3 upload api when it comes to file sizes). #10977
  • Loading branch information
landreev committed Nov 25, 2024
1 parent e67bc6e commit e8093c6
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,15 @@ public class AddReplaceFileHelper{
private String newFileName; // step 30
private String newFileContentType; // step 30
private String newStorageIdentifier; // step 30
private String newCheckSum; // step 30
private ChecksumType newCheckSumType; //step 30
private Long suppliedFileSize = null;


// -- Optional
private DataFile fileToReplace; // step 25

private DatasetVersion workingVersion;
private DatasetVersion clone;
List<DataFile> initialFileList;
List<DataFile> finalFileList;
private boolean trustSuppliedFileSizes;

// -----------------------------------
// Ingested files
Expand Down Expand Up @@ -611,18 +609,9 @@ private boolean runAddReplacePhase1(Dataset owner,
return false;

}
if (optionalFileParams != null) {
if (optionalFileParams.hasCheckSum()) {
newCheckSum = optionalFileParams.getCheckSum();
newCheckSumType = optionalFileParams.getCheckSumType();
}
if (optionalFileParams.hasFileSize()) {
suppliedFileSize = optionalFileParams.getFileSize();
}
}

msgt("step_030_createNewFilesViaIngest");
if (!this.step_030_createNewFilesViaIngest()){
if (!this.step_030_createNewFilesViaIngest(optionalFileParams)){
return false;

}
Expand Down Expand Up @@ -1195,7 +1184,7 @@ private boolean step_007_auto_isReplacementInLatestVersion(DataFile existingFile
}


private boolean step_030_createNewFilesViaIngest(){
private boolean step_030_createNewFilesViaIngest(OptionalFileParams optionalFileParams){

if (this.hasError()){
return false;
Expand All @@ -1207,6 +1196,22 @@ private boolean step_030_createNewFilesViaIngest(){
//Don't repeatedly update the clone (losing changes) in multifile case
clone = workingVersion.cloneDatasetVersion();
}

Long suppliedFileSize = null;
String newCheckSum = null;
ChecksumType newCheckSumType = null;


if (optionalFileParams != null) {
if (optionalFileParams.hasCheckSum()) {
newCheckSum = optionalFileParams.getCheckSum();
newCheckSumType = optionalFileParams.getCheckSumType();
}
if (trustSuppliedFileSizes && optionalFileParams.hasFileSize()) {
suppliedFileSize = optionalFileParams.getFileSize();
}
}

try {
UploadSessionQuotaLimit quota = null;
if (systemConfig.isStorageQuotasEnforced()) {
Expand Down Expand Up @@ -2028,9 +2033,15 @@ public void setDuplicateFileWarning(String duplicateFileWarning) {
* @param jsonData - an array of jsonData entries (one per file) using the single add file jsonData format
* @param dataset
* @param authUser
* @param trustSuppliedSizes - whether to accept the fileSize values passed
* in jsonData (we don't want to trust the users of the S3 direct
* upload API with that information - we will verify the status of
* the files in the S3 bucket and confirm the sizes in the process.
* we do want GlobusService to be able to pass the file sizes, since
* they are obtained and verified via a Globus API lookup).
* @return
*/
public Response addFiles(String jsonData, Dataset dataset, User authUser) {
public Response addFiles(String jsonData, Dataset dataset, User authUser, boolean trustSuppliedFileSizes) {
msgt("(addFilesToDataset) jsonData: " + jsonData.toString());

JsonArrayBuilder jarr = Json.createArrayBuilder();
Expand All @@ -2039,6 +2050,7 @@ public Response addFiles(String jsonData, Dataset dataset, User authUser) {

int totalNumberofFiles = 0;
int successNumberofFiles = 0;
this.trustSuppliedFileSizes = trustSuppliedFileSizes;
// -----------------------------------------------------------
// Read jsonData and Parse files information from jsondata :
// -----------------------------------------------------------
Expand Down Expand Up @@ -2171,6 +2183,10 @@ public Response addFiles(String jsonData, Dataset dataset, User authUser) {
.add("data", Json.createObjectBuilder().add("Files", jarr).add("Result", result)).build() ).build();
}

public Response addFiles(String jsonData, Dataset dataset, User authUser) {
return addFiles(jsonData, dataset, authUser, false);
}

/**
* Replace multiple files with prepositioned replacements as listed in the
* jsonData. Works with direct upload, Globus, and other out-of-band methods.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ private void processUploadedFiles(JsonArray filesJsonArray, Dataset dataset, Aut
// The old code had 2 sec. of sleep, so ...
Thread.sleep(2000);

Response addFilesResponse = addFileHelper.addFiles(newjsonData, dataset, authUser);
Response addFilesResponse = addFileHelper.addFiles(newjsonData, dataset, authUser, true);

if (addFilesResponse == null) {
logger.info("null response from addFiles call");
Expand Down

0 comments on commit e8093c6

Please sign in to comment.