Skip to content

Commit 36dd404

Browse files
alperozturk96alperozturk96
authored
Merge pull request #16297 from nextcloud/ph/test_15636_deleting_from_slideshow
Bugfix and automated tests for #15636: deleting from slideshow
2 parents e507a67 + ba85d16 commit 36dd404

File tree

8 files changed

+387
-10
lines changed

8 files changed

+387
-10
lines changed

‎app/src/androidTest/java/com/nextcloud/test/ConnectivityServiceOfflineMock.kt‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info>
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
package com.nextcloud.test
8+
9+
import com.nextcloud.client.network.Connectivity
10+
import com.nextcloud.client.network.ConnectivityService
11+
12+
/** A mocked connectivity service returning that the device is offline **/
13+
class ConnectivityServiceOfflineMock : ConnectivityService {
14+
override fun isNetworkAndServerAvailable(callback: ConnectivityService.GenericCallback<Boolean>) {
15+
callback.onComplete(false)
16+
}
17+
18+
override fun isConnected(): Boolean = false
19+
20+
override fun isInternetWalled(): Boolean = false
21+
22+
override fun getConnectivity(): Connectivity = Connectivity.CONNECTED_WIFI
23+
}

‎app/src/androidTest/java/com/nextcloud/test/FileRemovedIdlingResource.kt‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info>
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
package com.nextcloud.test
8+
9+
import androidx.test.espresso.IdlingResource
10+
import com.owncloud.android.datamodel.FileDataStorageManager
11+
import com.owncloud.android.datamodel.OCFile
12+
import java.util.concurrent.atomic.AtomicLong
13+
import java.util.concurrent.atomic.AtomicReference
14+
15+
/**
16+
* IdlingResource that can be reused to watch the removal of different file ids sequentially.
17+
*
18+
* Use setFileId(fileId) before triggering the deletion. The resource will call the Espresso callback
19+
* once the file no longer exists. Call unregister from IdlingRegistry in @After.
20+
*/
21+
class FileRemovedIdlingResource(private val storageManager: FileDataStorageManager) : IdlingResource {
22+
private var resourceCallback: IdlingResource.ResourceCallback? = null
23+
24+
// null means "no file set"
25+
private var currentFile = AtomicReference<OCFile>(null)
26+
27+
override fun getName(): String = "${this::class.java.simpleName}"
28+
29+
override fun isIdleNow(): Boolean {
30+
val file = currentFile.get()
31+
// If no file set, consider idle. If file set, idle only if it doesn't exist.
32+
val idle = file == null || (!storageManager.fileExists(file.fileId) && !file.exists())
33+
if (idle && file != null) {
34+
// if we detect it's already removed, notify and clear
35+
resourceCallback?.onTransitionToIdle()
36+
currentFile.set(null)
37+
}
38+
return idle
39+
}
40+
41+
override fun registerIdleTransitionCallback(callback: IdlingResource.ResourceCallback) {
42+
this.resourceCallback = callback
43+
}
44+
45+
/**
46+
* Start watching the given file. Call this right before performing the UI action that triggers deletion.
47+
*/
48+
fun setFile(file: OCFile) {
49+
currentFile.set(file)
50+
}
51+
}

‎app/src/androidTest/java/com/nextcloud/test/LoopFailureHandler.kt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info>
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
package com.nextcloud.test
8+
9+
import android.content.Context
10+
import android.view.View
11+
import androidx.test.espresso.FailureHandler
12+
import androidx.test.espresso.base.DefaultFailureHandler
13+
import org.hamcrest.Matcher
14+
15+
/**
16+
* When testing inside of a loop, test failures are hard to attribute. For that, wrap them in an outer
17+
* exception detailing more about the context.
18+
*
19+
* Set the failure handler via
20+
* ```
21+
* Espresso.setFailureHandler(
22+
* LoopFailureHandler(targetContext, "Test failed in iteration $yourTestIterationCounter")
23+
* )
24+
* ```
25+
* and set it back to the default afterwards via
26+
* ```
27+
* Espresso.setFailureHandler(DefaultFailureHandler(targetContext))
28+
* ```
29+
*/
30+
class LoopFailureHandler(targetContext: Context, private val loopMessage: String) : FailureHandler {
31+
private val delegate: FailureHandler = DefaultFailureHandler(targetContext)
32+
33+
override fun handle(error: Throwable?, viewMatcher: Matcher<View?>?) {
34+
// Wrap in additional Exception
35+
delegate.handle(Exception(loopMessage, error), viewMatcher)
36+
}
37+
}

