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

1 files changed, 108 insertions, 64 deletions

diff --git a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/LectureDetailsWindow.java b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/LectureDetailsWindow.java
index 0fc05ee8..17316387 100644
--- a/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/LectureDetailsWindow.java
+++ b/dozentenmodul/src/main/java/org/openslx/dozmod/gui/window/LectureDetailsWindow.java
@@ -13,9 +13,11 @@ import java.util.Calendar;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
 import javax.swing.DefaultComboBoxModel;
 import javax.swing.JFrame;
@@ -41,7 +43,6 @@ import org.openslx.dozmod.gui.helper.MessageType;
 import org.openslx.dozmod.gui.helper.UiFeedback;
 import org.openslx.dozmod.gui.window.UserListWindow.UserAddedCallback;
 import org.openslx.dozmod.gui.window.layout.LectureDetailsWindowLayout;
-import org.openslx.dozmod.permissions.DefaultCustomPerms;
 import org.openslx.dozmod.permissions.ImagePerms;
 import org.openslx.dozmod.permissions.LecturePerms;
 import org.openslx.dozmod.thrift.Session;
@@ -51,6 +52,7 @@ import org.openslx.dozmod.thrift.ThriftActions.LectureMetaCallback;
 import org.openslx.dozmod.thrift.ThriftError;
 import org.openslx.dozmod.thrift.cache.UserCache;
 import org.openslx.dozmod.util.FormatHelper;
+import org.openslx.dozmod.util.MapHelper;
 import org.openslx.thrifthelper.ThriftManager;
 
 /**
@@ -86,20 +88,28 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 	/**
 	 * The custom permissions of the lecture
 	 */
-	private Map<String, LecturePermissions> permissionMap;
+	private Map<String, LecturePermissions> customPermissions;
 
 	/**
-	 * Image, that the lecture is linked to.
+	 * The original custom permissions as fetched from the server
 	 */
-	private ImageDetailsRead image = null;
+	private Map<String, LecturePermissions> originalCustomPermissions;
 
 	/**
-	 * Whether the custom permission window has been used.
+	 * The original default permissions as fetched from the server
+	 */
+	private LecturePermissions originalDefaultPermissions;
+	/**
+	 * Image, that the lecture is linked to.
 	 */
-	private boolean permissionsChanged = false;
+	private ImageDetailsRead image = null;
 
 	private boolean imageLinkChanged = false;
 
+	private boolean metadataChanged = false;
+
+	private boolean permissionsChanged;
+
 	/**
 	 * Constructor
 	 * 
@@ -144,7 +154,7 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 		btnClose.addActionListener(new ActionListener() {
 			@Override
 			public void actionPerformed(ActionEvent e) {
-				if (reactToChange() && !Gui.showMessageBox(me,
+				if (metadataChanged && !Gui.showMessageBox(me,
 						"Änderungen werden verworfen, wollen Sie wirklich abbrechen?",
 						MessageType.QUESTION_YESNO, LOGGER, null))
 					return;
@@ -182,15 +192,11 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 		btnPermissions.addActionListener(new ActionListener() {
 			@Override
 			public void actionPerformed(ActionEvent arg0) {
-				LOGGER.info(permissionMap);
-				LOGGER.info(lecture.defaultPermissions);
-				DefaultCustomPerms<LecturePermissions> pl = LecturePermissionWindow.open(me, permissionMap,
-						lecture.defaultPermissions, lecture.ownerId);
-				if (pl != null && pl.defaultPermissions != null && pl.customPermissions != null) {
-					lecture.defaultPermissions = pl.defaultPermissions;
-					permissionMap = pl.customPermissions;
-					permissionsChanged = true;
-				}
+				// NOTE once the following window is closed, customPermissions and lecture.defaultPermissions
+				// objects will contain the changes the user did. We will later only compare these with
+				// the original values as saved in originalCustomPermissions and originalDefaultPermissions
+				LecturePermissionWindow.open(me, customPermissions, lecture.defaultPermissions,
+						lecture.ownerId);
 				reactToChange();
 			}
 		});
@@ -263,8 +269,10 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 	}
 
 	/**
-	 * Sets the lecture to show the details of by setting the 'lecture' and 'image'
-	 * members to its metadata. This method will fetch the information from the sat
+	 * Sets the lecture to show the details of by setting the 'lecture' and
+	 * 'image'
+	 * members to its metadata. This method will fetch the information from the
+	 * sat
 	 *
 	 * @param lectureId the id of the lecture to be displayed
 	 */
