Skip to content

Commit

Permalink
Prov: Fix optimistic lock saving issue #4343
Browse files Browse the repository at this point in the history
Also fixed a bug where changing the entityName on its own did not save.
  • Loading branch information
matthew-a-dunlap committed May 4, 2018
1 parent 5b0f2d6 commit fe21ceb
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 19 deletions.
52 changes: 38 additions & 14 deletions src/main/java/edu/harvard/iq/dataverse/EditDatafilesPage.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package edu.harvard.iq.dataverse;

import edu.harvard.iq.dataverse.ProvPopupFragmentBean.UpdatesEntry;
import edu.harvard.iq.dataverse.api.AbstractApiBean;
import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean;
import edu.harvard.iq.dataverse.authorization.Permission;
Expand Down Expand Up @@ -1094,15 +1095,22 @@ public String save() {
ingestService.addFiles(workingVersion, newFiles);
//boolean newDraftVersion = false;


Boolean provJsonChanges = false;

if(systemConfig.isProvCollectionEnabled()) {
Boolean provChanges = provPopupFragmentBean.updatePageMetadatasWithProvFreeform(fileMetadatas);
if(datasetUpdateRequired == false) {
datasetUpdateRequired = provChanges;
Boolean provFreeChanges = provPopupFragmentBean.updatePageMetadatasWithProvFreeform(fileMetadatas);

try {
provJsonChanges = provPopupFragmentBean.saveStagedProvJson(false);
} catch (AbstractApiBean.WrappedResponse ex) {
JsfHelper.addErrorMessage(getBundleString("file.metadataTab.provenance.error"));
Logger.getLogger(EditDatafilesPage.class.getName()).log(Level.SEVERE, null, ex);
}
//Always update the whole dataset if updating prov
//The flow that happens when datasetUpdateRequired is false has problems with doing saving actions after its merge
//This was the simplest way to work around this issue for prov. --MAD 4.8.6.
datasetUpdateRequired = datasetUpdateRequired || provFreeChanges || provJsonChanges;
}
Boolean provSaveContext = !datasetUpdateRequired; //need to track whether save happened here for later prov saving

if (workingVersion.getId() == null || datasetUpdateRequired) {
logger.fine("issuing the dataset update command");
Expand All @@ -1124,6 +1132,22 @@ public String save() {
}
}


//Moves DataFile updates from popupFragment to page for saving
//This does not seem to collide with the tags updating below
if(systemConfig.isProvCollectionEnabled() && provJsonChanges) {
HashMap<String,ProvPopupFragmentBean.UpdatesEntry> provenanceUpdates = provPopupFragmentBean.getProvenanceUpdates();
for (int i = 0; i < dataset.getFiles().size(); i++) {
for (UpdatesEntry ue : provenanceUpdates.values()) { //for (Map.Entry<String, UpdatesEntry> m : provenanceUpdates.entrySet()) {
if (ue.dataFile.getStorageIdentifier() != null ) {
if (ue.dataFile.getStorageIdentifier().equals(dataset.getFiles().get(i).getStorageIdentifier())) {
dataset.getFiles().set(i, ue.dataFile);
}
}
}
}
}

// Tabular data tags are assigned to datafiles, not to
// version-specfic filemetadatas!
// So if tabular tags have been modified, we also need to
Expand Down Expand Up @@ -1267,15 +1291,15 @@ public String save() {
workingVersion = dataset.getEditVersion();
logger.fine("working version id: "+workingVersion.getId());

if(systemConfig.isProvCollectionEnabled()) {
try {
//If datasetUpdateRequired did not trigger save the prov code will need to save its staged changes.
provPopupFragmentBean.saveStagedProvJson(mode != FileEditMode.UPLOAD); //If not uploading prov needs to save context to save entityId which is a dataFile attribute
} catch (AbstractApiBean.WrappedResponse ex) {
JsfHelper.addErrorMessage(getBundleString("file.metadataTab.provenance.error"));
Logger.getLogger(EditDatafilesPage.class.getName()).log(Level.SEVERE, null, ex);
}
}
// if(systemConfig.isProvCollectionEnabled()) {
// try {
// //If datasetUpdateRequired did not trigger save the prov code will need to save its staged changes.
// provPopupFragmentBean.saveStagedProvJson(mode != FileEditMode.UPLOAD); //If not uploading prov needs to save context to save entityId which is a dataFile attribute
// } catch (AbstractApiBean.WrappedResponse ex) {
// JsfHelper.addErrorMessage(getBundleString("file.metadataTab.provenance.error"));
// Logger.getLogger(EditDatafilesPage.class.getName()).log(Level.SEVERE, null, ex);
// }
// }



Expand Down
19 changes: 14 additions & 5 deletions src/main/java/edu/harvard/iq/dataverse/ProvPopupFragmentBean.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ public void updatePopupState() throws AbstractApiBean.WrappedResponse, IOExcepti
&& !provenanceUpdates.get(popupDataFile.getChecksumValue()).dataFile.getProvEntityName().isEmpty()) {
if(null == provJsonState) { //If the prov json hasn't been updated but other values for the datafile have

//MAD: I think this is right, it should be "permanent" if its not in the temp stored updates.
JsonObject provJsonObject = execCommand(new GetProvJsonCommand(dvRequestService.getDataverseRequest(), popupDataFile));
if(null != provJsonObject) {
provJsonState = provUtil.getPrettyJsonString(provJsonObject);
Expand Down Expand Up @@ -177,7 +176,7 @@ public void updatePopupState() throws AbstractApiBean.WrappedResponse, IOExcepti
public String stagePopupChanges(boolean saveInPopup) throws IOException{
UpdatesEntry stagingEntry = provenanceUpdates.get(popupDataFile.getChecksumValue());
if(stagingEntry == null) {
stagingEntry = new UpdatesEntry(popupDataFile, null, false, null);// = new UpdatesEntry(popupDataFile, null, null);
stagingEntry = new UpdatesEntry(popupDataFile, null, false, null);// (DataFile dataFile, String provJson, Boolean deleteJson, String provFreeform) {
}
if(null == freeformTextInput && null != freeformTextState) {
freeformTextInput = "";
Expand All @@ -198,6 +197,9 @@ public String stagePopupChanges(boolean saveInPopup) throws IOException{
}

if(null != dropdownSelectedEntity && !(null != storedSelectedEntityName && storedSelectedEntityName.equals(dropdownSelectedEntity.getEntityName()))) {
if(null == stagingEntry.provJson) {
stagingEntry.provJson = provJsonState; //this will trigger the prov json save command to save the entity if it was set without a json update
}
popupDataFile.setProvEntityName(dropdownSelectedEntity.getEntityName());
}

Expand Down Expand Up @@ -253,20 +255,24 @@ public void addSuccessMessageToPage(boolean saveInPopup) {
JsfHelper.addSuccessMessage(message); //We have to call this after to ensure it is the success message shown
}

public void saveStagedProvJson(boolean saveContext) throws AbstractApiBean.WrappedResponse {
public boolean saveStagedProvJson(boolean saveContext) throws AbstractApiBean.WrappedResponse {
boolean commandsCalled = false;
for (Map.Entry<String, UpdatesEntry> m : provenanceUpdates.entrySet()) {
UpdatesEntry mapEntry = m.getValue();
DataFile df = mapEntry.dataFile;
String provString = mapEntry.provJson;

if(mapEntry.deleteJson) {
df = execCommand(new DeleteProvJsonCommand(dvRequestService.getDataverseRequest(), df, saveContext));
commandsCalled = true;
} else if(null != provString) {
df = execCommand(new PersistProvJsonCommand(dvRequestService.getDataverseRequest(), df, provString, df.getProvEntityName(), saveContext));
commandsCalled = true;
}
mapEntry.dataFile = df;
provenanceUpdates.put(mapEntry.dataFile.getChecksumValue(), mapEntry); //Updates the datafile to the latest.
}
return commandsCalled;
}

public void saveStageProvFreeformToLatestVersion() {
Expand All @@ -282,7 +288,6 @@ Boolean updatePageMetadatasWithProvFreeform(List<FileMetadata> fileMetadatas) {
Boolean changes = false;
for(FileMetadata fm : fileMetadatas) {
UpdatesEntry ue = provenanceUpdates.get(fm.getDataFile().getChecksumValue());
//MAD: Does this null check for provFreeform stop provFreeform from being cleared?
if(null != ue && null != ue.provFreeform) {
fm.setProvFreeForm(ue.provFreeform);
changes = true;
Expand Down Expand Up @@ -377,7 +382,7 @@ public void setDropdownSelectedEntity(ProvEntityFileData entity) {
}

public void generateProvJsonParsedEntities() throws IOException {
provJsonParsedEntities = new HashMap<>(); //MAD: DID NOTHING
provJsonParsedEntities = new HashMap<>();
provJsonParsedEntities = provUtil.startRecurseNames(provJsonState);
}

Expand All @@ -402,6 +407,10 @@ public ProvEntityFileData getEntityByEntityName(String entityName) {
return provJsonParsedEntities.get(entityName);
}

public HashMap<String,UpdatesEntry> getProvenanceUpdates() {
return provenanceUpdates;
}

//for storing datafile and provjson in a map value
class UpdatesEntry {
String provJson;
Expand Down

0 comments on commit fe21ceb

Please sign in to comment.