‎app/src/androidTest/java/com/owncloud/android/ui/preview/PreviewImageActivityIT.kt‎

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2025 Philipp Hasper <vcs@hasper.info>
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
package com.owncloud.android.ui.preview
8+
9+
import androidx.appcompat.widget.ActionBarContainer
10+
import androidx.test.core.app.launchActivity
11+
import androidx.test.espresso.Espresso
12+
import androidx.test.espresso.Espresso.onView
13+
import androidx.test.espresso.IdlingRegistry
14+
import androidx.test.espresso.action.ViewActions
15+
import androidx.test.espresso.assertion.ViewAssertions.matches
16+
import androidx.test.espresso.base.DefaultFailureHandler
17+
import androidx.test.espresso.matcher.RootMatchers.isDialog
18+
import androidx.test.espresso.matcher.ViewMatchers.isAssignableFrom
19+
import androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA
20+
import androidx.test.espresso.matcher.ViewMatchers.isDisplayed
21+
import androidx.test.espresso.matcher.ViewMatchers.isRoot
22+
import androidx.test.espresso.matcher.ViewMatchers.withId
23+
import androidx.test.espresso.matcher.ViewMatchers.withText
24+
import com.nextcloud.test.ConnectivityServiceOfflineMock
25+
import com.nextcloud.test.FileRemovedIdlingResource
26+
import com.nextcloud.test.LoopFailureHandler
27+
import com.owncloud.android.AbstractOnServerIT
28+
import com.owncloud.android.R
29+
import com.owncloud.android.datamodel.OCFile
30+
import com.owncloud.android.db.OCUpload
31+
import com.owncloud.android.files.services.NameCollisionPolicy
32+
import com.owncloud.android.lib.resources.files.ExistenceCheckRemoteOperation
33+
import org.hamcrest.Matchers.allOf
34+
import org.junit.After
35+
import org.junit.Assert.assertFalse
36+
import org.junit.Assert.assertTrue
37+
import org.junit.Before
38+
import org.junit.Ignore
39+
import org.junit.Test
40+
import java.io.File
41+
42+
class PreviewImageActivityIT : AbstractOnServerIT() {
43+
companion object {
44+
private const val REMOTE_FOLDER: String = "/PreviewImageActivityIT/"
45+
}
46+
47+
var fileRemovedIdlingResource = FileRemovedIdlingResource(storageManager)
48+
49+
@Suppress("SameParameterValue")
50+
private fun createLocalMockedImageFiles(count: Int): List<OCFile> {
51+
val srcPngFile = getFile("imageFile.png")
52+
return (0 until count).map { i ->
53+
val pngFile = File(srcPngFile.parent ?: ".", "image$i.png")
54+
srcPngFile.copyTo(pngFile, overwrite = true)
55+
56+
OCFile("/${pngFile.name}").apply {
57+
storagePath = pngFile.absolutePath
58+
mimeType = "image/png"
59+
modificationTimestamp = 1000000
60+
permissions = "D" // OCFile.PERMISSION_CAN_DELETE_OR_LEAVE_SHARE. Required for deletion button to show
61+
}.also {
62+
storageManager.saveNewFile(it)
63+
}
64+
}
65+
}
66+
67+
/**
68+
* Create image files and upload them to the connected server.
69+
*
70+
* This function relies on the images not existing beforehand, as AbstractOnServerIT#deleteAllFilesOnServer()
71+
* should clean up. If it does fail, likely because that clean up didn't work and there are leftovers from
72+
* a previous run
73+
* @param count Number of files to create
74+
* @param folder Parent folder to which to upload. Must start and end with a slash
75+
*/
76+
private fun createAndUploadImageFiles(count: Int, folder: String = REMOTE_FOLDER): List<OCFile> {
77+
val srcPngFile = getFile("imageFile.png")
78+
return (0 until count).map { i ->
79+
val pngFile = File(srcPngFile.parent ?: ".", "image$i.png")
80+
srcPngFile.copyTo(pngFile, overwrite = true)
81+
82+
val ocUpload = OCUpload(
83+
pngFile.absolutePath,
84+
folder + pngFile.name,
85+
account.name
86+
).apply {
87+
nameCollisionPolicy = NameCollisionPolicy.OVERWRITE
88+
}
89+
uploadOCUpload(ocUpload)
90+
91+
fileDataStorageManager.getFileByDecryptedRemotePath(folder + pngFile.name)!!
92+
}
93+
}
94+
95+
private fun veryImageThenDelete(testFile: OCFile) {
96+
Espresso.setFailureHandler(
97+
LoopFailureHandler(targetContext, "Test failed with image file ${testFile.fileName}")
98+
)
99+
100+
assertTrue(testFile.exists())
101+
assertTrue(testFile.fileExists())
102+
103+
onView(withId(R.id.image))
104+
.check(matches(isDisplayed()))
105+
106+
// Check that the Action Bar shows the file name as title
107+
onView(
108+
allOf(
109+
isDescendantOfA(isAssignableFrom(ActionBarContainer::class.java)),
110+
withText(testFile.fileName)
111+
)
112+
).check(matches(isDisplayed()))
113+
114+
// Open the Action Bar's overflow menu.
115+
// The official way would be:
116+
// openActionBarOverflowOrOptionsMenu(targetContext)
117+
// But this doesn't find the view. Presumably because Espresso.OVERFLOW_BUTTON_MATCHER looks for the description
118+
// "More options", whereas it actually says "More menu".
119+
// selecting by this would also work:
120+
// onView(withContentDescription("More menu")).perform(ViewActions.click())
121+
// For now, we identify it by the ID we know it to be
122+
onView(withId(R.id.custom_menu_placeholder_item)).perform(ViewActions.click())
123+
124+
// Click the "Remove" button
125+
onView(withText(R.string.common_remove)).perform(ViewActions.click())
126+
127+
// Check confirmation dialog and then confirm the deletion by clicking the main button of the dialog
128+
val expectedText = targetContext.getString(R.string.confirmation_remove_file_alert, testFile.fileName)
129+
onView(withId(android.R.id.message))
130+
.inRoot(isDialog())
131+
.check(matches(withText(expectedText)))
132+
133+
onView(withId(android.R.id.button1))
134+
.inRoot(isDialog())
135+
.check(matches(withText(R.string.file_delete)))
136+
.perform(ViewActions.click())
137+
138+
// Register the idling resource to wait for successful deletion
139+
fileRemovedIdlingResource.setFile(testFile)
140+
141+
// Wait for idle, then verify that the file is gone. Somehow waitForIdleSync() doesn't work and we need onIdle()
142+
Espresso.onIdle()
143+
assertFalse("test file still exists: ${testFile.fileName}", testFile.exists())
144+
145+
Espresso.setFailureHandler(DefaultFailureHandler(targetContext))
146+
}
147+
148+
private fun executeDeletionTestScenario(
149+
localOnly: Boolean,
150+
offline: Boolean,
151+
fileListTransformation: (List<OCFile>) -> List<OCFile>
152+
) {
153+
val imageCount = 5
154+
val testFiles = if (localOnly) {
155+
createLocalMockedImageFiles(
156+
imageCount
157+
)
158+
} else {
159+
createAndUploadImageFiles(imageCount)
160+
}
161+
val expectedFileOrder = fileListTransformation(testFiles)
162+
163+
val intent = PreviewImageActivity.previewFileIntent(targetContext, user, expectedFileOrder.first())
164+
launchActivity<PreviewImageActivity>(intent).use { scenario ->
165+
if (offline) {
166+
scenario.onActivity { activity ->
167+
activity.connectivityService = ConnectivityServiceOfflineMock()
168+
}
169+
}
170+
onView(isRoot()).check(matches(isDisplayed()))
171+
172+
for (testFile in expectedFileOrder) {
173+
veryImageThenDelete(testFile)
174+
assertTrue(
175+
"Test file still exists on the server: ${testFile.remotePath}",
176+
ExistenceCheckRemoteOperation(testFile.remotePath, true).execute(client).isSuccess
177+
)
178+
}
179+
}
180+
}
181+
182+
private fun testDeleteFromSlideshow_impl(localOnly: Boolean, offline: Boolean) {
183+
// Case 1: start at first image
184+
executeDeletionTestScenario(localOnly, offline) { list -> list }
185+
// Case 2: start at last image (reversed)
186+
executeDeletionTestScenario(localOnly, offline) { list -> list.reversed() }
187+
// Case 3: Start in the middle. From middle to the end, then backwards through remaining files of the first half
188+
executeDeletionTestScenario(localOnly, offline) { list ->
189+
list.subList(list.size / 2, list.size) + list.subList(0, list.size / 2).reversed()
190+
}
191+
}
192+
193+
@Before
194+
fun bringUp() {
195+
IdlingRegistry.getInstance().register(fileRemovedIdlingResource)
196+
}
197+
198+
@After
199+
fun tearDown() {
200+
IdlingRegistry.getInstance().unregister(fileRemovedIdlingResource)
201+
}
202+
203+
@Test
204+
fun deleteFromSlideshow_localOnly_online() {
205+
testDeleteFromSlideshow_impl(localOnly = true, offline = false)
206+
}
207+
208+
@Test
209+
fun deleteFromSlideshow_localOnly_offline() {
210+
testDeleteFromSlideshow_impl(localOnly = true, offline = true)
211+
}
212+
213+
@Test
214+
fun deleteFromSlideshow_remote_online() {
215+
testDeleteFromSlideshow_impl(localOnly = false, offline = false)
216+
}
217+
218+
@Test
219+
@Ignore(
220+
"Offline deletion is following a different UX and it is also brittle: Deletion might happen 10 minutes later"
221+
)
222+
fun deleteFromSlideshow_remote_offline() {
223+
// Note: the offline mock doesn't actually do what it is supposed to. The OfflineOperationsWorker uses its
224+
// own connectivityService, which is online, and may still execute the server deletion.
225+
// You'll need to address this, should you activate that test. Otherwise it might not catch all error cases
226+
testDeleteFromSlideshow_impl(localOnly = false, offline = true)
227+
}
228+
}

