diff --git a/catroid/build.gradle b/catroid/build.gradle index 8789356a11e..f3617aa4c45 100644 --- a/catroid/build.gradle +++ b/catroid/build.gradle @@ -103,9 +103,9 @@ def defaultVersionCode = 99 def defaultVersionName = "1.3.2" android { - compileSdk = 35 + compileSdkVersion 35 - namespace = 'org.catrobat.catroid' + namespace 'org.catrobat.catroid' buildFeatures { buildConfig = true diff --git a/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/ScriptUndoIntegrationTest.kt b/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/ScriptUndoIntegrationTest.kt new file mode 100644 index 00000000000..0d316c84d3f --- /dev/null +++ b/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/ScriptUndoIntegrationTest.kt @@ -0,0 +1,332 @@ +/* + * Catroid: An on-device visual programming system for Android devices + * Copyright (C) 2010-2026 The Catrobat Team + * () + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * An additional term exception under section 7 of the GNU Affero + * General Public License, version 3, is available at + * http://developer.catrobat.org/license_additional_term + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package org.catrobat.catroid.uiespresso.ui.fragment + +import android.view.View +import androidx.test.espresso.Espresso.onView +import androidx.test.espresso.Espresso.pressBack +import androidx.test.espresso.UiController +import androidx.test.espresso.ViewAction +import androidx.test.espresso.action.ViewActions.click +import androidx.test.espresso.assertion.ViewAssertions.matches +import androidx.test.espresso.matcher.ViewMatchers.isAssignableFrom +import androidx.test.espresso.matcher.ViewMatchers.isDisplayed +import androidx.test.espresso.matcher.ViewMatchers.isEnabled +import androidx.test.espresso.matcher.ViewMatchers.withId +import androidx.test.espresso.matcher.ViewMatchers.withText +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.catrobat.catroid.ProjectManager +import org.catrobat.catroid.R +import org.catrobat.catroid.WaitForConditionAction.Companion.waitFor +import org.catrobat.catroid.content.bricks.IfLogicBeginBrick +import org.catrobat.catroid.content.bricks.SetXBrick +import org.catrobat.catroid.io.XstreamSerializer +import org.catrobat.catroid.test.utils.TestUtils +import org.catrobat.catroid.ui.SpriteActivity +import org.catrobat.catroid.ui.dragndrop.BrickListView +import org.catrobat.catroid.uiespresso.content.brick.utils.BrickDataInteractionWrapper.onBrickAtPosition +import org.catrobat.catroid.uiespresso.util.UiTestUtils +import org.catrobat.catroid.uiespresso.util.rules.FragmentActivityTestRule +import org.hamcrest.Matcher +import org.hamcrest.Matchers.allOf +import org.hamcrest.Matchers.not +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Integration-style Espresso tests that exercise the full production wiring + * for each undo-able script action: copy, move, comment out, comment in, + * delete, and cancel-move. + * + * Unlike [ScriptUndoRegressionTest][org.catrobat.catroid.test.ui.ScriptUndoRegressionTest] + * (which drives ProjectUndoManager directly via pushState / popUndo), these tests + * go through the real UI layer and verify that the undo button becomes enabled + * through the actual ScriptFragment / BrickListView / ProjectUndoManager chain. + * + * Project layout after setUp (brick positions): + * 0: WhenStartedBrick (script header) + * 1: IfLogicBeginBrick + * 2: SetXBrick (inside if-branch) + * 3: IfLogicElseBrick + * 4: SetXBrick (inside else-branch) + * 5: IfLogicEndBrick + * 6: SetXBrick (standalone, after the if) + */ +@RunWith(AndroidJUnit4::class) +class ScriptUndoIntegrationTest { + + private val waitThreshold = 5000L + + @get:Rule + val baseActivityTestRule = FragmentActivityTestRule( + SpriteActivity::class.java, + SpriteActivity.EXTRA_FRAGMENT_POSITION, + SpriteActivity.FRAGMENT_SCRIPTS + ) + + private lateinit var initialProject: String + + @Before + fun setUp() { + createProject() + baseActivityTestRule.launchActivity() + } + + @After + fun tearDown() { + TestUtils.deleteProjects(ScriptUndoIntegrationTest::class.java.simpleName) + } + + @Test + fun testCopyBrickUndoRedo() { + onBrickAtPosition(6).performClick() + onView(withText(R.string.brick_context_dialog_copy_brick)) + .perform(click()) + + // Commit the hovering brick by calling stopMoving on the BrickListView + dropHoveringBrick() + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_undo)) + .check(matches(isEnabled())) + + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_redo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun testAddBrickUndoRedo() { + onView(withId(R.id.button_add)).perform(click()) + onView(withText(R.string.category_motion)).perform(click()) + onView(withText(R.string.brick_place_at)).perform(click()) + + // Adding a brick via AddBrickFragment triggers startMoving() in ScriptFragment + dropHoveringBrick() + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_undo)) + .check(matches(isEnabled())) + + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_redo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun testCopyControlStructureUndoRedo() { + onBrickAtPosition(0).performClick() + onView(withText(R.string.brick_context_dialog_copy_script)) + .perform(click()) + + dropHoveringBrick() + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_redo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun testCommentOutBrickUndoRedo() { + onBrickAtPosition(6).performClick() + onView(withText(R.string.brick_context_dialog_comment_out)) + .perform(click()) + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_undo)) + .check(matches(isEnabled())) + + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_redo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun testCommentInScriptUndoRedo() { + onBrickAtPosition(0).performClick() + onView(withText(R.string.brick_context_dialog_comment_out_script)) + .perform(click()) + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + onBrickAtPosition(0).performClick() + onView(withText(R.string.brick_context_dialog_comment_in_script)) + .perform(click()) + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + // Undo the comment-in + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + // Undo the comment-out + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_redo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun testDeleteBrickUndoRedo() { + onBrickAtPosition(6).performDeleteBrick() + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isDisplayed(), waitThreshold)) + + onView(withId(R.id.menu_undo)).perform(click()) + + onView(withId(R.id.menu_redo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun testMoveBrickUndoRedo() { + onBrickAtPosition(6).performClick() + onView(withText(R.string.brick_context_dialog_move_brick)) + .perform(click()) + + dropHoveringBrick() + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_undo)) + .check(matches(isEnabled())) + + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_redo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun testMoveControlStructureUndoRedo() { + onBrickAtPosition(0).performClick() + onView(withText(R.string.brick_context_dialog_move_script)) + .perform(click()) + + dropHoveringBrick() + + onView(withId(R.id.menu_undo)) + .perform(waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_undo)) + .check(matches(isEnabled())) + + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_redo)) + .perform(waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun testCancelMoveDoesNotCreateUndoEntry() { + onBrickAtPosition(6).performClick() + onView(withText(R.string.brick_context_dialog_move_brick)) + .perform(click()) + + // Cancel the move via back-press instead of dropping + pressBack() + + onView(withId(R.id.menu_undo)) + .check(matches(not(isEnabled()))) + } + + /** + * Commits a hovering brick by calling [BrickListView.stopMoving] directly. + * This is the same production path triggered when the user lifts their finger + * (ACTION_UP) during a drag-and-drop operation. + * + * We use a custom [ViewAction] that finds the [BrickListView] and calls + * stopMoving() on it, which is safe because Espresso runs ViewActions on the + * main thread. + */ + private fun dropHoveringBrick() { + onView(allOf(isAssignableFrom(BrickListView::class.java), isDisplayed())) + .perform(object : ViewAction { + override fun getConstraints(): Matcher = + allOf(isAssignableFrom(BrickListView::class.java), isDisplayed()) + + override fun getDescription(): String = "call stopMoving() on BrickListView" + + override fun perform(uiController: UiController, view: View) { + (view as BrickListView).stopMoving() + uiController.loopMainThreadUntilIdle() + } + }) + } + + private fun getProjectAsXmlString(): String = + XstreamSerializer.getInstance() + .getXmlAsStringFromProject(ProjectManager.getInstance().currentProject) + + private fun createProject() { + val script = UiTestUtils.createProjectAndGetStartScript( + ScriptUndoIntegrationTest::class.java.simpleName + ) + + val ifBrick = IfLogicBeginBrick() + ifBrick.addBrickToIfBranch(SetXBrick()) + ifBrick.addBrickToElseBranch(SetXBrick()) + script.addBrick(ifBrick) + + script.addBrick(SetXBrick()) + + XstreamSerializer.getInstance() + .saveProject(ProjectManager.getInstance().currentProject) + initialProject = getProjectAsXmlString() + } +} diff --git a/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/UndoTest.java b/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/UndoTest.java deleted file mode 100644 index 72b2412e88d..00000000000 --- a/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/UndoTest.java +++ /dev/null @@ -1,188 +0,0 @@ -/* - * Catroid: An on-device visual programming system for Android devices - * Copyright (C) 2010-2025 The Catrobat Team - * () - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * An additional term exception under section 7 of the GNU Affero - * General Public License, version 3, is available at - * http://developer.catrobat.org/license_additional_term - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -package org.catrobat.catroid.uiespresso.ui.fragment; - -import org.catrobat.catroid.ProjectManager; -import org.catrobat.catroid.R; -import org.catrobat.catroid.content.Script; -import org.catrobat.catroid.content.bricks.IfLogicBeginBrick; -import org.catrobat.catroid.content.bricks.SetXBrick; -import org.catrobat.catroid.io.XstreamSerializer; -import org.catrobat.catroid.test.utils.TestUtils; -import org.catrobat.catroid.ui.ProjectActivity; -import org.catrobat.catroid.uiespresso.util.UiTestUtils; -import org.catrobat.catroid.uiespresso.util.rules.FragmentActivityTestRule; -import org.junit.After; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; - -import java.io.IOException; -import java.util.Arrays; -import java.util.Collection; - -import static org.catrobat.catroid.WaitForConditionAction.waitFor; -import static org.catrobat.catroid.uiespresso.content.brick.utils.BrickDataInteractionWrapper.onBrickAtPosition; -import static org.junit.Assert.assertEquals; - -import static androidx.test.espresso.Espresso.onView; -import static androidx.test.espresso.Espresso.pressBack; -import static androidx.test.espresso.action.ViewActions.click; -import static androidx.test.espresso.assertion.ViewAssertions.doesNotExist; -import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed; -import static androidx.test.espresso.matcher.ViewMatchers.withId; -import static androidx.test.espresso.matcher.ViewMatchers.withText; - -@RunWith(Parameterized.class) -public class UndoTest { - - private final long waitThreshold = 5000; - - @Rule - public FragmentActivityTestRule baseActivityTestRule = new - FragmentActivityTestRule<>(ProjectActivity.class, ProjectActivity.EXTRA_FRAGMENT_POSITION, ProjectActivity.FRAGMENT_SPRITES); - - @Parameterized.Parameters(name = "{0}") - public static Collection data() { - return Arrays.asList(new Object[][] { - {"SingleScript", 0, R.string.brick_when_started}, - {"CompositeBrick", 1, R.string.brick_if_begin}, - {"SingleBrick", 2, R.string.brick_set_x}, - }); - } - - @Parameterized.Parameter - public String name; - - @Parameterized.Parameter(1) - public int brickPosition; - - @Parameterized.Parameter(2) - public int brickText; - - String initialProject; - - @After - public void tearDown() throws IOException { - TestUtils.deleteProjects(UndoTest.class.getSimpleName()); - } - - @Before - public void setUp() throws Exception { - createProject(); - baseActivityTestRule.launchActivity(); - onView(withText("testSprite")) - .perform(click()); - } - - @Test - public void testUndoSpinnerActionVisible() { - onBrickAtPosition(brickPosition) - .performDeleteBrick(); - - onView(withId(R.id.menu_undo)) - .perform(waitFor(isDisplayed(), waitThreshold)); - } - - @Test - public void testUndo() { - onBrickAtPosition(brickPosition).performDeleteBrick(); - - onView(withId(R.id.menu_undo)) - .perform(click()); - - onView(withId(R.id.menu_undo)) - .check(doesNotExist()); - - String projectAfterUndo = getProjectAsXmlString(); - assertEquals(projectAfterUndo, initialProject); - } - - @Test - public void checkScriptAfterUndo() { - onBrickAtPosition(brickPosition).performDeleteBrick(); - - onView(withId(R.id.menu_undo)) - .perform(click()); - - pressBack(); - - onView(withText("testSprite")) - .perform(click()); - - onBrickAtPosition(brickPosition).checkShowsText(brickText); - } - - @Test - public void testImmediateUndoRefresh() { - onBrickAtPosition(brickPosition).performDeleteBrick(); - - onView(withId(R.id.menu_undo)) - .perform(waitFor(isDisplayed(), waitThreshold)); - - onView(withId(R.id.menu_undo)) - .perform(click()); - - onView(withId(R.id.menu_undo)) - .check(doesNotExist()); - - onBrickAtPosition(brickPosition).checkShowsText(brickText); - } - - @Test - public void testUndoButtonHiddenAfterScriptViewUndo() { - onBrickAtPosition(brickPosition).performDeleteBrick(); - - onView(withId(R.id.menu_undo)) - .perform(waitFor(isDisplayed(), waitThreshold)); - - onView(withId(R.id.menu_undo)) - .perform(click()); - - onView(withId(R.id.menu_undo)) - .check(doesNotExist()); - - onBrickAtPosition(brickPosition).checkShowsText(brickText); - - onView(withId(R.id.menu_undo)) - .check(doesNotExist()); - } - - public String getProjectAsXmlString() { - return XstreamSerializer.getInstance().getXmlAsStringFromProject(ProjectManager.getInstance().getCurrentProject()); - } - - private void createProject() { - Script script = UiTestUtils.createProjectAndGetStartScript(UndoTest.class.getSimpleName()); - IfLogicBeginBrick compositeBrick = new IfLogicBeginBrick(); - compositeBrick.addBrickToIfBranch(new SetXBrick()); - compositeBrick.addBrickToElseBranch(new SetXBrick()); - script.addBrick(compositeBrick); - - XstreamSerializer.getInstance().saveProject(ProjectManager.getInstance().getCurrentProject()); - initialProject = getProjectAsXmlString(); - } -} diff --git a/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/UndoTest.kt b/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/UndoTest.kt new file mode 100644 index 00000000000..9e00626f951 --- /dev/null +++ b/catroid/src/androidTest/java/org/catrobat/catroid/uiespresso/ui/fragment/UndoTest.kt @@ -0,0 +1,209 @@ +/* + * Catroid: An on-device visual programming system for Android devices + * Copyright (C) 2010-2026 The Catrobat Team + * () + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * An additional term exception under section 7 of the GNU Affero + * General Public License, version 3, is available at + * http://developer.catrobat.org/license_additional_term + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package org.catrobat.catroid.uiespresso.ui.fragment + +import androidx.test.espresso.Espresso.onView +import androidx.test.espresso.Espresso.pressBack +import androidx.test.espresso.action.ViewActions.click +import androidx.test.espresso.assertion.ViewAssertions.matches +import androidx.test.espresso.matcher.ViewMatchers.isDisplayed +import androidx.test.espresso.matcher.ViewMatchers.isEnabled +import androidx.test.espresso.matcher.ViewMatchers.withId +import androidx.test.espresso.matcher.ViewMatchers.withText +import org.catrobat.catroid.ProjectManager +import org.catrobat.catroid.R +import org.catrobat.catroid.WaitForConditionAction +import org.catrobat.catroid.content.bricks.IfLogicBeginBrick +import org.catrobat.catroid.content.bricks.SetXBrick +import org.catrobat.catroid.io.XstreamSerializer +import org.catrobat.catroid.test.utils.TestUtils +import org.catrobat.catroid.ui.ProjectActivity +import org.catrobat.catroid.uiespresso.content.brick.utils.BrickDataInteractionWrapper.onBrickAtPosition +import org.catrobat.catroid.uiespresso.util.UiTestUtils +import org.catrobat.catroid.uiespresso.util.rules.FragmentActivityTestRule +import org.hamcrest.Matchers.not +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import java.util.Arrays + +@RunWith(Parameterized::class) +class UndoTest { + + private val waitThreshold = 5000L + + @get:Rule + val baseActivityTestRule = FragmentActivityTestRule( + ProjectActivity::class.java, + ProjectActivity.EXTRA_FRAGMENT_POSITION, + ProjectActivity.FRAGMENT_SPRITES + ) + + @Parameterized.Parameter + @JvmField + var name: String = "" + + @Parameterized.Parameter(1) + @JvmField + var brickPosition: Int = 0 + + @Parameterized.Parameter(2) + @JvmField + var brickText: Int = 0 + + private lateinit var initialProject: String + + @After + fun tearDown() { + TestUtils.deleteProjects(UndoTest::class.java.simpleName) + } + + @Before + fun setUp() { + createProject() + baseActivityTestRule.launchActivity() + onView(withText("testSprite")) + .perform(click()) + } + + @Test + fun testUndoSpinnerActionVisible() { + onBrickAtPosition(brickPosition) + .performDeleteBrick() + + onView(withId(R.id.menu_undo)) + .perform(WaitForConditionAction.waitFor(isDisplayed(), waitThreshold)) + } + + @Test + fun testUndo() { + onBrickAtPosition(brickPosition).performDeleteBrick() + + onView(withId(R.id.menu_undo)) + .perform(click()) + + onView(withId(R.id.menu_redo)) + .perform(WaitForConditionAction.waitFor(isEnabled(), waitThreshold)) + + val projectAfterUndo = getProjectAsXmlString() + assertEquals(initialProject, projectAfterUndo) + } + + @Test + fun checkScriptAfterUndo() { + onBrickAtPosition(brickPosition).performDeleteBrick() + + onView(withId(R.id.menu_undo)) + .perform(click()) + + onView(withId(R.id.menu_redo)) + .perform(WaitForConditionAction.waitFor(isEnabled(), waitThreshold)) + + pressBack() + + onView(withText("testSprite")) + .perform(click()) + + onBrickAtPosition(brickPosition).checkShowsText(brickText) + } + + fun getProjectAsXmlString(): String { + return XstreamSerializer.getInstance() + .getXmlAsStringFromProject(ProjectManager.getInstance().currentProject) + } + + private fun createProject() { + val script = UiTestUtils.createProjectAndGetStartScript(UndoTest::class.java.simpleName) + val compositeBrick = IfLogicBeginBrick() + compositeBrick.addBrickToIfBranch(SetXBrick()) + compositeBrick.addBrickToElseBranch(SetXBrick()) + script.addBrick(compositeBrick) + + XstreamSerializer.getInstance() + .saveProject(ProjectManager.getInstance().currentProject) + initialProject = getProjectAsXmlString() + } + + @Test + fun testMultiStepUndo() { + // 1. Delete first brick + onBrickAtPosition(brickPosition).performDeleteBrick() + onView(withId(R.id.menu_undo)).check(matches(isEnabled())) + + // 2. Delete second brick + onBrickAtPosition(brickPosition).performDeleteBrick() + + // 3. Undo first time + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_undo)).perform(WaitForConditionAction.waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_undo)).check(matches(isEnabled())) + onView(withId(R.id.menu_redo)).check(matches(isEnabled())) + + // 4. Undo second time + onView(withId(R.id.menu_undo)).perform(click()) + onView(withId(R.id.menu_redo)).perform(WaitForConditionAction.waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_undo)).check(matches(not(isEnabled()))) + onView(withId(R.id.menu_redo)).check(matches(isEnabled())) + + // 5. Redo first time + onView(withId(R.id.menu_redo)).perform(click()) + onView(withId(R.id.menu_undo)).perform(WaitForConditionAction.waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_undo)).check(matches(isEnabled())) + + // 6. Redo second time + onView(withId(R.id.menu_redo)).perform(click()) + onView(withId(R.id.menu_undo)).perform(WaitForConditionAction.waitFor(isEnabled(), waitThreshold)) + onView(withId(R.id.menu_redo)).check(matches(not(isEnabled()))) + } + + @Test + fun testConcurrentUndoRedo() { + onBrickAtPosition(brickPosition).performDeleteBrick() + + // Attempt double click to simulate rapid interaction + onView(withId(R.id.menu_undo)).perform(click(), click()) + + // Wait for async undo processing to complete before checking final state + onView(withId(R.id.menu_redo)).perform(WaitForConditionAction.waitFor(isEnabled(), waitThreshold)) + + // Verify that it still works and didn't crash + onView(withId(R.id.menu_redo)).check(matches(isEnabled())) + } + + companion object { + @JvmStatic + @Parameterized.Parameters(name = "{0}") + fun data(): Collection> { + return Arrays.asList( + arrayOf("SingleScript", 0, R.string.brick_when_started), + arrayOf("CompositeBrick", 1, R.string.brick_if_begin), + arrayOf("SingleBrick", 2, R.string.brick_set_x) + ) + } + } +} diff --git a/catroid/src/main/java/org/catrobat/catroid/common/Constants.java b/catroid/src/main/java/org/catrobat/catroid/common/Constants.java index 55788052a19..9869f6e5918 100644 --- a/catroid/src/main/java/org/catrobat/catroid/common/Constants.java +++ b/catroid/src/main/java/org/catrobat/catroid/common/Constants.java @@ -57,7 +57,7 @@ public final class Constants { public static final String PERMISSIONS_FILE_NAME = "permissions.txt"; public static final String TMP_CODE_XML_FILE_NAME = "tmp_" + CODE_XML_FILE_NAME; public static final String UNDO_CODE_XML_FILE_NAME = "undo_" + CODE_XML_FILE_NAME; - + public static final String UNDO_DIRECTORY_NAME = "undo_history"; public static final String DEVICE_VARIABLE_JSON_FILE_NAME = "DeviceVariables.json"; public static final String DEVICE_LIST_JSON_FILE_NAME = "DeviceLists.json"; diff --git a/catroid/src/main/java/org/catrobat/catroid/ui/SpriteActivity.java b/catroid/src/main/java/org/catrobat/catroid/ui/SpriteActivity.java index f499330634c..0543dc52aa0 100644 --- a/catroid/src/main/java/org/catrobat/catroid/ui/SpriteActivity.java +++ b/catroid/src/main/java/org/catrobat/catroid/ui/SpriteActivity.java @@ -1,6 +1,6 @@ /* * Catroid: An on-device visual programming system for Android devices - * Copyright (C) 2010-2025 The Catrobat Team + * Copyright (C) 2010-2026 The Catrobat Team * () * * This program is free software: you can redistribute it and/or modify @@ -68,6 +68,7 @@ import org.catrobat.catroid.ui.recyclerview.fragment.DataListFragment; import org.catrobat.catroid.ui.recyclerview.fragment.ListSelectorFragment; import org.catrobat.catroid.ui.recyclerview.fragment.LookListFragment; +import org.catrobat.catroid.ui.recyclerview.fragment.ProjectUndoManager; import org.catrobat.catroid.ui.recyclerview.fragment.ScriptFragment; import org.catrobat.catroid.ui.recyclerview.fragment.SoundListFragment; import org.catrobat.catroid.ui.recyclerview.util.UniqueNameProvider; @@ -108,9 +109,20 @@ import static org.catrobat.catroid.visualplacement.VisualPlacementActivity.Y_COORDINATE_BUNDLE_ARGUMENT; public class SpriteActivity extends BaseActivity { + private ProjectUndoManager undoManager; public static final String TAG = SpriteActivity.class.getSimpleName(); + public ProjectUndoManager getUndoManager() { + if (undoManager == null) { + Project project = ProjectManager.getInstance().getCurrentProject(); + if (project != null) { + undoManager = new ProjectUndoManager(project.getDirectory()); + } + } + return undoManager; + } + public static final int FRAGMENT_SCRIPTS = 0; public static final int FRAGMENT_LOOKS = 1; public static final int FRAGMENT_SOUNDS = 2; @@ -203,7 +215,8 @@ public void onCreate(Bundle savedInstanceState) { } public String createActionBarTitle() { - if (currentProject != null && currentProject.getSceneList() != null && currentProject.getSceneList().size() == 1) { + if (currentProject != null && currentProject.getSceneList() != null + && currentProject.getSceneList().size() == 1) { return currentSprite.getName(); } else { return currentScene.getName() + ": " + currentSprite.getName(); @@ -221,18 +234,28 @@ public boolean onCreateOptionsMenu(Menu menu) { return super.onCreateOptionsMenu(menu); } - public void showUndo(boolean visible) { + public void showUndo(boolean enabled) { if (optionsMenu != null) { - optionsMenu.findItem(R.id.menu_undo).setVisible(visible); - if (visible) { + MenuItem undoItem = optionsMenu.findItem(R.id.menu_undo); + undoItem.setEnabled(enabled); + undoItem.setIcon(enabled ? R.drawable.icon_undo : R.drawable.icon_undo_disabled); + if (enabled) { ProjectManager.getInstance().changedProject(currentProject.getName()); } } } + public void showRedo(boolean enabled) { + if (optionsMenu != null) { + MenuItem redoItem = optionsMenu.findItem(R.id.menu_redo); + redoItem.setEnabled(enabled); + redoItem.setIcon(enabled ? R.drawable.icon_redo : R.drawable.icon_redo_disabled); + } + } + public void checkForChange() { if (optionsMenu != null) { - if (optionsMenu.findItem(R.id.menu_undo).isVisible()) { + if (optionsMenu.findItem(R.id.menu_undo).isEnabled()) { ProjectManager.getInstance().changedProject(currentProject.getName()); } else { ProjectManager.getInstance().resetChangedFlag(currentProject); @@ -248,32 +271,53 @@ public void setUndoMenuItemVisibility(boolean isVisible) { public boolean onPrepareOptionsMenu(Menu menu) { if (getCurrentFragment() instanceof ScriptFragment) { menu.findItem(R.id.comment_in_out).setVisible(true); - showUndo(isUndoMenuItemVisible); + menu.findItem(R.id.menu_undo).setVisible(true); + menu.findItem(R.id.menu_redo).setVisible(true); + boolean canUndo = getUndoManager() != null && getUndoManager().canUndo(); + boolean canRedo = getUndoManager() != null && getUndoManager().canRedo(); + showUndo(isUndoMenuItemVisible || canUndo); + showRedo(canRedo); } else if (getCurrentFragment() instanceof LookListFragment) { + menu.findItem(R.id.comment_in_out).setVisible(false); + menu.findItem(R.id.menu_undo).setVisible(isUndoMenuItemVisible); + menu.findItem(R.id.menu_redo).setVisible(false); showUndo(isUndoMenuItemVisible); + } else { + menu.findItem(R.id.comment_in_out).setVisible(false); + menu.findItem(R.id.menu_undo).setVisible(false); + menu.findItem(R.id.menu_redo).setVisible(false); } return super.onPrepareOptionsMenu(menu); } @Override public boolean onOptionsItemSelected(MenuItem item) { - boolean isDragAndDropActiveInFragment = getCurrentFragment() instanceof ScriptFragment - && ((ScriptFragment) getCurrentFragment()).isCurrentlyMoving(); + boolean isDragAndDropActiveInFragment = getCurrentFragment() instanceof ScriptFragment scriptFragment + && scriptFragment.isCurrentlyMoving(); if (item.getItemId() == android.R.id.home && isDragAndDropActiveInFragment) { ((ScriptFragment) getCurrentFragment()).highlightMovingItem(); return true; } - if (item.getItemId() == R.id.menu_undo && getCurrentFragment() instanceof LookListFragment) { - setUndoMenuItemVisibility(false); - showUndo(isUndoMenuItemVisible); - Fragment fragment = getCurrentFragment(); - if (fragment instanceof LookListFragment && !((LookListFragment) fragment).undo() && currentLookData != null) { - ((LookListFragment) fragment).deleteItem(currentLookData); - currentLookData.dispose(); - currentLookData = null; + if (item.getItemId() == R.id.menu_undo) { + if (getCurrentFragment() instanceof ScriptFragment scriptFragment) { + scriptFragment.loadProjectAfterUndoOption(); + return true; + } else if (getCurrentFragment() instanceof LookListFragment lookListFragment) { + setUndoMenuItemVisibility(false); + showUndo(isUndoMenuItemVisible); + if (!lookListFragment.undo() && currentLookData != null) { + lookListFragment.deleteItem(currentLookData); + currentLookData.dispose(); + currentLookData = null; + } + return true; } + } + + if (item.getItemId() == R.id.menu_redo && getCurrentFragment() instanceof ScriptFragment scriptFragment) { + scriptFragment.loadProjectAfterRedoOption(); return true; } @@ -293,6 +337,12 @@ protected void onSaveInstanceState(Bundle outState) { outState.putBoolean(BUNDLE_IS_UNDO_MENU_ITEM_VISIBLE, isUndoMenuItemVisible); } + @Override + protected void onDestroy() { + super.onDestroy(); + undoManager = null; + } + @Override public void onBackPressed() { saveProject(); @@ -833,9 +883,11 @@ public void handleAddUserDataButton() { DuplicateInputTextWatcher textWatcher = new DuplicateInputTextWatcher(variables); TextInputDialog.Builder builder = new TextInputDialog.Builder(this); - UniqueNameProvider uniqueVariableNameProvider = builder.createUniqueNameProvider(R.string.default_variable_name); + UniqueNameProvider uniqueVariableNameProvider = builder + .createUniqueNameProvider(R.string.default_variable_name); UniqueNameProvider uniqueListNameProvider = builder.createUniqueNameProvider(R.string.default_list_name); - generatedVariableName = uniqueVariableNameProvider.getUniqueName(getString(R.string.default_variable_name), null); + generatedVariableName = uniqueVariableNameProvider.getUniqueName(getString(R.string.default_variable_name), + null); builder.setTextWatcher(textWatcher) .setText(generatedVariableName) .setPositiveButton(getString(R.string.ok), (TextInputDialog.OnClickListener) (dialog, textInput) -> { @@ -889,9 +941,9 @@ public void handleAddUserDataButton() { alertDialog.setTitle(getString(R.string.formula_editor_variable_dialog_title)); textWatcher.setOriginalScope(variables); if (currentName.equals(generatedVariableName)) { - generatedVariableName = - uniqueVariableNameProvider.getUniqueName(getString(R.string.default_variable_name), - null); + generatedVariableName = uniqueVariableNameProvider.getUniqueName( + getString(R.string.default_variable_name), + null); textInputEditText.setText(generatedVariableName); } } @@ -966,7 +1018,8 @@ public ActionMode startActionMode(ActionMode.Callback callback) { @Override public void onActionModeFinished(ActionMode mode) { Fragment fragment = getCurrentFragment(); - if (isFragmentWithTablayout(fragment) && (!(fragment instanceof ScriptFragment) || !((ScriptFragment) fragment).isFinderOpen())) { + if (isFragmentWithTablayout(fragment) + && (!(fragment instanceof ScriptFragment scriptFragment) || !scriptFragment.isFinderOpen())) { addTabLayout(this, getTabPositionInSpriteActivity(fragment)); } super.onActionModeFinished(mode); diff --git a/catroid/src/main/java/org/catrobat/catroid/ui/dragndrop/BrickListView.kt b/catroid/src/main/java/org/catrobat/catroid/ui/dragndrop/BrickListView.kt index 99e9e857e8f..9d761df74f3 100644 --- a/catroid/src/main/java/org/catrobat/catroid/ui/dragndrop/BrickListView.kt +++ b/catroid/src/main/java/org/catrobat/catroid/ui/dragndrop/BrickListView.kt @@ -1,6 +1,6 @@ /* * Catroid: An on-device visual programming system for Android devices - * Copyright (C) 2010-2025 The Catrobat Team + * Copyright (C) 2010-2026 The Catrobat Team * () * * This program is free software: you can redistribute it and/or modify @@ -53,6 +53,11 @@ private const val LOWER_SCROLL_BOUND_MULTIPLIER = 0.85 private const val Y_TRANSLATION_CONSTANT = 10 class BrickListView : ListView { + + fun interface OnMoveCommittedListener { + fun onMoveCommitted() + } + private var upperScrollBound = 0 private var lowerScrollBound = 0 private var hoveringDrawable: BitmapDrawable? = null @@ -67,6 +72,11 @@ class BrickListView : ListView { private val translucentBlack = Color.argb(TRANSLUCENT_BLACK_ALPHA, 0, 0, 0) private var scrollRunnable: Runnable? = null private var isScrollRunnableRunning = false + private var onMoveCommittedListener: OnMoveCommittedListener? = null + + fun setOnMoveCommittedListener(listener: OnMoveCommittedListener?) { + onMoveCommittedListener = listener + } constructor(context: Context?) : super(context) constructor(context: Context?, attributes: AttributeSet?) : super(context, attributes) @@ -146,6 +156,7 @@ class BrickListView : ListView { fun stopMoving() { removeCallbacks(scrollRunnable) isScrollRunnableRunning = false + onMoveCommittedListener?.onMoveCommitted() brickAdapterInterface?.moveItemTo(currentPositionOfHoveringBrick, brickToMove) cancelMove() } diff --git a/catroid/src/main/java/org/catrobat/catroid/ui/fragment/ProjectOptionsFragment.kt b/catroid/src/main/java/org/catrobat/catroid/ui/fragment/ProjectOptionsFragment.kt index f8e62a799ee..4b0326eb29e 100644 --- a/catroid/src/main/java/org/catrobat/catroid/ui/fragment/ProjectOptionsFragment.kt +++ b/catroid/src/main/java/org/catrobat/catroid/ui/fragment/ProjectOptionsFragment.kt @@ -63,6 +63,7 @@ import org.catrobat.catroid.ui.runtimepermissions.RequiresPermissionTask import org.catrobat.catroid.utils.ToastUtil import org.catrobat.catroid.utils.notifications.StatusBarNotificationManager import org.koin.android.ext.android.inject +import org.catrobat.catroid.ui.recyclerview.fragment.ProjectUndoManager import java.io.File import java.io.IOException @@ -239,6 +240,7 @@ class ProjectOptionsFragment : Fragment() { if (project!!.name != name) { XstreamSerializer.getInstance().saveProject(project) + ProjectUndoManager.clearUndoHistoryForProject(project!!.directory) val renamedDirectory = renameProject(project!!.directory, name) if (renamedDirectory == null) { Log.e(TAG, "Creating renamed directory failed!") @@ -357,6 +359,7 @@ class ProjectOptionsFragment : Fragment() { private fun deleteProject(selectedProject: ProjectData) { try { + ProjectUndoManager.clearUndoHistoryForProject(selectedProject.directory) StorageOperations.deleteDir(selectedProject.directory) } catch (exception: IOException) { Log.e(TAG, Log.getStackTraceString(exception)) diff --git a/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectListFragment.kt b/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectListFragment.kt index bfb5e9fae3a..9c4d932ed90 100644 --- a/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectListFragment.kt +++ b/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectListFragment.kt @@ -379,6 +379,7 @@ class ProjectListFragment( item ?: continue try { projectManager.deleteDownloadedProjectInformation(item.name) + ProjectUndoManager.clearUndoHistoryForProject(item.directory) StorageOperations.deleteDir(item.directory) items.remove(item) } catch (e: IOException) { @@ -422,6 +423,7 @@ class ProjectListFragment( name ?: return if (name != item.name) { setShowProgressBar(true) + ProjectUndoManager.clearUndoHistoryForProject(item.directory) ProjectRenamer(item.directory, name) .renameProjectAsync({ success: Boolean -> onRenameFinished(success) }) } diff --git a/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectUndoManager.kt b/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectUndoManager.kt new file mode 100644 index 00000000000..a52305ff9cc --- /dev/null +++ b/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectUndoManager.kt @@ -0,0 +1,294 @@ +/* + * Catroid: An on-device visual programming system for Android devices + * Copyright (C) 2010-2026 The Catrobat Team + * () + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * An additional term exception under section 7 of the GNU Affero + * General Public License, version 3, is available at + * http://developer.catrobat.org/license_additional_term + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package org.catrobat.catroid.ui.recyclerview.fragment + +import android.util.Log +import org.catrobat.catroid.common.Constants +import org.catrobat.catroid.formulaeditor.UserList +import org.catrobat.catroid.formulaeditor.UserVariable +import org.catrobat.catroid.io.StorageOperations +import java.io.File +import java.io.IOException + +class ProjectUndoManager(private val projectDir: File) { + + private val undoDir: File = File(projectDir, Constants.UNDO_DIRECTORY_NAME) + private val undoStack = mutableListOf() + private val redoStack = mutableListOf() + + data class VariableSnapshot( + @JvmField val savedUserVariables: List, + @JvmField val savedMultiplayerVariables: List, + @JvmField val savedUserLists: List, + @JvmField val savedLocalUserVariables: List, + @JvmField val savedLocalLists: List + ) + + data class UndoEntry( + @JvmField val snapshotFileName: String, + @JvmField val sceneName: String, + @JvmField val spriteName: String, + @JvmField val variableSnapshot: VariableSnapshot + ) + + init { + if (undoDir.exists()) { + val currentTime = System.currentTimeMillis() + undoDir.listFiles()?.forEach { file -> + if (currentTime - file.lastModified() > UNDO_SNAPSHOT_TTL_MS && file.exists() && !file.delete()) { + Log.w(TAG, "Failed to delete stale snapshot on init: ${file.absolutePath}") + } + } + } else if (!undoDir.mkdirs()) { + Log.e(TAG, "Failed to create undo history directory: ${undoDir.absolutePath}") + } + } + + fun pushState( + sceneName: String, + spriteName: String, + variableSnapshot: VariableSnapshot + ) { + val currentCodeFile = File(projectDir, Constants.CODE_XML_FILE_NAME) + if (!currentCodeFile.exists()) { + return + } + + var snapshotFile: File? = null + try { + snapshotFile = File.createTempFile("snap_", ".xml", undoDir) + val snapshotName = snapshotFile.name + + StorageOperations.copyFile(currentCodeFile, snapshotFile) + val variables = VariableSnapshot( + ArrayList(variableSnapshot.savedUserVariables), + ArrayList(variableSnapshot.savedMultiplayerVariables), + ArrayList(variableSnapshot.savedUserLists), + ArrayList(variableSnapshot.savedLocalUserVariables), + ArrayList(variableSnapshot.savedLocalLists) + ) + undoStack.add(UndoEntry(snapshotName, sceneName, spriteName, variables)) + for (redoEntry in redoStack) { + val redoFile = File(undoDir, redoEntry.snapshotFileName) + if (redoFile.exists() && !redoFile.delete()) { + Log.w(TAG, "Failed to delete redo snapshot: ${redoFile.absolutePath}") + } + } + redoStack.clear() + + if (undoStack.size > MAX_UNDO_STEPS) { + val oldest = undoStack.removeAt(0) + val oldestFile = File(undoDir, oldest.snapshotFileName) + if (oldestFile.exists() && !oldestFile.delete()) { + Log.w(TAG, "Failed to delete oldest snapshot: ${oldestFile.absolutePath}") + } + } + } catch (e: IOException) { + if (snapshotFile != null && snapshotFile.exists() && !snapshotFile.delete()) { + Log.w(TAG, "Failed to delete orphaned snapshot: ${snapshotFile.absolutePath}") + } + Log.e(TAG, "Failed to push undo state", e) + } + } + + fun popUndo( + sceneName: String, + spriteName: String, + variableSnapshot: VariableSnapshot + ): UndoEntry? { + if (undoStack.isEmpty()) { + return null + } + + val redoEntry = pushCurrentToRedo(sceneName, spriteName, variableSnapshot) ?: return null + + val entry = undoStack.removeAt(undoStack.size - 1) + return if (restoreSnapshot(entry)) { + val snapshotFile = File(undoDir, entry.snapshotFileName) + if (snapshotFile.exists() && !snapshotFile.delete()) { + Log.w(TAG, "Failed to delete undo snapshot file: ${snapshotFile.absolutePath}") + } + entry + } else { + undoStack.add(entry) + redoStack.removeAt(redoStack.size - 1) + val redoFile = File(undoDir, redoEntry.snapshotFileName) + if (redoFile.exists() && !redoFile.delete()) { + Log.w(TAG, "Failed to delete redundant redo snapshot: ${redoFile.absolutePath}") + } + null + } + } + + fun popRedo( + sceneName: String, + spriteName: String, + variableSnapshot: VariableSnapshot + ): UndoEntry? { + if (redoStack.isEmpty()) { + return null + } + + val undoEntry = pushCurrentToUndoInternal(sceneName, spriteName, variableSnapshot) ?: return null + + val entry = redoStack.removeAt(redoStack.size - 1) + return if (restoreSnapshot(entry)) { + val snapshotFile = File(undoDir, entry.snapshotFileName) + if (snapshotFile.exists() && !snapshotFile.delete()) { + Log.w(TAG, "Failed to delete redo snapshot file: ${snapshotFile.absolutePath}") + } + entry + } else { + redoStack.add(entry) + undoStack.removeAt(undoStack.size - 1) + val undoFile = File(undoDir, undoEntry.snapshotFileName) + if (undoFile.exists() && !undoFile.delete()) { + Log.w(TAG, "Failed to delete redundant undo snapshot: ${undoFile.absolutePath}") + } + null + } + } + + private fun pushCurrentToRedo( + sceneName: String, + spriteName: String, + variableSnapshot: VariableSnapshot + ): UndoEntry? { + val currentCodeFile = File(projectDir, Constants.CODE_XML_FILE_NAME) + var snapshotFile: File? = null + return try { + snapshotFile = File.createTempFile("redo_", ".xml", undoDir) + val snapshotName = snapshotFile.name + + StorageOperations.copyFile(currentCodeFile, snapshotFile) + if (redoStack.size >= MAX_UNDO_STEPS) { + val oldest = redoStack.removeAt(0) + val oldestFile = File(undoDir, oldest.snapshotFileName) + if (oldestFile.exists() && !oldestFile.delete()) { + Log.w(TAG, "Failed to delete oldest redo snapshot: ${oldestFile.absolutePath}") + } + } + val variables = VariableSnapshot( + ArrayList(variableSnapshot.savedUserVariables), + ArrayList(variableSnapshot.savedMultiplayerVariables), + ArrayList(variableSnapshot.savedUserLists), + ArrayList(variableSnapshot.savedLocalUserVariables), + ArrayList(variableSnapshot.savedLocalLists) + ) + val entry = UndoEntry(snapshotName, sceneName, spriteName, variables) + redoStack.add(entry) + entry + } catch (e: IOException) { + if (snapshotFile != null && snapshotFile.exists() && !snapshotFile.delete()) { + Log.w(TAG, "Failed to delete orphaned redo snapshot: ${snapshotFile.absolutePath}") + } + Log.e(TAG, "Failed to push redo state", e) + null + } + } + + private fun pushCurrentToUndoInternal( + sceneName: String, + spriteName: String, + variableSnapshot: VariableSnapshot + ): UndoEntry? { + val currentCodeFile = File(projectDir, Constants.CODE_XML_FILE_NAME) + var snapshotFile: File? = null + return try { + snapshotFile = File.createTempFile("snap_", ".xml", undoDir) + val snapshotName = snapshotFile.name + + StorageOperations.copyFile(currentCodeFile, snapshotFile) + if (undoStack.size >= MAX_UNDO_STEPS) { + val oldest = undoStack.removeAt(0) + val oldestFile = File(undoDir, oldest.snapshotFileName) + if (oldestFile.exists() && !oldestFile.delete()) { + Log.w(TAG, "Failed to delete oldest undo internal snapshot: ${oldestFile.absolutePath}") + } + } + val variables = VariableSnapshot( + ArrayList(variableSnapshot.savedUserVariables), + ArrayList(variableSnapshot.savedMultiplayerVariables), + ArrayList(variableSnapshot.savedUserLists), + ArrayList(variableSnapshot.savedLocalUserVariables), + ArrayList(variableSnapshot.savedLocalLists) + ) + val entry = UndoEntry(snapshotName, sceneName, spriteName, variables) + undoStack.add(entry) + entry + } catch (e: IOException) { + if (snapshotFile != null && snapshotFile.exists() && !snapshotFile.delete()) { + Log.w(TAG, "Failed to delete orphaned undo snapshot: ${snapshotFile.absolutePath}") + } + Log.e(TAG, "Failed to push undo internal state", e) + null + } + } + + private fun restoreSnapshot(entry: UndoEntry): Boolean { + val snapshotFile = File(undoDir, entry.snapshotFileName) + val currentCodeFile = File(projectDir, Constants.CODE_XML_FILE_NAME) + return try { + StorageOperations.copyFile(snapshotFile, currentCodeFile) + true + } catch (e: IOException) { + Log.e(TAG, "Failed to restore snapshot ${entry.snapshotFileName}", e) + false + } + } + + fun canUndo(): Boolean = undoStack.isNotEmpty() + + fun canRedo(): Boolean = redoStack.isNotEmpty() + + fun clearHistory() { + undoStack.clear() + redoStack.clear() + undoDir.listFiles()?.forEach { file -> + if (file.exists() && !file.delete()) { + Log.w(TAG, "Failed to delete snapshot file: ${file.absolutePath}") + } + } + } + + companion object { + private val TAG = ProjectUndoManager::class.java.simpleName + private const val MAX_UNDO_STEPS = 20 + private const val UNDO_SNAPSHOT_TTL_MS = 60 * 60 * 1000L // 1 hour + + @JvmStatic + fun clearUndoHistoryForProject(projectDir: File) { + val undoDir = File(projectDir, Constants.UNDO_DIRECTORY_NAME) + if (undoDir.exists()) { + undoDir.listFiles()?.forEach { file -> + if (file.exists() && !file.delete()) { + Log.w(TAG, "Failed to delete project snapshot file: ${file.absolutePath}") + } + } + if (undoDir.exists() && !undoDir.delete()) { + Log.w(TAG, "Failed to delete undo directory: ${undoDir.absolutePath}") + } + } + } + } +} diff --git a/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ScriptFragment.java b/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ScriptFragment.java index 059d7bdb59e..8f573ab8a13 100644 --- a/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ScriptFragment.java +++ b/catroid/src/main/java/org/catrobat/catroid/ui/recyclerview/fragment/ScriptFragment.java @@ -105,9 +105,8 @@ import androidx.fragment.app.ListFragment; import static org.catrobat.catroid.common.Constants.CODE_XML_FILE_NAME; -import static org.catrobat.catroid.common.Constants.UNDO_CODE_XML_FILE_NAME; -public class ScriptFragment extends ListFragment implements ActionMode.Callback, BrickAdapter.OnBrickClickListener, BrickAdapter.SelectionListener, OnCategorySelectedListener, AddBrickFragment.OnAddBrickListener, ProjectLoader.ProjectLoadListener { +public class ScriptFragment extends ListFragment implements ActionMode.Callback, BrickAdapter.OnBrickClickListener, BrickAdapter.SelectionListener, OnCategorySelectedListener, AddBrickFragment.OnAddBrickListener, ProjectLoader.ProjectLoadListener, BrickListView.OnMoveCommittedListener { public static final String TAG = ScriptFragment.class.getSimpleName(); private static final String BRICK_TAG = "brickToFocus"; @@ -143,6 +142,7 @@ public class ScriptFragment extends ListFragment implements ActionMode.Callback, private Script scriptToFocus; private SpriteActivity activity; + private boolean pendingMoveUndoSnapshot = false; public static ScriptFragment newInstance(Brick brickToFocus) { ScriptFragment scriptFragment = new ScriptFragment(); @@ -352,6 +352,9 @@ public void onActivityCreated(Bundle savedInstanceState) { Sprite currentSprite = ProjectManager.getInstance().getCurrentSprite(); currentProject.getBroadcastMessageContainer().update(); + currentSceneName = currentScene.getName(); + currentSpriteName = currentSprite.getName(); + adapter = new BrickAdapter(ProjectManager.getInstance().getCurrentSprite()); adapter.setSelectionListener(this); adapter.setOnItemClickListener(this); @@ -359,6 +362,7 @@ public void onActivityCreated(Bundle savedInstanceState) { listView.setAdapter(adapter); listView.setOnItemClickListener(adapter); listView.setOnItemLongClickListener(adapter); + listView.setOnMoveCommittedListener(this); if (currentSprite.equals(currentScene.getBackgroundSprite())) { InternToExternGenerator.setInternExternLanguageConverterMap(Sensors.OBJECT_NUMBER_OF_LOOKS, R.string.formula_editor_object_number_of_backgrounds); @@ -476,11 +480,20 @@ public void highlightMovingItem() { } public void cancelMove() { + pendingMoveUndoSnapshot = false; listView.cancelMove(); Sprite sprite = ProjectManager.getInstance().getCurrentSprite(); adapter.updateItems(sprite); } + @Override + public void onMoveCommitted() { + if (pendingMoveUndoSnapshot) { + copyProjectForUndoOption(); + pendingMoveUndoSnapshot = false; + } + } + public boolean isCurrentlyHighlighted() { return listView.isCurrentlyHighlighted(); } @@ -598,6 +611,7 @@ public void handleAddButton() { } public void addBrick(Brick brick) { + copyProjectForUndoOption(); try { if (!brick.getClass().equals(UserDefinedReceiverBrick.class) && !brick.getClass().equals(UserDefinedBrick.class)) { RecentBrickListManager.getInstance().addBrick(brick.clone()); @@ -714,7 +728,6 @@ public static List getContextMenuItems(Brick brick) { } private void handleContextMenuItemClick(int itemId, Brick brick, int position) { - showUndo(false); switch (itemId) { case R.string.backpack_add: List bricksToPack = new ArrayList<>(); @@ -723,6 +736,7 @@ private void handleContextMenuItemClick(int itemId, Brick brick, int position) { break; case R.string.brick_context_dialog_copy_brick: case R.string.brick_context_dialog_copy_script: + copyProjectForUndoOption(); try { Brick clonedBrick = brick.getAllParts().get(0).clone(); adapter.addItem(position, clonedBrick); @@ -741,6 +755,7 @@ private void handleContextMenuItemClick(int itemId, Brick brick, int position) { break; case R.string.brick_context_dialog_comment_in: case R.string.brick_context_dialog_comment_in_script: + copyProjectForUndoOption(); for (Brick brickPart : brick.getAllParts()) { brickPart.setCommentedOut(false); } @@ -748,6 +763,7 @@ private void handleContextMenuItemClick(int itemId, Brick brick, int position) { break; case R.string.brick_context_dialog_comment_out: case R.string.brick_context_dialog_comment_out_script: + copyProjectForUndoOption(); for (Brick brickPart : brick.getAllParts()) { brickPart.setCommentedOut(true); } @@ -791,10 +807,10 @@ private void openWebViewWithHelpPage(Brick brick) { @Override public boolean onBrickLongClick(Brick brick, int position) { - showUndo(false); if (listView.isCurrentlyHighlighted()) { listView.cancelHighlighting(); } else { + pendingMoveUndoSnapshot = true; listView.startMoving(brick); } return true; @@ -847,6 +863,7 @@ private void switchToBackpack() { } private void copy(List selectedBricks) { + copyProjectForUndoOption(); Sprite sprite = ProjectManager.getInstance().getCurrentSprite(); brickController.copy(selectedBricks, sprite); adapter.updateItems(sprite); @@ -869,6 +886,7 @@ private void delete(List selectedItems) { } private void toggleComments(List selectedBricks) { + copyProjectForUndoOption(); for (Brick brick : adapter.getItems()) { brick.setCommentedOut(selectedBricks.contains(brick)); } @@ -886,97 +904,111 @@ public boolean copyProjectForUndoOption() { currentSceneName = projectManager.getCurrentlyEditedScene().getName(); Project project = projectManager.getCurrentProject(); XstreamSerializer.getInstance().saveProject(project); - File currentCodeFile = new File(project.getDirectory(), CODE_XML_FILE_NAME); - File undoCodeFile = new File(project.getDirectory(), UNDO_CODE_XML_FILE_NAME); - - if (currentCodeFile.exists()) { - try { - StorageOperations.transferData(currentCodeFile, undoCodeFile); - saveVariables(); - return true; - } catch (IOException exception) { - Log.e(TAG, "Copying project " + project.getName() + " failed.", exception); - } + + SpriteActivity spriteActivity = (SpriteActivity) getActivity(); + if (spriteActivity != null && spriteActivity.getUndoManager() != null) { + saveVariables(); + ProjectUndoManager.VariableSnapshot snapshot = new ProjectUndoManager.VariableSnapshot( + savedUserVariables, savedMultiplayerVariables, savedUserLists, + savedLocalUserVariables, savedLocalLists); + spriteActivity.getUndoManager().pushState(currentSceneName, currentSpriteName, snapshot); + spriteActivity.setUndoMenuItemVisibility(false); + spriteActivity.invalidateOptionsMenu(); + return true; } return false; } - public void loadProjectAfterUndoOption() { - Project project = ProjectManager.getInstance().getCurrentProject(); - File currentCodeFile = new File(project.getDirectory(), CODE_XML_FILE_NAME); - File undoCodeFile = new File(project.getDirectory(), UNDO_CODE_XML_FILE_NAME); - Context context = getContext(); - - if (!isAdded() || context == null) { - Log.w(TAG, "Cannot load project after undo because fragment is not attached."); - return; - } + private boolean isUndoRedoInProgress = false; - if (!undoCodeFile.exists()) { - Log.e(TAG, "Undo code file " + UNDO_CODE_XML_FILE_NAME + " does not exist."); - ToastUtil.showError(context, R.string.error_load_project); + public void loadProjectAfterUndoOption() { + if (isUndoRedoInProgress) { return; } - - if (currentCodeFile.exists()) { - try { - StorageOperations.transferData(undoCodeFile, currentCodeFile); - SpriteActivity spriteActivity = (SpriteActivity) getActivity(); - if (spriteActivity != null) { - spriteActivity.setUndoMenuItemVisibility(false); - spriteActivity.showUndo(false); - } - new ProjectLoader(project.getDirectory(), context).setListener(this).loadProjectAsync(); - } catch (IOException exception) { - Log.e(TAG, "Replacing project " + project.getName() + " failed.", exception); - ToastUtil.showError(context, R.string.error_load_project); - SpriteActivity spriteActivity = (SpriteActivity) getActivity(); - if (spriteActivity != null && undoCodeFile.exists()) { - spriteActivity.setUndoMenuItemVisibility(true); - spriteActivity.showUndo(true); - } + SpriteActivity spriteActivity = (SpriteActivity) getActivity(); + if (spriteActivity != null && spriteActivity.getUndoManager() != null) { + Project project = ProjectManager.getInstance().getCurrentProject(); + XstreamSerializer.getInstance().saveProject(project); + saveVariables(); + ProjectUndoManager.VariableSnapshot snapshot = new ProjectUndoManager.VariableSnapshot( + savedUserVariables, savedMultiplayerVariables, savedUserLists, + savedLocalUserVariables, savedLocalLists); + ProjectUndoManager.UndoEntry entry = spriteActivity.getUndoManager().popUndo( + currentSceneName, currentSpriteName, snapshot); + if (entry != null) { + isUndoRedoInProgress = true; + spriteActivity.showUndo(false); + spriteActivity.showRedo(false); + listView.cancelMove(); + listView.cancelHighlighting(); + restoreFromEntry(entry); } } } - - @Override - public void onLoadFinished(boolean success) { - if (!isAdded() || getContext() == null) { + public void loadProjectAfterRedoOption() { + if (isUndoRedoInProgress) { return; } - SpriteActivity spriteActivity = (SpriteActivity) getActivity(); - if (!success) { - Log.e(TAG, "Loading project after undo failed."); - ToastUtil.showError(getContext(), R.string.error_load_project); - if (spriteActivity != null) { - spriteActivity.setUndoMenuItemVisibility(true); - spriteActivity.showUndo(true); + if (spriteActivity != null && spriteActivity.getUndoManager() != null) { + Project project = ProjectManager.getInstance().getCurrentProject(); + XstreamSerializer.getInstance().saveProject(project); + saveVariables(); + ProjectUndoManager.VariableSnapshot snapshot = new ProjectUndoManager.VariableSnapshot( + savedUserVariables, savedMultiplayerVariables, savedUserLists, + savedLocalUserVariables, savedLocalLists); + ProjectUndoManager.UndoEntry entry = spriteActivity.getUndoManager().popRedo( + currentSceneName, currentSpriteName, snapshot); + if (entry != null) { + isUndoRedoInProgress = true; + spriteActivity.showUndo(false); + spriteActivity.showRedo(false); + listView.cancelMove(); + listView.cancelHighlighting(); + restoreFromEntry(entry); } - return; - } - - if (!ProjectManager.getInstance().setCurrentSceneAndSprite(currentSceneName, currentSpriteName)) { - Log.e(TAG, "Could not set scene/sprite after undo: " + currentSceneName + "/" + currentSpriteName); } + } - loadVariables(); + private void restoreFromEntry(ProjectUndoManager.UndoEntry entry) { + Project project = ProjectManager.getInstance().getCurrentProject(); - if (spriteActivity != null) { - spriteActivity.setUndoMenuItemVisibility(false); - spriteActivity.showUndo(false); + if (entry.sceneName != null) { + this.currentSceneName = entry.sceneName; } - - File undoCodeFile = new File(ProjectManager.getInstance().getCurrentProject().getDirectory(), UNDO_CODE_XML_FILE_NAME); - if (undoCodeFile.exists() && !undoCodeFile.delete()) { - Log.w(TAG, "Could not delete undo code file: " + undoCodeFile.getAbsolutePath()); + if (entry.spriteName != null) { + this.currentSpriteName = entry.spriteName; } + this.savedUserVariables = entry.variableSnapshot.savedUserVariables; + this.savedMultiplayerVariables = entry.variableSnapshot.savedMultiplayerVariables; + this.savedUserLists = entry.variableSnapshot.savedUserLists; + this.savedLocalUserVariables = entry.variableSnapshot.savedLocalUserVariables; + this.savedLocalLists = entry.variableSnapshot.savedLocalLists; + new ProjectLoader(project.getDirectory(), getContext()).setListener(this).loadProjectAsync(); + } - if (getView() == null || listView == null) { + @Override + public void onLoadFinished(boolean success) { + isUndoRedoInProgress = false; + if (!success) { + ToastUtil.showError(getActivity(), R.string.error_load_project); + if (getActivity() instanceof SpriteActivity spriteActivity) { + spriteActivity.invalidateOptionsMenu(); + } return; } + ProjectManager.getInstance().setCurrentSceneAndSprite(currentSceneName, currentSpriteName); + if (adapter != null) { + adapter.updateItems(ProjectManager.getInstance().getCurrentSprite()); + } + if (checkVariables()) { + loadVariables(); + } refreshFragmentAfterUndo(); + if (getActivity() instanceof SpriteActivity spriteActivity) { + spriteActivity.invalidateOptionsMenu(); + } } private void saveVariables() { @@ -996,31 +1028,25 @@ public boolean checkVariables() { Sprite currentSprite = projectManager.getCurrentSprite(); Project project = projectManager.getCurrentProject(); - return (project != null && hasProjectVariablesChanged(project)) - || (currentSprite != null && hasSpriteVariablesChanged(currentSprite)); - } - - private boolean hasProjectVariablesChanged(Project project) { - boolean changed = false; - if (savedUserVariables != null && project.getUserVariables() != null) { - changed |= project.hasUserDataChanged(project.getUserVariables(), savedUserVariables); - } - if (savedMultiplayerVariables != null && project.getMultiplayerVariables() != null) { - changed |= project.hasUserDataChanged(project.getMultiplayerVariables(), savedMultiplayerVariables); - } - if (savedUserLists != null && project.getUserLists() != null) { - changed |= project.hasUserDataChanged(project.getUserLists(), savedUserLists); - } - return changed; - } - - private boolean hasSpriteVariablesChanged(Sprite currentSprite) { boolean changed = false; - if (savedLocalUserVariables != null && currentSprite.getUserVariables() != null) { - changed |= currentSprite.hasUserDataChanged(currentSprite.getUserVariables(), savedLocalUserVariables); + if (project != null) { + if (savedUserVariables != null) { + changed |= project.hasUserDataChanged(project.getUserVariables(), savedUserVariables); + } + if (savedMultiplayerVariables != null) { + changed |= project.hasUserDataChanged(project.getMultiplayerVariables(), savedMultiplayerVariables); + } + if (savedUserLists != null) { + changed |= project.hasUserDataChanged(project.getUserLists(), savedUserLists); + } } - if (savedLocalLists != null && currentSprite.getUserLists() != null) { - changed |= currentSprite.hasUserDataChanged(currentSprite.getUserLists(), savedLocalLists); + if (currentSprite != null) { + if (savedLocalUserVariables != null) { + changed |= currentSprite.hasUserDataChanged(currentSprite.getUserVariables(), savedLocalUserVariables); + } + if (savedLocalLists != null) { + changed |= currentSprite.hasUserDataChanged(currentSprite.getUserLists(), savedLocalLists); + } } return changed; } @@ -1055,8 +1081,12 @@ private void refreshFragmentAfterUndo() { if (!isAdded() || getActivity() == null || getParentFragmentManager().isStateSaved()) { return; } - Fragment scriptFragment = getParentFragmentManager().findFragmentByTag(TAG); - if (scriptFragment == null) { + Fragment scriptFragment = getParentFragmentManager().findFragmentById(R.id.fragment_container); + if (!(scriptFragment instanceof ScriptFragment)) { + scriptFragment = getParentFragmentManager().findFragmentByTag(TAG); + } + + if (!(scriptFragment instanceof ScriptFragment)) { return; } @@ -1068,9 +1098,10 @@ private void refreshFragmentAfterUndo() { final FragmentTransaction fragmentTransaction = getParentFragmentManager().beginTransaction(); fragmentTransaction.detach(scriptFragment); fragmentTransaction.attach(scriptFragment); - fragmentTransaction.commitNow(); + fragmentTransaction.commitNowAllowingStateLoss(); if (listView != null + && undoBrickPosition >= 0 && (undoBrickPosition < listView.getFirstVisiblePosition() || undoBrickPosition > listView.getLastVisiblePosition())) { listView.post(() -> listView.setSelection(undoBrickPosition)); diff --git a/catroid/src/main/res/menu/menu_script_activity.xml b/catroid/src/main/res/menu/menu_script_activity.xml index e9f6b2d551f..497ecf2aa62 100644 --- a/catroid/src/main/res/menu/menu_script_activity.xml +++ b/catroid/src/main/res/menu/menu_script_activity.xml @@ -29,7 +29,16 @@ android:id="@+id/menu_undo" android:title="@string/undo" app:showAsAction="ifRoom" - android:icon="@drawable/icon_undo" + android:icon="@drawable/icon_undo_disabled" + android:enabled="false" + android:visible="false"/> + + ) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * An additional term exception under section 7 of the GNU Affero + * General Public License, version 3, is available at + * http://developer.catrobat.org/license_additional_term + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package org.catrobat.catroid.test.ui + +import org.catrobat.catroid.ui.recyclerview.fragment.ProjectUndoManager +import org.catrobat.catroid.ui.recyclerview.fragment.ProjectUndoManager.VariableSnapshot +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +class ScriptUndoRegressionTest { + + @get:Rule + val tempFolder = TemporaryFolder() + + private lateinit var projectDir: File + private lateinit var undoManager: ProjectUndoManager + private lateinit var codeFile: File + + private val emptySnapshot = VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + + @Before + fun setUp() { + projectDir = tempFolder.newFolder("testProject") + codeFile = File(projectDir, "code.xml") + codeFile.writeText("original") + undoManager = ProjectUndoManager(projectDir) + } + + /** + * Simulates the scenario where a user inserts a brick. + * After an insert, copyProjectForUndoOption() pushes an undo state. + * The undo stack should contain exactly one entry that can be popped. + */ + @Test + fun testInsertBrickCreatesUndoEntry() { + undoManager.pushState("scene", "sprite", emptySnapshot) + + assertTrue("Undo should be available after inserting a brick", undoManager.canUndo()) + assertFalse("Redo should not be available after a fresh action", undoManager.canRedo()) + + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Should be able to pop the undo entry for insert", entry) + assertFalse("Undo stack should be empty after popping the only entry", undoManager.canUndo()) + } + + /** + * Simulates the scenario where a user copies a single brick. + * The copy action calls copyProjectForUndoOption() before cloning. + * The undo stack should contain one entry. + */ + @Test + fun testCopyBrickCreatesUndoEntry() { + codeFile.writeText("before-copy") + undoManager.pushState("scene", "sprite", emptySnapshot) + + assertTrue("Undo should be available after copying a brick", undoManager.canUndo()) + + codeFile.writeText("after-copy") + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Should be able to pop the undo entry for copy", entry) + + val restoredContent = codeFile.readText() + assertEquals( + "code.xml should be restored to the pre-copy snapshot", + "before-copy", + restoredContent + ) + } + + /** + * Simulates copying a block/control structure (composite brick). + * The behavior is the same as copying a single brick: one undo entry. + */ + @Test + fun testCopyBlockStructureCreatesUndoEntry() { + codeFile.writeText("before-copy-block") + undoManager.pushState("scene", "sprite", emptySnapshot) + + assertTrue("Undo should be available after copying a block structure", undoManager.canUndo()) + + codeFile.writeText("after-copy-block") + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Should be able to undo a block copy", entry) + + assertEquals( + "code.xml should be restored to the pre-copy-block snapshot", + "before-copy-block", + codeFile.readText() + ) + } + + /** + * Simulates the scenario where a user moves a brick and commits the move. + * With the new deferred undo approach, copyProjectForUndoOption() is called + * only when the move is committed (stopMoving). The undo stack should have one entry. + */ + @Test + fun testMoveBrickCommittedCreatesUndoEntry() { + codeFile.writeText("before-move") + + // onMoveCommitted calls copyProjectForUndoOption only on commit + undoManager.pushState("scene", "sprite", emptySnapshot) + + assertTrue("Undo should be available after a committed move", undoManager.canUndo()) + + codeFile.writeText("after-move") + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Should be able to undo a committed move", entry) + + assertEquals( + "code.xml should be restored to pre-move state", + "before-move", + codeFile.readText() + ) + } + + /** + * Simulates the scenario where a user moves a block/control structure and commits. + * Same behavior as single brick move. + */ + @Test + fun testMoveBlockStructureCommittedCreatesUndoEntry() { + codeFile.writeText("before-move-block") + undoManager.pushState("scene", "sprite", emptySnapshot) + + assertTrue("Undo should be available after a committed block move", undoManager.canUndo()) + + codeFile.writeText("after-move-block") + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Should be able to undo a committed block move", entry) + assertEquals( + "before-move-block", + codeFile.readText() + ) + } + + /** + * Simulates commenting/uncommenting bricks. + * The comment toggle calls copyProjectForUndoOption() before changing state. + */ + @Test + fun testCommentOutCreatesUndoEntry() { + codeFile.writeText("before-comment") + undoManager.pushState("scene", "sprite", emptySnapshot) + + assertTrue("Undo should be available after commenting out", undoManager.canUndo()) + + codeFile.writeText("after-comment") + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Should be able to undo a comment toggle", entry) + assertEquals( + "before-comment", + codeFile.readText() + ) + } + + /** + * Simulates uncommenting bricks - same mechanism as commenting. + */ + @Test + fun testCommentInCreatesUndoEntry() { + codeFile.writeText("before-uncomment") + undoManager.pushState("scene", "sprite", emptySnapshot) + + codeFile.writeText("after-uncomment") + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Should be able to undo an uncomment", entry) + assertEquals( + "before-uncomment", + codeFile.readText() + ) + } + + /** + * KEY TEST: Simulates the scenario where a user starts dragging a brick (move) + * but then cancels (back-press). With the deferred undo approach, NO undo entry + * should be created. This is the main fix requested by the maintainer. + * + * Before the fix, copyProjectForUndoOption() was called BEFORE startMoving(), + * so canceling would leave a no-op undo entry. Now, pushState is only called + * on commit (stopMoving), so canceling should result in an empty undo stack. + */ + @Test + fun testCancelMoveDoesNotCreateUndoEntry() { + // The user has NOT called pushState because the move was never committed. + // The pendingMoveUndoSnapshot flag would be set to true and then reset + // by cancelMove() without ever calling copyProjectForUndoOption(). + + // Simulate: no pushState was ever called + assertFalse( + "Undo should NOT be available after a canceled move", + undoManager.canUndo() + ) + assertFalse( + "Redo should NOT be available after a canceled move", + undoManager.canRedo() + ) + + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNull("Popping undo after cancel should return null", entry) + } + + /** + * Tests that canceling a move after a previous committed action does not + * create an additional undo entry. The undo stack should only contain + * the entry from the committed action. + */ + @Test + fun testCancelMovePreservesExistingUndoStack() { + // Simulate a previous committed action (e.g., delete) + codeFile.writeText("before-delete") + undoManager.pushState("scene", "sprite", emptySnapshot) + + assertTrue(undoManager.canUndo()) + + // Simulate user starts a drag but cancels — no additional pushState + // pendingMoveUndoSnapshot flag is set and cleared without pushing + + // Verify only one undo entry exists (from the delete, not from the canceled move) + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Should pop the undo from the prior delete action", entry) + + assertFalse( + "Undo stack should be empty — canceled move should NOT have added an entry", + undoManager.canUndo() + ) + } + + /** + * Tests the full undo/redo cycle: perform an action, undo it, then redo it. + */ + @Test + fun testUndoRedoCycle() { + codeFile.writeText("original-state") + undoManager.pushState("scene", "sprite", emptySnapshot) + + codeFile.writeText("modified-state") + + // Undo + assertTrue(undoManager.canUndo()) + val undoEntry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull("Undo should return an entry", undoEntry) + assertEquals("original-state", codeFile.readText()) + assertTrue("Redo should be available after undo", undoManager.canRedo()) + + // Redo + codeFile.writeText("original-state") + val redoEntry = undoManager.popRedo("scene", "sprite", emptySnapshot) + assertNotNull("Redo should return an entry", redoEntry) + assertEquals("modified-state", codeFile.readText()) + } + + /** + * Tests that performing a new action after an undo clears the redo stack. + */ + @Test + fun testNewActionAfterUndoClearsRedo() { + codeFile.writeText("state1") + undoManager.pushState("scene", "sprite", emptySnapshot) + + codeFile.writeText("state2") + + // Undo + undoManager.popUndo("scene", "sprite", emptySnapshot) + assertTrue("Redo should be available after undo", undoManager.canRedo()) + + // New action — should clear redo + codeFile.writeText("state1") + undoManager.pushState("scene", "sprite", emptySnapshot) + + assertFalse( + "Redo should be cleared after a new action following an undo", + undoManager.canRedo() + ) + } + + /** + * Tests multiple sequential actions create multiple undo entries. + */ + @Test + fun testMultipleActionsCreateMultipleUndoEntries() { + for (i in 1..5) { + codeFile.writeText("state-$i") + undoManager.pushState("scene", "sprite", emptySnapshot) + } + + var count = 0 + while (undoManager.canUndo()) { + val entry = undoManager.popUndo("scene", "sprite", emptySnapshot) + assertNotNull(entry) + count++ + } + assertEquals("Should have 5 undo entries for 5 actions", 5, count) + } + + /** + * Tests variable snapshot is properly stored and returned on undo. + */ + @Test + fun testVariableSnapshotPreservedInUndo() { + val snapshot = VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + undoManager.pushState("testScene", "testSprite", snapshot) + + val entry = undoManager.popUndo("testScene", "testSprite", emptySnapshot) + assertNotNull(entry) + assertEquals("testScene", entry!!.sceneName) + assertEquals("testSprite", entry.spriteName) + assertNotNull(entry.variableSnapshot) + } +} diff --git a/catroid/src/test/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectUndoManagerTest.kt b/catroid/src/test/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectUndoManagerTest.kt new file mode 100644 index 00000000000..5b29b722f44 --- /dev/null +++ b/catroid/src/test/java/org/catrobat/catroid/ui/recyclerview/fragment/ProjectUndoManagerTest.kt @@ -0,0 +1,146 @@ +/* + * Catroid: An on-device visual programming system for Android devices + * Copyright (C) 2010-2026 The Catrobat Team + * () + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * An additional term exception under section 7 of the GNU Affero + * General Public License, version 3, is available at + * http://developer.catrobat.org/license_additional_term + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package org.catrobat.catroid.ui.recyclerview.fragment + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.File + +class ProjectUndoManagerTest { + + @get:Rule + val tempFolder = TemporaryFolder() + + private lateinit var projectDir: File + private lateinit var undoManager: ProjectUndoManager + + @Before + fun setUp() { + projectDir = tempFolder.newFolder("testProject") + File(projectDir, "code.xml").createNewFile() + undoManager = ProjectUndoManager(projectDir) + } + + @Test + fun testUndoStackLimit() { + for (i in 0 until 25) { + undoManager.pushState( + "scene", "sprite", + ProjectUndoManager.VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + ) + } + + assertTrue("Undo stack should be limited", undoManager.canUndo()) + + var count = 0 + while (undoManager.popUndo( + "scene", "sprite", + ProjectUndoManager.VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + ) != null + ) { + count++ + } + assertEquals(20, count) + } + + @Test + fun testInitPrunesOldHistoryOnly() { + val undoDir = File(projectDir, "undo_history") + if (!undoDir.exists()) { + undoDir.mkdirs() + } + val oldFile = File(undoDir, "old_snap.xml") + oldFile.createNewFile() + // Set last modified to 2 hours ago + oldFile.setLastModified(System.currentTimeMillis() - 2 * 60 * 60 * 1000L) + + val recentFile = File(undoDir, "recent_snap.xml") + recentFile.createNewFile() + // Recent file is current + + undoManager = ProjectUndoManager(projectDir) + + assertTrue("Recent file should NOT be deleted during initialization", recentFile.exists()) + assertFalse("Old file should be deleted during initialization", oldFile.exists()) + assertTrue("Undo directory should still exist after initialization", undoDir.exists()) + } + + @Test + fun testClearHistory() { + undoManager.pushState( + "scene", "sprite", + ProjectUndoManager.VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + ) + assertTrue(undoManager.canUndo()) + + undoManager.clearHistory() + assertFalse("Undo stack should be empty after clearHistory", undoManager.canUndo()) + assertFalse("Redo stack should be empty after clearHistory", undoManager.canRedo()) + } + + @Test + fun testUniqueFilenames() { + undoManager.pushState( + "scene", "sprite", + ProjectUndoManager.VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + ) + undoManager.pushState( + "scene", "sprite", + ProjectUndoManager.VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + ) + + val entry1 = undoManager.popUndo( + "scene", "sprite", + ProjectUndoManager.VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + ) + val entry2 = undoManager.popUndo( + "scene", "sprite", + ProjectUndoManager.VariableSnapshot( + emptyList(), emptyList(), emptyList(), emptyList(), emptyList() + ) + ) + + assertTrue( + "Snapshot names should be unique", + entry1!!.snapshotFileName != entry2!!.snapshotFileName + ) + } +}