[client] Image/Lectures: only push metadata/permissions if they were changed

1 files changed, 78 insertions, 48 deletions

diff --git a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/ImageDetailsWindow.java b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/ImageDetailsWindow.java
index 254b8767..a3255c57 100644
--- a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/ImageDetailsWindow.java
+++ b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/ImageDetailsWindow.java
@@ -23,7 +23,6 @@ import javax.swing.event.DocumentEvent;
 import javax.swing.event.DocumentListener;
 
 import org.apache.log4j.Logger;
-import org.apache.thrift.TException;
 import org.openslx.bwlp.thrift.iface.ImageBaseWrite;
 import org.openslx.bwlp.thrift.iface.ImageDetailsRead;
 import org.openslx.bwlp.thrift.iface.ImagePermissions;
@@ -42,9 +41,7 @@ import org.openslx.dozmod.gui.window.UserListWindow.UserAddedCallback;
 import org.openslx.dozmod.gui.window.layout.ImageDetailsWindowLayout;
 import org.openslx.dozmod.gui.wizard.ImageUpdateWizard;
 import org.openslx.dozmod.gui.wizard.LectureWizard;
-import org.openslx.dozmod.permissions.DefaultCustomPerms;
 import org.openslx.dozmod.permissions.ImagePerms;
-import org.openslx.dozmod.permissions.PermsHelper;
 import org.openslx.dozmod.thrift.Session;
 import org.openslx.dozmod.thrift.ThriftActions;
 import org.openslx.dozmod.thrift.ThriftActions.DeleteCallback;
@@ -52,7 +49,7 @@ import org.openslx.dozmod.thrift.ThriftActions.ImageMetaCallback;
 import org.openslx.dozmod.thrift.cache.MetaDataCache;
 import org.openslx.dozmod.thrift.cache.UserCache;
 import org.openslx.dozmod.util.FormatHelper;
-import org.openslx.thrifthelper.ThriftManager;
+import org.openslx.dozmod.util.MapHelper;
 
 /**
  * Window for displaying and editing the details of an image.
@@ -106,6 +103,9 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	private JMenuItem popupItemDownload = new JMenuItem("Download");
 	private JMenuItem popupItemDelete = new JMenuItem("Löschen");
 
+	private boolean metadataChanged = false;
+	private boolean permissionsChanged = false;
+
 	/**
 	 * Constructor
 	 * 
@@ -166,11 +166,10 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 			@Override
 			public void actionPerformed(ActionEvent arg0) {
 				// open the custom permission window and save returned default/custom permissions
-				DefaultCustomPerms<ImagePermissions> combined = ImagePermissionWindow.open(me,
-						customPermissions, image.defaultPermissions, image.ownerId);
-						// since the window above gets references of the default/custom permission object
-						// there is no need to further save the return value ...
-						// THAT or we do work with copies in ImagePermissionWindow ...
+				ImagePermissionWindow.open(me, customPermissions, image.defaultPermissions, image.ownerId);
+				// since the window above gets references of the default/custom permission object
+				// there is no need to further save the return value ...
+				// THAT or we do work with copies in ImagePermissionWindow ...
 
 				// for now let's stay with refs and just reactToChange() where we check if
 				// the permissions stuff changed since the last call of fill()
@@ -241,17 +240,7 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 		versionTableScrollPane.addMouseListener(ma);
 		versionTable.addMouseListener(ma);
 
-		/**
-		 * Initial state of GUI elements
-		 */
-		setFocusable(true);
-		btnSaveChanges.setEnabled(false);
-		txtVersion.setEditable(false);
-		txtId.setEditable(false);
-		chkIsTemplate.setEnabled(Session.isSuperUser());
-		makeEditable(false);
-		// TODO finish ShareMode in server
-		cboShareMode.setEnabled(false);
+	
 
 		// listen to changes
 		final DocumentListener docListener = new DocumentListener() {
@@ -290,6 +279,18 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 		cboOperatingSystem.addItemListener(comboItemListener);
 		cboShareMode.addItemListener(comboItemListener);
 		chkIsTemplate.addActionListener(checkBoxListener);
+		
+		/**
+		 * Initial state of GUI elements
+		 */
+		setFocusable(true);
+		btnSaveChanges.setEnabled(false);
+		txtVersion.setEditable(false);
+		txtId.setEditable(false);
+		chkIsTemplate.setEnabled(Session.isSuperUser());
+		makeEditable(false);
+		// TODO finish ShareMode in server
+		cboShareMode.setEnabled(false);
 	}
 
 	/********************************************************************************
@@ -325,7 +326,7 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 					customPermissions = permissions;
 
 				}
-				fill();
+				fillDetails();
 			}
 		};
 		ThriftActions.getImageFullDetails(JOptionPane.getFrameForComponent(me), imageBaseId, callback);
@@ -348,9 +349,28 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	}
 
 	/**
-	 * Pushes the changes of the image details to the satellite
+	 * Called by the "Save" button, tries to save the changes internally and
+	 * then react based depending on the outcome of the save 
 	 */
 	private void saveChanges() {
+		boolean saved = saveChangesInternal();
+		// if there was nothing to save, saved would be true from the above call
+		// however we wouldn't even get to saving if nothing was changed, so its fine
+		if (saved) {
+			callback.updated();
+			dispose();
+		} else {
+			btnSaveChanges.setEnabled(true);
+		}
+
+	}
+
+	/**
+	 * Helper to only save the changes, nothing else. Updating GUI elements is done by saveChanges()
+	 *
+	 * @return false if any try to save changes failed, true otherwise
+	 */
+	private boolean saveChangesInternal() {
 		// first build the ImageBaseWrite from the GUI fields
 		final ImageBaseWrite ibw = new ImageBaseWrite(txtTitle.getText(), txtDescription.getText(),
 				cboOperatingSystem.getItemAt(cboOperatingSystem.getSelectedIndex()).osId, image.virtId,
@@ -359,21 +379,25 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 						image.defaultPermissions.edit, image.defaultPermissions.admin),
 				cboShareMode.getItemAt(cboShareMode.getSelectedIndex()));
 		// now trigger the actual action