‎app/src/main/java/com/owncloud/android/ui/activity/FileDisplayActivity.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/*
22
* Nextcloud - Android Client
33
*
4+
* SPDX-FileCopyrightText: 2026 Philipp Hasper <vcs@hasper.info>
45
* SPDX-FileCopyrightText: 2025 Alper Ozturk <alper.ozturk@nextcloud.com>
56
* SPDX-FileCopyrightText: 2023-2024 TSI-mc <surinder.kumar@t-systems.com>
67
* SPDX-FileCopyrightText: 2023 Archontis E. Kostis <arxontisk02@gmail.com>
@@ -185,6 +186,7 @@ class FileDisplayActivity :
185186
OnEnforceableRefreshListener,
186187
OnSortingOrderListener,
187188
SendShareDialogDownloader,
189+
OnFilesRemovedListener,
188190
Injectable {
189191
private lateinit var binding: FilesBinding
190192

@@ -3026,6 +3028,10 @@ class FileDisplayActivity :
30263028
})
30273029
}
30283030

3031+
override fun onFilesRemoved() {
3032+
refreshCurrentDirectory()
3033+
}
3034+
30293035
private fun handleEcosystemIntent(intent: Intent?) {
30303036
ecosystemManager.receiveAccount(
30313037
intent,

‎app/src/main/java/com/owncloud/android/ui/activity/OnFilesRemovedListener.kt‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* Nextcloud - Android Client
3+
*
4+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
package com.owncloud.android.ui.activity
9+
10+
interface OnFilesRemovedListener {
11+
fun onFilesRemoved()
12+
}

0 commit comments

Comments
 (0)