@@ -278,11 +286,11 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 							lecture = lectureDetails;
 							image = imageDetails;
 							if (lecture != null) {
-								permissionMap = ThriftActions.getLecturePermissions(
+								customPermissions = ThriftActions.getLecturePermissions(
 										JOptionPane.getFrameForComponent(me), lecture.lectureId);
 							}
 						}
-						fill();
+						fillDetails();
 					}
 				});
 	}
@@ -290,7 +298,7 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 	/**
 	 * callback function when we received the lecture's details from the server
 	 */
-	private void fill() {
+	private void fillDetails() {
 		if (lecture == null) {
 			txtTitle.setText("-");
 			makeEditable(false);
@@ -301,6 +309,22 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 		} else {
 			txtImageName.setText(image.getImageName());
 		}
+		// remember default permissions
+		if (lecture.defaultPermissions != null) {
+			originalDefaultPermissions = new LecturePermissions(lecture.defaultPermissions);
+		}
+		// remember custom permissions
+		if (customPermissions != null) {
+			// need a deep copy of the permission map to be able to check for changes after ImageCustomPermissionWindow
+			if (originalCustomPermissions == null)
+				originalCustomPermissions = new HashMap<String, LecturePermissions>();
+			else
+				originalCustomPermissions.clear();
+			// fill it
+			for (Entry<String, LecturePermissions> entry : customPermissions.entrySet()) {
+				originalCustomPermissions.put(entry.getKey(), new LecturePermissions(entry.getValue()));
+			}
+		}
 		txtTitle.setText(lecture.getLectureName());
 		txtDescription.setText(lecture.getDescription());
 		lblOwner.setUser(UserCache.find(lecture.getOwnerId()));
@@ -436,47 +460,56 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 	 * Push the changes of the image details to the satellite
 	 */
 	private void saveChanges() {
+		boolean saved = saveChangesInternal();
+		callback.updated(saved);
+		if (saved)
+			dispose();
+		else
+			btnSaveChanges.setEnabled(true);
+	}
+
+	private boolean saveChangesInternal() {
 		// check, whether autoupdate is selected and choose version accordingly
 		if (image != null) {
 			lecture.imageVersionId = chkAutoUpdate.isSelected() ? image.latestVersionId
 					: cboVersions.getItemAt(cboVersions.getSelectedIndex()).versionId;
 		}
-
-		// date valid TODO sat max time
-		if (!isPeriodValid(DateTimeHelper.getDateFrom(startDate, startTime),
-				DateTimeHelper.getDateFrom(endDate, endTime), true)) {
-			return;
-		}
-		// first build the ImageBaseWrite from the GUI fields
-		final LectureWrite lectureWrite = new LectureWrite(txtTitle.getText(), txtDescription.getText(),
-				lecture.getImageVersionId(), chkAutoUpdate.isSelected(), chkIsActive.isSelected(),
-				DateTimeHelper.getDateFrom(startDate, startTime).getTime() / 1000L,
-				DateTimeHelper.getDateFrom(endDate, endTime).getTime() / 1000L, null, null,
-				chkIsExam.isSelected(), chkHasInternetAccess.isSelected(), lecture.getDefaultPermissions());
-		// now trigger the actual action 
-		btnSaveChanges.setEnabled(false);
-		try {
-			ThriftManager.getSatClient().updateLecture(Session.getSatelliteToken(), lecture.getLectureId(),
-					lectureWrite);
-		} catch (TException e) {
-			ThriftError.showMessage(JOptionPane.getFrameForComponent(this), LOGGER, e,
-					"Fehler beim Updaten der Veranstaltung");
-			callback.updated(true);
-			return;
+		// now check if we need to push a new LectureWrite
+		if (metadataChanged) {
+			// first build the LectureWrite from the GUI fields
+			final LectureWrite metadata = new LectureWrite(txtTitle.getText(), txtDescription.getText(),
+					lecture.getImageVersionId(), chkAutoUpdate.isSelected(), chkIsActive.isSelected(),
+					DateTimeHelper.getDateFrom(startDate, startTime).getTime() / 1000L,
+					DateTimeHelper.getDateFrom(endDate, endTime).getTime() / 1000L, null, null,
+					chkIsExam.isSelected(), chkHasInternetAccess.isSelected(),
+					lecture.getDefaultPermissions());
+			// now trigger the actual action 
+			try {
+				ThriftManager.getSatClient().updateLecture(Session.getSatelliteToken(),
+						lecture.getLectureId(), metadata);
+				metadataChanged = false;
+				LOGGER.info("Successfully save new metadata");
+			} catch (TException e) {
+				ThriftError.showMessage(JOptionPane.getFrameForComponent(this), LOGGER, e,
+						"Fehler beim Updaten der Veranstaltung!");
+				callback.updated(false);
+				return false;
+			}
 		}
 		if (permissionsChanged) {
 			try {
 				ThriftManager.getSatClient().writeLecturePermissions(Session.getSatelliteToken(),
-						lecture.lectureId, permissionMap);
+						lecture.lectureId, customPermissions);
 				permissionsChanged = false;
+				LOGGER.info("Successfully save new permissions");
 			} catch (TException e) {
-				LOGGER.error("Fehler beim Übertragen der Berechtigungen: ", e);
+				ThriftError.showMessage(JOptionPane.getFrameForComponent(this), LOGGER, e,
+						"Fehler beim Übertragen der Berechtigungen!");
+				callback.updated(true);
+				return false;
 			}
 		}
-		btnSaveChanges.setEnabled(true);
-		callback.updated(true);
-		dispose();
-		// success
+		return true;
 	}
 
 	/**
@@ -518,12 +551,22 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 	 * Checks whether the user changed any fields of the image details and
 	 * enables the save button if so.
 	 */
-	private boolean reactToChange() {
+	private void reactToChange() {
+		// check details fields
+		metadataChanged = reactToChangeInternal();
+		permissionsChanged = MapHelper.hasChanged(originalCustomPermissions, customPermissions);
+		btnSaveChanges.setEnabled(metadataChanged || permissionsChanged);
+	}
+
+	/**
+	 * Checks whether the user changed any fields of the image details and
+	 * enables the save button if so.
+	 */
+	private boolean reactToChangeInternal() {
 		if (lecture == null)
 			return false;
-		
+
 		boolean changed = false;
-		btnSaveChanges.setEnabled(false);
 		// mandatory fields checks
 		if (txtTitle.getText().isEmpty()) {
 			lblError.setText("Kein Imagename!");
@@ -531,7 +574,7 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 		}
 		if (txtDescription.getText().isEmpty()) {
 			lblError.setText("Keine Beschreibung!");
-			return false;				
+			return false;
 		}
 		// version checkbox changed?
 		ImageVersionDetails currentVersion = cboVersions.getItemAt(cboVersions.getSelectedIndex());
@@ -544,24 +587,25 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 		Date end = DateTimeHelper.getDateFrom(endDate, endTime);
 		if (!isPeriodValid(start, end, false)) {
 			lblError.setText("Ungültiger Zeitraum!");
-			return false;				
+			return false;
 		}
-		// done with mandatory checks, now only check for changes
-		// there first remove error message
+
+		// done with mandatory checks, remove error message
 		lblError.setText(null);
+
+		// check for changes in all fields
 		changed = !txtTitle.getText().equals(lecture.getLectureName())
 				|| !txtDescription.getText().equals(lecture.getDescription())
 				|| !currentVersion.getVersionId().equals(lecture.getImageVersionId())
-				|| (start.getTime() / 1000L) != lecture.getStartTime()
-				|| (end.getTime() / 1000L) != lecture.getEndTime()
+				|| (DateTimeHelper.getDateFrom(startDate, startTime).getTime() / 1000L) != lecture
+						.getStartTime()
+				|| (DateTimeHelper.getDateFrom(endDate, endTime).getTime() / 1000L) != lecture.getEndTime()
 				|| chkAutoUpdate.isSelected() != lecture.autoUpdate
 				|| chkIsExam.isSelected() != lecture.isExam
 				|| chkHasInternetAccess.isSelected() != lecture.hasInternetAccess
 				|| chkIsActive.isSelected() != lecture.isEnabled
-				|| permissionsChanged
-				|| imageLinkChanged;
-		
-		btnSaveChanges.setEnabled(changed);
+				|| !lecture.defaultPermissions.equals(originalDefaultPermissions) || imageLinkChanged;
+
 		return changed;
 	}
 
@@ -619,7 +663,7 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 	 ********************************************************************************/
 	@Override
 	public boolean wantConfirmQuit() {
-		return reactToChange();
+		return metadataChanged || permissionsChanged;
 	}
 
 	@Override
@@ -633,7 +677,7 @@ public class LectureDetailsWindow extends LectureDetailsWindowLayout implements
 	 * confirmation if so
 	 */
 	private void safeClose() {
-		if (reactToChange()
+		if ((metadataChanged || permissionsChanged)
 				&& !Gui.showMessageBox(me, "Änderungen werden verworfen, wollen Sie wirklich abbrechen?",
 						MessageType.QUESTION_YESNO, null, null))
 			return;