-		if (!ThriftActions.updateImageBase(JOptionPane.getFrameForComponent(me), image.getImageBaseId(), ibw))
-			return;
-		if (PermsHelper.hasChanged(originalCustomPermissions, customPermissions)) {
-			try {
-				ThriftManager.getSatClient().writeImagePermissions(Session.getSatelliteToken(),
-						image.imageBaseId, customPermissions);
-			} catch (TException e) {
-				LOGGER.error("Fehler beim Übertragen der benutzerspezifischen Berechtigungen: ", e);
+		if (metadataChanged) {
+			if (ThriftActions.updateImageBase(JOptionPane.getFrameForComponent(me), image.getImageBaseId(),
+					ibw)) {
+				LOGGER.info("Successfully saved new metadata");
+				metadataChanged = false;
+			} else {
+				return false;
 			}
 		}
-		// success
-		LOGGER.info("Successfully saved changes to satellite.");
-		btnSaveChanges.setEnabled(false);
-		callback.updated();
-		dispose();
+		if (permissionsChanged) {
+			if (ThriftActions.writeImagePermissions(JOptionPane.getFrameForComponent(me),
+					image.getImageBaseId(), customPermissions)) {
+				LOGGER.info("Successfully saved new permissions");
+				permissionsChanged = false;
+			} else {
+				return false;
+			}
+		}
+		return true;
 	}
 
 	/**
@@ -426,7 +450,7 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	/**
 	 * callback function when we received the image's details from the server
 	 */
-	private void fill() {
+	private void fillDetails() {
 		if (image == null)
 			return;
 		// remember default permissions
@@ -453,8 +477,9 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 		lblUpdateTime.setText(FormatHelper.longDate(image.getUpdateTime()));
 		txtVersion.setText(image.getLatestVersionId());
 		txtId.setText(image.getImageBaseId());
+		chkIsTemplate.setSelected(image.isTemplate);
 		setTitle(image.getImageName());
-
+		
 		// fill os combo, but only once :)
 		if (cboOperatingSystem.getItemCount() == 0) {
 			List<OperatingSystem> osList = MetaDataCache.getOperatingSystems();
@@ -487,7 +512,6 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 			tagsString = tagsString + ", " + tag;
 		}
 		txtTags.setText(tagsString);
-		chkIsTemplate.setSelected(image.isTemplate);
 
 		// set the versions of the image to the table
 		versionTable.setData(image.getVersions(), true);
@@ -502,9 +526,15 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	 * enables the save button if so
 	 * TODO TAGS
 	 */
-	private boolean reactToChange() {
-		// disable save by default here, we will enable it at the end if all checks passed
-		btnSaveChanges.setEnabled(false);
+	private void reactToChange() {
+		metadataChanged = reactToChangeInternal();
+		permissionsChanged = MapHelper.hasChanged(originalCustomPermissions, customPermissions);
+		btnSaveChanges.setEnabled(metadataChanged || permissionsChanged);
+	}
+
+	private boolean reactToChangeInternal() {
+		if (image == null)
+			return false;
 		boolean changed = false;
 		// Image name
 		if (txtTitle.getText().isEmpty()) {
@@ -530,17 +560,17 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 			lblError.setText("Kein Share-Modus ausgewählt!");
 			return false;
 		}
+		// mandatory checks done, remove error message
+		lblError.setText(null);
+
 		// Template, default permissions and custom permissions:
 		// no sanity checks here only check if they changed
 		changed = !txtTitle.getText().equals(image.getImageName())
 				|| !txtDescription.getText().equals(image.getDescription())
 				|| newOs.getOsId() != image.getOsId() || !newShareMode.equals(image.shareMode)
 				|| chkIsTemplate.isSelected() != image.isTemplate
-				|| !image.defaultPermissions.equals(originalDefaultPermissions)
-				|| PermsHelper.hasChanged(originalCustomPermissions, customPermissions);
+				|| !image.defaultPermissions.equals(originalDefaultPermissions);
 
-		lblError.setText(null);
-		btnSaveChanges.setEnabled(changed);
 		return changed;
 	}
 
@@ -596,7 +626,7 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	 ********************************************************************************/
 	@Override
 	public boolean wantConfirmQuit() {
-		return reactToChange() && btnSaveChanges.isEnabled();
+		return metadataChanged || permissionsChanged ;
 	}
 
 	@Override
@@ -609,9 +639,9 @@ public class ImageDetailsWindow extends ImageDetailsWindowLayout implements UiFe
 	 * confirmation if so
 	 */
 	private void safeClose() {
-		if (reactToChange()
+		if ((metadataChanged || permissionsChanged)
 				&& !Gui.showMessageBox(me, "Änderungen werden verworfen, wollen Sie wirklich abbrechen?",
-						MessageType.QUESTION_YESNO, LOGGER, null))
+						MessageType.QUESTION_YESNO, null, null))
 			return;
 		dispose();
 	}