Data Migration: Setter changes are persisted before data verification checks

[marquestye]

In some logic class methods,

1. an entity is retrieved from the database,
2. the entity's fields are updated with new values, before
3. the entity is passed into an db.updateEntity() method, which then performs the verification checks.

For example:

teammates/src/main/java/teammates/sqllogic/core/FeedbackQuestionsLogic.java

Lines 193 to 211 in b697768

	// update question
	question.setQuestionNumber(updateRequest.getQuestionNumber());
	question.setDescription(updateRequest.getQuestionDescription());
	question.setQuestionDetails(updateRequest.getQuestionDetails());
	question.setGiverType(updateRequest.getGiverType());
	question.setRecipientType(updateRequest.getRecipientType());
	question.setNumOfEntitiesToGiveFeedbackTo(updateRequest.getNumberOfEntitiesToGiveFeedbackTo());
	question.setShowResponsesTo(updateRequest.getShowResponsesTo());
	question.setShowGiverNameTo(updateRequest.getShowGiverNameTo());
	question.setShowRecipientNameTo(updateRequest.getShowRecipientNameTo());
	// validate questions (giver & recipient)
	String err = question.getQuestionDetailsCopy().validateGiverRecipientVisibility(question);
	if (!err.isEmpty()) {
	throw new InvalidParametersException(err);
	}
	// validate questions (question details)
	FeedbackQuestionDetails questionDetails = question.getQuestionDetailsCopy();
	List<String> questionDetailsErrors = questionDetails.validateQuestionDetails();

teammates/src/main/java/teammates/sqllogic/core/FeedbackResponsesLogic.java

Lines 205 to 214 in b697768

	for (FeedbackResponseComment oldResponseComment : oldResponseComments) {
	if (isGiverSectionChanged) {
	oldResponseComment.setGiverSection(newResponse.getGiverSection());
	}
	if (isRecipientSectionChanged) {
	oldResponseComment.setRecipientSection(newResponse.getRecipientSection());
	}
	frcLogic.updateFeedbackResponseComment(oldResponseComment);

However, according to the Hibernate documentation, changes made by the setters are automatically persisted into the database, even though the new values could be invalid and fail the verification checks. This results in the database entity being polluted with invalid data.

I've tested this locally on NotificationsLogic.updateNotification:

teammates/src/main/java/teammates/sqllogic/core/NotificationsLogic.java

Lines 81 to 90 in b697768

	notification.setStartTime(startTime);
	notification.setEndTime(endTime);
	notification.setStyle(style);
	notification.setTargetUser(targetUser);
	notification.setTitle(title);
	notification.setMessage(message);
	if (!notification.isValid()) {
	throw new InvalidParametersException(notification.getInvalidityInfo());
	}

    @Test
    public void testUpdateNotification_invalidParameters_originalUnchanged() throws Exception {

        Notification notif = new Notification(
                Instant.parse("2011-01-01T00:00:00Z"),
                Instant.parse("2099-01-01T00:00:00Z"),
                NotificationStyle.DANGER,
                NotificationTargetUser.GENERAL,
                "A deprecation note",
                "<p>Deprecation happens in three minutes</p>");
        notificationsLogic.createNotification(notif);

        String invalidLongTitle = "1234567890".repeat(10);

        assertThrows(
                InvalidParametersException.class,
                () -> notificationsLogic.updateNotification(
                        notif.getId(),
                        Instant.parse("2011-01-01T00:00:00Z"),
                        Instant.parse("2099-01-01T00:00:00Z"),
                        NotificationStyle.DANGER,
                        NotificationTargetUser.GENERAL,
                        invalidLongTitle,
                        "<p>Deprecation happens in three minutes</p>"
                )
        );

        Notification actual = notificationsLogic.getNotification(notif.getId());
        assert actual.getTitle().equals("A deprecation note") : actual.getTitle();
        // java.lang.AssertionError: 12345678901234567890...

    }

The update fails, but the entity retrieved from the database afterwards is still updated with invalidLongTitle.

[jayasting98]

The only solutions I can think of are to validate before using the setters, or evicting the object from the session cache.

void evict(Object object) throws HibernateException
Remove this instance from the session cache. Changes to the instance will not be synchronized with the database. This operation cascades to associated instances if the association is mapped with cascade="evict".

See also:

• https://stackoverflow.com/a/46118379