From 03a3f12aa007cbf20d5e887fdc02cbbcbde0cf15 Mon Sep 17 00:00:00 2001 From: Li Wei Date: Tue, 10 Nov 2026 13:27:05 +0800 Subject: [PATCH 1/5] feat(logging): export logs from settings and control debug logging Assisted-by: Codex:GPT-6 Signed-off-by: Li Wei --- .../java/com/nextcloud/client/di/AppModule.kt | 5 +- .../com/nextcloud/client/logger/LoggerImpl.kt | 6 +- .../nextcloud/client/logger/LogsExporter.kt | 124 +++++++++++++ .../client/logger/ui/LogsExportController.kt | 90 ++++++++++ .../client/preferences/AppPreferences.java | 3 + .../preferences/AppPreferencesImpl.java | 11 ++ .../android/ui/activity/SettingsActivity.java | 58 +++++- app/src/main/res/values/strings.xml | 8 + app/src/main/res/xml/preferences.xml | 9 + .../client/logger/DebugLoggingTest.kt | 83 +++++++++ .../com/nextcloud/client/logger/LoggerTest.kt | 2 +- .../client/logger/LogsExporterTest.kt | 166 ++++++++++++++++++ .../DebugLoggingPreferencesTest.kt | 40 +++++ 13 files changed, 592 insertions(+), 13 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/client/logger/LogsExporter.kt create mode 100644 app/src/main/java/com/nextcloud/client/logger/ui/LogsExportController.kt create mode 100644 app/src/test/java/com/nextcloud/client/logger/DebugLoggingTest.kt create mode 100644 app/src/test/java/com/nextcloud/client/logger/LogsExporterTest.kt create mode 100644 app/src/test/java/com/nextcloud/client/preferences/DebugLoggingPreferencesTest.kt diff --git a/app/src/main/java/com/nextcloud/client/di/AppModule.kt b/app/src/main/java/com/nextcloud/client/di/AppModule.kt index 7dad415d7bc1..a9ceb7a3a8a3 100644 --- a/app/src/main/java/com/nextcloud/client/di/AppModule.kt +++ b/app/src/main/java/com/nextcloud/client/di/AppModule.kt @@ -145,10 +145,11 @@ internal class AppModule { @Provides @Singleton - fun logger(context: Context, clock: Clock): Logger { + fun logger(context: Context, clock: Clock, preferences: AppPreferences): Logger { val logDir = File(context.filesDir, LOG_DIR_NAME) val handler = FileLogHandler(logDir, LOG_FILE_NAME, LOG_FILE_MAX_SIZE_BYTES) - return LoggerImpl(clock, handler, Handler(), LOGGER_QUEUE_CAPACITY).apply { start() } + return LoggerImpl(clock, handler, Handler(), LOGGER_QUEUE_CAPACITY, preferences::isDebugLoggingEnabled) + .apply { start() } } @Provides diff --git a/app/src/main/java/com/nextcloud/client/logger/LoggerImpl.kt b/app/src/main/java/com/nextcloud/client/logger/LoggerImpl.kt index b4ab48bd0bf1..09e12cb8eab0 100644 --- a/app/src/main/java/com/nextcloud/client/logger/LoggerImpl.kt +++ b/app/src/main/java/com/nextcloud/client/logger/LoggerImpl.kt @@ -20,7 +20,8 @@ internal class LoggerImpl( private val clock: Clock, private val handler: FileLogHandler, private val mainThreadHandler: Handler, - queueCapacity: Int + queueCapacity: Int, + private val isDebugLoggingEnabled: () -> Boolean ) : Logger, LogsRepository { @@ -45,16 +46,19 @@ internal class LoggerImpl( } override fun v(tag: String, message: String) { + if (!isDebugLoggingEnabled()) return Log.v(tag, message) enqueue(Level.VERBOSE, tag, message) } override fun d(tag: String, message: String) { + if (!isDebugLoggingEnabled()) return Log.d(tag, message) enqueue(Level.DEBUG, tag, message) } override fun d(tag: String, message: String, t: Throwable) { + if (!isDebugLoggingEnabled()) return Log.d(tag, message, t) enqueue(Level.DEBUG, tag, message) } diff --git a/app/src/main/java/com/nextcloud/client/logger/LogsExporter.kt b/app/src/main/java/com/nextcloud/client/logger/LogsExporter.kt new file mode 100644 index 000000000000..09f0341a18d4 --- /dev/null +++ b/app/src/main/java/com/nextcloud/client/logger/LogsExporter.kt @@ -0,0 +1,124 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.logger + +import android.content.ContentValues +import android.content.Context +import android.net.Uri +import android.os.Build +import android.os.Environment +import android.provider.MediaStore +import androidx.annotation.RequiresApi +import com.nextcloud.client.core.Clock +import com.owncloud.android.lib.common.utils.Log_OC +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.withContext +import java.io.File +import java.io.IOException +import java.io.Writer +import java.text.SimpleDateFormat +import java.util.Locale +import javax.inject.Inject +import kotlin.coroutines.coroutineContext +import kotlin.coroutines.resume + +class LogsExporter @Inject constructor( + private val context: Context, + private val clock: Clock, + private val repository: LogsRepository +) { + suspend fun export(): Boolean = withContext(Dispatchers.IO) { + val entries = suspendCancellableCoroutine { continuation -> + repository.load { entries, _ -> + continuation.resume(entries) + } + } + if (entries.isEmpty()) return@withContext false + ensureActive() + + val timestamp = SimpleDateFormat("yyyy-MM-dd_HH-mm-ss-SSS", Locale.ROOT).apply { + timeZone = clock.tz + }.format(clock.currentDate) + val fileName = "Nextcloud-logs-$timestamp.txt" + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + writeToMediaStore(fileName, entries) + } else { + writeToPublicDirectory(fileName, entries) + } + true + } + + @RequiresApi(Build.VERSION_CODES.Q) + internal suspend fun writeToMediaStore(fileName: String, entries: List) { + val resolver = context.contentResolver + val uri = createPendingExport(fileName) + try { + val stream = resolver.openOutputStream(uri, "w") ?: throw IOException("Could not open log export") + stream.bufferedWriter(Charsets.UTF_8).use { writeEntries(it, entries) } + coroutineContext.ensureActive() + publishExport(uri) + } catch (e: Exception) { + runCatching { resolver.delete(uri, null, null) }.onFailure { + Log_OC.e(TAG, "Could not remove incomplete log export", it) + } + throw e + } + } + + @RequiresApi(Build.VERSION_CODES.Q) + private fun createPendingExport(fileName: String): Uri { + val values = ContentValues().apply { + put(MediaStore.MediaColumns.DISPLAY_NAME, fileName) + put(MediaStore.MediaColumns.MIME_TYPE, "text/plain") + put(MediaStore.MediaColumns.RELATIVE_PATH, "${Environment.DIRECTORY_DOWNLOADS}/Nextcloud") + put(MediaStore.MediaColumns.IS_PENDING, 1) + } + return context.contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, values) + ?: throw IOException("Could not create log export") + } + + @RequiresApi(Build.VERSION_CODES.Q) + private fun publishExport(uri: Uri) { + val published = ContentValues().apply { put(MediaStore.MediaColumns.IS_PENDING, 0) } + if (context.contentResolver.update(uri, published, null, null) != 1) { + throw IOException("Could not publish log export") + } + } + + @Suppress("DEPRECATION") + internal suspend fun writeToPublicDirectory(fileName: String, entries: List) { + val directory = File( + Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), + "Nextcloud" + ) + if (!directory.isDirectory && !directory.mkdirs()) { + throw IOException("Could not create log export directory") + } + val file = File.createTempFile(fileName.removeSuffix(".txt") + "-", ".txt", directory) + try { + file.bufferedWriter(Charsets.UTF_8).use { writeEntries(it, entries) } + } catch (e: Exception) { + file.delete() + throw e + } + } + + private suspend fun writeEntries(writer: Writer, entries: List) { + for (entry in entries) { + coroutineContext.ensureActive() + writer.write(entry.toString(clock.tz)) + writer.write("\n") + } + } + + companion object { + private const val TAG = "LogsExporter" + } +} diff --git a/app/src/main/java/com/nextcloud/client/logger/ui/LogsExportController.kt b/app/src/main/java/com/nextcloud/client/logger/ui/LogsExportController.kt new file mode 100644 index 000000000000..d0802290274f --- /dev/null +++ b/app/src/main/java/com/nextcloud/client/logger/ui/LogsExportController.kt @@ -0,0 +1,90 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.logger.ui + +import android.Manifest +import android.app.Activity +import android.content.pm.PackageManager +import android.os.Build +import android.preference.Preference +import android.widget.Toast +import androidx.core.content.ContextCompat +import com.nextcloud.client.logger.LogsExporter +import com.nextcloud.utils.mdm.MDMConfig +import com.owncloud.android.R +import com.owncloud.android.lib.common.utils.Log_OC +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.launch +import java.io.IOException + +@Suppress("DEPRECATION") +class LogsExportController( + private val activity: Activity, + private val exporter: LogsExporter, + private val preference: Preference +) { + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) + + fun export() { + if (!preference.isEnabled || !MDMConfig.isLogEnabled(activity)) return + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q && + ContextCompat.checkSelfPermission(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE) != + PackageManager.PERMISSION_GRANTED + ) { + activity.requestPermissions(arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE), STORAGE_PERMISSION_REQUEST) + return + } + + preference.isEnabled = false + scope.launch { + try { + val message = if (exporter.export()) { + R.string.logs_export_to_downloads_success + } else { + R.string.logs_export_to_downloads_empty + } + Toast.makeText(activity, message, Toast.LENGTH_LONG).show() + } catch (e: CancellationException) { + throw e + } catch (e: IOException) { + showError(e) + } catch (e: SecurityException) { + showError(e) + } finally { + preference.isEnabled = true + } + } + } + + fun onRequestPermissionsResult(requestCode: Int, grantResults: IntArray) { + if (requestCode != STORAGE_PERMISSION_REQUEST) return + if (grantResults.firstOrNull() == PackageManager.PERMISSION_GRANTED) { + export() + } else { + Toast.makeText(activity, R.string.logs_export_storage_permission_required, Toast.LENGTH_LONG).show() + } + } + + fun close() { + scope.cancel() + } + + private fun showError(exception: Exception) { + Log_OC.e(TAG, "Could not export logs", exception) + Toast.makeText(activity, R.string.logs_export_to_downloads_failed, Toast.LENGTH_LONG).show() + } + + companion object { + private const val TAG = "LogsExportController" + private const val STORAGE_PERMISSION_REQUEST = 101 + } +} diff --git a/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java b/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java index 4f4e85599480..38452dc7fc27 100644 --- a/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java +++ b/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java @@ -62,6 +62,9 @@ default void onDarkThemeModeChanged(DarkMode mode) { boolean instantVideoUploadEnabled(); boolean isDarkModeEnabled(); + boolean isDebugLoggingEnabled(); + void setDebugLoggingEnabled(boolean enabled); + boolean isShowHiddenFilesEnabled(); void setShowHiddenFilesEnabled(boolean enabled); diff --git a/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java b/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java index 07d50e6f48c1..4c741c21941d 100644 --- a/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java +++ b/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java @@ -116,6 +116,7 @@ public final class AppPreferencesImpl implements AppPreferences { private static final String LOG_ENTRY = "log_entry"; + private static final String PREF__DEBUG_LOGGING = "record_debug_logs"; private final Context context; private final SharedPreferences preferences; @@ -226,6 +227,16 @@ public boolean instantVideoUploadEnabled() { return preferences.getBoolean(PREF__INSTANT_VIDEO_UPLOADING, false); } + @Override + public boolean isDebugLoggingEnabled() { + return preferences.getBoolean(PREF__DEBUG_LOGGING, false); + } + + @Override + public void setDebugLoggingEnabled(boolean enabled) { + preferences.edit().putBoolean(PREF__DEBUG_LOGGING, enabled).apply(); + } + @Override public boolean isShowHiddenFilesEnabled() { return preferences.getBoolean(PREF__SHOW_HIDDEN_FILES, false); diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java index 0b8c51755f78..3118be7de35e 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java @@ -46,7 +46,9 @@ import com.nextcloud.client.account.UserAccountManager; import com.nextcloud.client.di.Injectable; import com.nextcloud.client.etm.EtmActivity; +import com.nextcloud.client.logger.LogsExporter; import com.nextcloud.client.logger.ui.LogsActivity; +import com.nextcloud.client.logger.ui.LogsExportController; import com.nextcloud.client.network.ClientFactory; import com.nextcloud.client.network.ConnectivityService; import com.nextcloud.client.preferences.AppPreferences; @@ -147,6 +149,7 @@ public class SettingsActivity extends PreferenceActivity private E2EDeletionService e2EDeletionService; private E2ECertificateRenewalService e2eCertificateRenewalService; + private LogsExportController logsExportController; private User user; @Inject ArbitraryDataProvider arbitraryDataProvider; @@ -155,6 +158,7 @@ public class SettingsActivity extends PreferenceActivity @Inject ClientFactory clientFactory; @Inject ViewThemeUtils viewThemeUtils; @Inject ConnectivityService connectivityService; + @Inject LogsExporter logsExporter; @SuppressWarnings("deprecation") @Override @@ -417,17 +421,42 @@ private void setupImprintPreference(PreferenceCategory preferenceCategoryMore) { private void setupLoggingPreference(PreferenceCategory preferenceCategoryMore) { Preference pLogger = findPreference("logger"); - if (pLogger != null) { - if (MDMConfig.INSTANCE.isLogEnabled(this)) { - pLogger.setOnPreferenceClickListener(preference -> { - Intent loggerIntent = new Intent(getApplicationContext(), LogsActivity.class); - startActivity(loggerIntent); - - return true; - }); - } else { + Preference pExport = findPreference("export_logs"); + ThemeableSwitchPreference pDebug = (ThemeableSwitchPreference) findPreference("record_debug_logs"); + if (!MDMConfig.INSTANCE.isLogEnabled(this)) { + if (pLogger != null) { preferenceCategoryMore.removePreference(pLogger); } + if (pExport != null) { + preferenceCategoryMore.removePreference(pExport); + } + if (pDebug != null) { + preferenceCategoryMore.removePreference(pDebug); + } + return; + } + + if (pLogger != null) { + pLogger.setOnPreferenceClickListener(preference -> { + Intent loggerIntent = new Intent(getApplicationContext(), LogsActivity.class); + startActivity(loggerIntent); + + return true; + }); + } + if (pDebug != null) { + pDebug.setChecked(preferences.isDebugLoggingEnabled()); + pDebug.setOnPreferenceChangeListener((preference, enabled) -> { + preferences.setDebugLoggingEnabled((Boolean) enabled); + return true; + }); + } + if (pExport != null) { + logsExportController = new LogsExportController(this, logsExporter, pExport); + pExport.setOnPreferenceClickListener(preference -> { + logsExportController.export(); + return true; + }); } } @@ -1237,8 +1266,19 @@ protected void onPostCreate(Bundle savedInstanceState) { getDelegate().onPostCreate(savedInstanceState); } + @Override + public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { + super.onRequestPermissionsResult(requestCode, permissions, grantResults); + if (logsExportController != null) { + logsExportController.onRequestPermissionsResult(requestCode, grantResults); + } + } + @Override protected void onDestroy() { + if (logsExportController != null) { + logsExportController.close(); + } super.onDestroy(); getDelegate().onDestroy(); } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f3d6f60bf3e1..a343b2ce356a 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -686,6 +686,14 @@ Secure connection redirected through an unsecured route. Logs + Record debug logs + Include debug and verbose messages in future logs + Export logs + Save app logs to Download/Nextcloud + Logs saved to Download/Nextcloud + No logs to export. Reproduce the issue and try again. + Could not save logs. Check available storage and try again. + Storage permission is needed to save logs to Download/Nextcloud. Refresh Send logs by email Export logs diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index aa73c18f2f11..8a67dad21559 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -129,6 +129,15 @@ + + diff --git a/app/src/test/java/com/nextcloud/client/logger/DebugLoggingTest.kt b/app/src/test/java/com/nextcloud/client/logger/DebugLoggingTest.kt new file mode 100644 index 000000000000..1fc8ae337dcd --- /dev/null +++ b/app/src/test/java/com/nextcloud/client/logger/DebugLoggingTest.kt @@ -0,0 +1,83 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.logger + +import android.os.Handler +import com.nextcloud.client.core.ClockImpl +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit + +class DebugLoggingTest { + @get:Rule + val directory = TemporaryFolder() + + @Test + fun normalLogsRemainAvailableWhenDebugLoggingIsDisabled() { + val logger = createLogger { false } + logger.d("test", "debug") + logger.d("test", "debug exception", IllegalStateException()) + logger.v("test", "verbose") + logger.i("test", "info") + logger.w("test", "warning") + logger.e("test", "error") + logger.e("test", "error exception", IllegalStateException()) + + assertEquals(listOf(Level.INFO, Level.WARNING, Level.ERROR, Level.ERROR), load(logger).map { it.level }) + } + + @Test + fun switchingDebugLoggingTakesEffectWithoutRestartingLogger() { + var enabled = false + val logger = createLogger { enabled } + logger.d("test", "before enabling") + enabled = true + logger.d("test", "debug") + logger.d("test", "debug exception", IllegalStateException()) + logger.v("test", "verbose") + enabled = false + logger.d("test", "after disabling") + logger.v("test", "after disabling verbose") + + assertEquals(listOf("debug", "debug exception", "verbose"), load(logger).map { it.message }) + } + + private fun createLogger(isDebugEnabled: () -> Boolean): LoggerImpl { + val handler: Handler = mock() + whenever(handler.post(any())).thenAnswer { + it.getArgument(0).run() + true + } + val logHandler = FileLogHandler(directory.newFolder(), "log.txt", LOG_SIZE) + return LoggerImpl(ClockImpl(), logHandler, handler, QUEUE_CAPACITY, isDebugEnabled).apply { start() } + } + + private fun load(logger: LoggerImpl): List { + val loaded = CountDownLatch(1) + var entries = emptyList() + logger.load { logs, _ -> + entries = logs + loaded.countDown() + } + assertTrue("Log snapshot was not delivered", loaded.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)) + return entries + } + + companion object { + private const val LOG_SIZE = 4096L + private const val QUEUE_CAPACITY = 100 + private const val TIMEOUT_SECONDS = 3L + } +} diff --git a/app/src/test/java/com/nextcloud/client/logger/LoggerTest.kt b/app/src/test/java/com/nextcloud/client/logger/LoggerTest.kt index 42114e551ebb..58bd0c90f51d 100644 --- a/app/src/test/java/com/nextcloud/client/logger/LoggerTest.kt +++ b/app/src/test/java/com/nextcloud/client/logger/LoggerTest.kt @@ -58,7 +58,7 @@ class LoggerTest { clock = ClockImpl() logHandler = spy(FileLogHandler(tempDir, "log.txt", FILE_SIZE)) osHandler = mock() - logger = LoggerImpl(clock, logHandler, osHandler, QUEUE_CAPACITY) + logger = LoggerImpl(clock, logHandler, osHandler, QUEUE_CAPACITY) { true } } @Test diff --git a/app/src/test/java/com/nextcloud/client/logger/LogsExporterTest.kt b/app/src/test/java/com/nextcloud/client/logger/LogsExporterTest.kt new file mode 100644 index 000000000000..bf1539f7c4e6 --- /dev/null +++ b/app/src/test/java/com/nextcloud/client/logger/LogsExporterTest.kt @@ -0,0 +1,166 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.logger + +import android.annotation.SuppressLint +import android.content.ContentResolver +import android.content.ContentValues +import android.content.Context +import android.net.Uri +import android.os.Environment +import android.provider.MediaStore +import com.nextcloud.client.core.Clock +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.mockito.Mockito.mockConstruction +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doThrow +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.IOException +import java.io.OutputStream +import java.util.Date +import java.util.TimeZone + +// Android storage APIs are mocked to exercise both paths on the host JVM. +@SuppressLint("NewApi") +class LogsExporterTest { + @get:Rule + val directory = TemporaryFolder() + + private val resolver: ContentResolver = mock() + private val context: Context = mock { + on { contentResolver }.thenReturn(resolver) + } + private val clock: Clock = mock { + on { tz }.thenReturn(TimeZone.getTimeZone("UTC")) + on { currentDate }.thenReturn(Date(0)) + } + private val repository: LogsRepository = mock() + private val exporter = LogsExporter(context, clock, repository) + private val uri: Uri = mock() + private val entries = listOf(LogEntry(Date(0), Level.ERROR, "test", "Failed to verify public key: 验证失败")) + + @Test + fun doesNotCreateAnEmptyExport() = runTest { + whenever(repository.load(any())).thenAnswer { + it.getArgument(0).invoke(emptyList(), 0) + } + + assertFalse(exporter.export()) + verify(resolver, never()).insert(anyOrNull(), any()) + } + + @Test + fun publishesUtf8LogsInNextcloudDownloadsDirectoryAfterWriting() = runTest { + mockConstruction(ContentValues::class.java).use { values -> + val output = ByteArrayOutputStream() + whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) + whenever(resolver.openOutputStream(uri, "w")).thenReturn(output) + whenever(resolver.update(eq(uri), any(), anyOrNull(), anyOrNull())).thenReturn(1) + + exporter.writeToMediaStore("logs.txt", entries) + + assertEquals(entries.single().toString(clock.tz) + "\n", output.toString(Charsets.UTF_8)) + verify(values.constructed()[0]).put( + MediaStore.MediaColumns.RELATIVE_PATH, + "${Environment.DIRECTORY_DOWNLOADS}/Nextcloud" + ) + verify(values.constructed()[0]).put(MediaStore.MediaColumns.IS_PENDING, 1) + verify(values.constructed()[1]).put(MediaStore.MediaColumns.IS_PENDING, 0) + verify(resolver, never()).delete(any(), anyOrNull(), anyOrNull()) + } + } + + @Test + fun reportsFailureWhenMediaStoreCannotCreateAFile() = runTest { + val result = runCatching { exporter.writeToMediaStore("logs.txt", entries) } + + assertTrue(result.exceptionOrNull() is IOException) + verify(resolver, never()).openOutputStream(any(), any()) + } + + @Test + fun removesPendingFileWhenOutputStreamIsUnavailable() = runTest { + whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) + + val result = runCatching { exporter.writeToMediaStore("logs.txt", entries) } + + assertTrue(result.exceptionOrNull() is IOException) + verify(resolver).delete(uri, null, null) + verify(resolver, never()).update(any(), any(), anyOrNull(), anyOrNull()) + } + + @Test + fun removesIncompleteFileWhenWritingFails() = runTest { + val output: OutputStream = mock() + whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) + whenever(resolver.openOutputStream(uri, "w")).thenReturn(output) + doThrow(IOException("Disk full")).whenever(output).write(any(), any(), any()) + + val result = runCatching { exporter.writeToMediaStore("logs.txt", entries) } + + assertTrue(result.exceptionOrNull() is IOException) + verify(resolver).delete(uri, null, null) + verify(resolver, never()).update(any(), any(), anyOrNull(), anyOrNull()) + } + + @Test + fun removesPendingFileWhenPublishingFails() = runTest { + whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) + whenever(resolver.openOutputStream(uri, "w")).thenReturn(ByteArrayOutputStream()) + + val result = runCatching { exporter.writeToMediaStore("logs.txt", entries) } + + assertTrue(result.exceptionOrNull() is IOException) + verify(resolver).delete(uri, null, null) + } + + @Test + fun legacyExportsUseNextcloudDirectoryWithoutOverwritingExistingLogs() = runTest { + mockStatic(Environment::class.java).use { environment -> + environment.`when` { + Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) + }.thenReturn(directory.root) + + exporter.writeToPublicDirectory("logs.txt", entries) + exporter.writeToPublicDirectory("logs.txt", entries) + + val files = File(directory.root, "Nextcloud").listFiles().orEmpty() + assertEquals(2, files.size) + files.forEach { assertEquals(entries.single().toString(clock.tz) + "\n", it.readText(Charsets.UTF_8)) } + } + } + + @Test + fun legacyExportReportsFailureWhenDestinationCannotBeCreated() = runTest { + mockStatic(Environment::class.java).use { environment -> + environment.`when` { + Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) + }.thenReturn(directory.root) + val existing = directory.newFile("Nextcloud").apply { writeText("existing data") } + + val result = runCatching { exporter.writeToPublicDirectory("logs.txt", entries) } + + assertTrue(result.exceptionOrNull() is IOException) + assertEquals("existing data", existing.readText()) + } + } +} diff --git a/app/src/test/java/com/nextcloud/client/preferences/DebugLoggingPreferencesTest.kt b/app/src/test/java/com/nextcloud/client/preferences/DebugLoggingPreferencesTest.kt new file mode 100644 index 000000000000..d5a49da336bf --- /dev/null +++ b/app/src/test/java/com/nextcloud/client/preferences/DebugLoggingPreferencesTest.kt @@ -0,0 +1,40 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.preferences + +import android.content.SharedPreferences +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.whenever + +class DebugLoggingPreferencesTest { + @Test + fun defaultsToDisabledAndPersistsBothSwitchStates() { + val values = mutableMapOf() + val sharedPreferences: SharedPreferences = mock() + val editor: SharedPreferences.Editor = mock() + whenever(sharedPreferences.getBoolean(any(), any())).thenAnswer { + values[it.getArgument(0)] ?: it.getArgument(1) + } + whenever(sharedPreferences.edit()).thenReturn(editor) + whenever(editor.putBoolean(any(), any())).thenAnswer { + values[it.getArgument(0)] = it.getArgument(1) + editor + } + + val preferences = AppPreferencesImpl(mock(), sharedPreferences, mock()) + assertFalse(preferences.isDebugLoggingEnabled) + preferences.setDebugLoggingEnabled(true) + assertTrue(AppPreferencesImpl(mock(), sharedPreferences, mock()).isDebugLoggingEnabled) + preferences.setDebugLoggingEnabled(false) + assertFalse(AppPreferencesImpl(mock(), sharedPreferences, mock()).isDebugLoggingEnabled) + } +} From e5326e7027a88788620cebc933f3c807748a1f53 Mon Sep 17 00:00:00 2001 From: Li Wei Date: Tue, 10 Nov 2026 14:33:42 +0800 Subject: [PATCH 2/5] refactor(logging): reuse existing log export flow Connect settings to LogsEmailSender and extend FileExportUtils with an optional Downloads subdirectory and reliable export results. Remove the separate exporter and controller while retaining background work, permission handling and failure feedback. Assisted-by: Codex:GPT-6 Signed-off-by: Li Wei --- .../nextcloud/client/logger/LogsExporter.kt | 124 ----------- .../client/logger/ui/LogsEmailSender.kt | 87 +++++--- .../client/logger/ui/LogsExportController.kt | 90 -------- .../android/ui/activity/SettingsActivity.java | 53 ++++- .../owncloud/android/utils/FileExportUtils.kt | 150 ++++++------- .../nextcloud/client/logger/LogsExportTest.kt | 204 ++++++++++++++++++ .../client/logger/LogsExporterTest.kt | 166 -------------- 7 files changed, 375 insertions(+), 499 deletions(-) delete mode 100644 app/src/main/java/com/nextcloud/client/logger/LogsExporter.kt delete mode 100644 app/src/main/java/com/nextcloud/client/logger/ui/LogsExportController.kt create mode 100644 app/src/test/java/com/nextcloud/client/logger/LogsExportTest.kt delete mode 100644 app/src/test/java/com/nextcloud/client/logger/LogsExporterTest.kt diff --git a/app/src/main/java/com/nextcloud/client/logger/LogsExporter.kt b/app/src/main/java/com/nextcloud/client/logger/LogsExporter.kt deleted file mode 100644 index 09f0341a18d4..000000000000 --- a/app/src/main/java/com/nextcloud/client/logger/LogsExporter.kt +++ /dev/null @@ -1,124 +0,0 @@ -/* - * Nextcloud - Android Client - * - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -package com.nextcloud.client.logger - -import android.content.ContentValues -import android.content.Context -import android.net.Uri -import android.os.Build -import android.os.Environment -import android.provider.MediaStore -import androidx.annotation.RequiresApi -import com.nextcloud.client.core.Clock -import com.owncloud.android.lib.common.utils.Log_OC -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.ensureActive -import kotlinx.coroutines.suspendCancellableCoroutine -import kotlinx.coroutines.withContext -import java.io.File -import java.io.IOException -import java.io.Writer -import java.text.SimpleDateFormat -import java.util.Locale -import javax.inject.Inject -import kotlin.coroutines.coroutineContext -import kotlin.coroutines.resume - -class LogsExporter @Inject constructor( - private val context: Context, - private val clock: Clock, - private val repository: LogsRepository -) { - suspend fun export(): Boolean = withContext(Dispatchers.IO) { - val entries = suspendCancellableCoroutine { continuation -> - repository.load { entries, _ -> - continuation.resume(entries) - } - } - if (entries.isEmpty()) return@withContext false - ensureActive() - - val timestamp = SimpleDateFormat("yyyy-MM-dd_HH-mm-ss-SSS", Locale.ROOT).apply { - timeZone = clock.tz - }.format(clock.currentDate) - val fileName = "Nextcloud-logs-$timestamp.txt" - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - writeToMediaStore(fileName, entries) - } else { - writeToPublicDirectory(fileName, entries) - } - true - } - - @RequiresApi(Build.VERSION_CODES.Q) - internal suspend fun writeToMediaStore(fileName: String, entries: List) { - val resolver = context.contentResolver - val uri = createPendingExport(fileName) - try { - val stream = resolver.openOutputStream(uri, "w") ?: throw IOException("Could not open log export") - stream.bufferedWriter(Charsets.UTF_8).use { writeEntries(it, entries) } - coroutineContext.ensureActive() - publishExport(uri) - } catch (e: Exception) { - runCatching { resolver.delete(uri, null, null) }.onFailure { - Log_OC.e(TAG, "Could not remove incomplete log export", it) - } - throw e - } - } - - @RequiresApi(Build.VERSION_CODES.Q) - private fun createPendingExport(fileName: String): Uri { - val values = ContentValues().apply { - put(MediaStore.MediaColumns.DISPLAY_NAME, fileName) - put(MediaStore.MediaColumns.MIME_TYPE, "text/plain") - put(MediaStore.MediaColumns.RELATIVE_PATH, "${Environment.DIRECTORY_DOWNLOADS}/Nextcloud") - put(MediaStore.MediaColumns.IS_PENDING, 1) - } - return context.contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, values) - ?: throw IOException("Could not create log export") - } - - @RequiresApi(Build.VERSION_CODES.Q) - private fun publishExport(uri: Uri) { - val published = ContentValues().apply { put(MediaStore.MediaColumns.IS_PENDING, 0) } - if (context.contentResolver.update(uri, published, null, null) != 1) { - throw IOException("Could not publish log export") - } - } - - @Suppress("DEPRECATION") - internal suspend fun writeToPublicDirectory(fileName: String, entries: List) { - val directory = File( - Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), - "Nextcloud" - ) - if (!directory.isDirectory && !directory.mkdirs()) { - throw IOException("Could not create log export directory") - } - val file = File.createTempFile(fileName.removeSuffix(".txt") + "-", ".txt", directory) - try { - file.bufferedWriter(Charsets.UTF_8).use { writeEntries(it, entries) } - } catch (e: Exception) { - file.delete() - throw e - } - } - - private suspend fun writeEntries(writer: Writer, entries: List) { - for (entry in entries) { - coroutineContext.ensureActive() - writer.write(entry.toString(clock.tz)) - writer.write("\n") - } - } - - companion object { - private const val TAG = "LogsExporter" - } -} diff --git a/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt b/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt index 4100364039f8..658af28280a4 100644 --- a/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt +++ b/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt @@ -24,26 +24,29 @@ import com.nextcloud.client.core.Cancellable import com.nextcloud.client.core.Clock import com.nextcloud.client.logger.LogEntry import com.owncloud.android.R +import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.ui.notifications.NotificationUtils import com.owncloud.android.utils.FileExportUtils import java.io.File import java.security.SecureRandom import java.util.TimeZone +import java.util.function.IntConsumer +import javax.inject.Inject -class LogsEmailSender(private val context: Context, private val clock: Clock, private val runner: AsyncRunner) { +class LogsEmailSender @Inject constructor( + private val context: Context, + private val clock: Clock, + private val runner: AsyncRunner +) { private companion object { const val LOGS_MIME_TYPE = "text/plain" } - private class Task( - private val context: Context, - private val logs: List, - private val file: File, - private val tz: TimeZone - ) : Function0 { + private class Task(private val logs: List, private val file: File, private val tz: TimeZone) : + Function0 { - override fun invoke(): Uri? { + override fun invoke(): File { file.parentFile?.mkdirs() file.outputStream().use { outputStream -> @@ -55,7 +58,7 @@ class LogsEmailSender(private val context: Context, private val clock: Clock, pr } } - return FileProvider.getUriForFile(context, context.getString(R.string.file_provider_authority), file) + return file } } @@ -64,21 +67,58 @@ class LogsEmailSender(private val context: Context, private val clock: Clock, pr fun send(logs: List) { if (task == null) { val outFile = File(context.cacheDir, "attachments/logs.txt") - task = runner.postQuickTask(Task(context, logs, outFile, clock.tz), onResult = { + task = runner.postQuickTask(Task(logs, outFile, clock.tz), onResult = { task = null - send(it) + send(FileProvider.getUriForFile(context, context.getString(R.string.file_provider_authority), it)) }) } } - fun export(logs: List) { - if (task == null) { - val outFile = File(context.cacheDir, "attachments/logs.txt") - task = runner.postQuickTask(Task(context, logs, outFile, clock.tz), onResult = { - task = null - export(outFile) - }) + @JvmOverloads + fun export( + logs: List, + onComplete: IntConsumer = IntConsumer { message -> + if (message == R.string.logs_export_to_downloads_success) { + showSuccessNotification(1) + } else { + Toast.makeText(context, message, Toast.LENGTH_LONG).show() + } + } + ) { + if (task != null) return + if (logs.isEmpty()) { + onComplete.accept(R.string.logs_export_to_downloads_empty) + return } + task = runner.postQuickTask( + task = { + val outFile = File.createTempFile("logs-", ".txt", context.cacheDir) + try { + Task(logs, outFile, clock.tz).invoke() + FileExportUtils().exportFile( + "Nextcloud Android Files Logs.txt", + LOGS_MIME_TYPE, + context.contentResolver, + null, + outFile, + "Nextcloud" + ) + } finally { + outFile.delete() + } + }, + onResult = { success -> + task = null + onComplete.accept( + if (success) R.string.logs_export_to_downloads_success else R.string.logs_export_to_downloads_failed + ) + }, + onError = { + task = null + Log_OC.e(this, "Could not export logs", it) + onComplete.accept(R.string.logs_export_to_downloads_failed) + } + ) } fun stop() { @@ -88,17 +128,6 @@ class LogsEmailSender(private val context: Context, private val clock: Clock, pr } } - private fun export(file: File) { - FileExportUtils().exportFile( - "Nextcloud Android Files Logs", - "text/plain", - context.contentResolver, - null, - file - ) - showSuccessNotification(1) - } - fun showSuccessNotification(successfulExports: Int) { showNotification( context.resources.getQuantityString( diff --git a/app/src/main/java/com/nextcloud/client/logger/ui/LogsExportController.kt b/app/src/main/java/com/nextcloud/client/logger/ui/LogsExportController.kt deleted file mode 100644 index d0802290274f..000000000000 --- a/app/src/main/java/com/nextcloud/client/logger/ui/LogsExportController.kt +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Nextcloud - Android Client - * - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -package com.nextcloud.client.logger.ui - -import android.Manifest -import android.app.Activity -import android.content.pm.PackageManager -import android.os.Build -import android.preference.Preference -import android.widget.Toast -import androidx.core.content.ContextCompat -import com.nextcloud.client.logger.LogsExporter -import com.nextcloud.utils.mdm.MDMConfig -import com.owncloud.android.R -import com.owncloud.android.lib.common.utils.Log_OC -import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel -import kotlinx.coroutines.launch -import java.io.IOException - -@Suppress("DEPRECATION") -class LogsExportController( - private val activity: Activity, - private val exporter: LogsExporter, - private val preference: Preference -) { - private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) - - fun export() { - if (!preference.isEnabled || !MDMConfig.isLogEnabled(activity)) return - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q && - ContextCompat.checkSelfPermission(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE) != - PackageManager.PERMISSION_GRANTED - ) { - activity.requestPermissions(arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE), STORAGE_PERMISSION_REQUEST) - return - } - - preference.isEnabled = false - scope.launch { - try { - val message = if (exporter.export()) { - R.string.logs_export_to_downloads_success - } else { - R.string.logs_export_to_downloads_empty - } - Toast.makeText(activity, message, Toast.LENGTH_LONG).show() - } catch (e: CancellationException) { - throw e - } catch (e: IOException) { - showError(e) - } catch (e: SecurityException) { - showError(e) - } finally { - preference.isEnabled = true - } - } - } - - fun onRequestPermissionsResult(requestCode: Int, grantResults: IntArray) { - if (requestCode != STORAGE_PERMISSION_REQUEST) return - if (grantResults.firstOrNull() == PackageManager.PERMISSION_GRANTED) { - export() - } else { - Toast.makeText(activity, R.string.logs_export_storage_permission_required, Toast.LENGTH_LONG).show() - } - } - - fun close() { - scope.cancel() - } - - private fun showError(exception: Exception) { - Log_OC.e(TAG, "Could not export logs", exception) - Toast.makeText(activity, R.string.logs_export_to_downloads_failed, Toast.LENGTH_LONG).show() - } - - companion object { - private const val TAG = "LogsExportController" - private const val STORAGE_PERMISSION_REQUEST = 101 - } -} diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java index 3118be7de35e..6028f78cc6c9 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java @@ -15,15 +15,18 @@ */ package com.owncloud.android.ui.activity; +import android.Manifest; import android.annotation.SuppressLint; import android.app.Activity; import android.content.Intent; import android.content.SharedPreferences; import android.content.pm.PackageInfo; +import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; import android.content.res.Configuration; import android.graphics.drawable.Drawable; import android.net.Uri; +import android.os.Build; import android.os.Bundle; import android.os.Handler; import android.os.Looper; @@ -40,15 +43,16 @@ import android.view.View; import android.view.ViewGroup; import android.webkit.URLUtil; +import android.widget.Toast; import com.google.android.material.dialog.MaterialAlertDialogBuilder; import com.nextcloud.client.account.User; import com.nextcloud.client.account.UserAccountManager; import com.nextcloud.client.di.Injectable; import com.nextcloud.client.etm.EtmActivity; -import com.nextcloud.client.logger.LogsExporter; +import com.nextcloud.client.logger.LogsRepository; import com.nextcloud.client.logger.ui.LogsActivity; -import com.nextcloud.client.logger.ui.LogsExportController; +import com.nextcloud.client.logger.ui.LogsEmailSender; import com.nextcloud.client.network.ClientFactory; import com.nextcloud.client.network.ConnectivityService; import com.nextcloud.client.preferences.AppPreferences; @@ -130,6 +134,7 @@ public class SettingsActivity extends PreferenceActivity private static final int ACTION_REQUEST_CODE_DAVDROID_SETUP = 10; private static final int ACTION_SHOW_MNEMONIC = 11; private static final int ACTION_E2E = 12; + private static final int REQUEST_EXPORT_LOGS_PERMISSION = 101; private static final int TRUE_VALUE = 1; private static final String DAV_PATH = "/remote.php/dav"; @@ -149,7 +154,6 @@ public class SettingsActivity extends PreferenceActivity private E2EDeletionService e2EDeletionService; private E2ECertificateRenewalService e2eCertificateRenewalService; - private LogsExportController logsExportController; private User user; @Inject ArbitraryDataProvider arbitraryDataProvider; @@ -158,7 +162,8 @@ public class SettingsActivity extends PreferenceActivity @Inject ClientFactory clientFactory; @Inject ViewThemeUtils viewThemeUtils; @Inject ConnectivityService connectivityService; - @Inject LogsExporter logsExporter; + @Inject LogsRepository logsRepository; + @Inject LogsEmailSender logsEmailSender; @SuppressWarnings("deprecation") @Override @@ -452,14 +457,38 @@ private void setupLoggingPreference(PreferenceCategory preferenceCategoryMore) { }); } if (pExport != null) { - logsExportController = new LogsExportController(this, logsExporter, pExport); pExport.setOnPreferenceClickListener(preference -> { - logsExportController.export(); + exportLogs(); return true; }); } } + private void exportLogs() { + Preference preference = findPreference("export_logs"); + if (preference == null || !preference.isEnabled() || !MDMConfig.INSTANCE.isLogEnabled(this)) { + return; + } + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q + && ContextCompat.checkSelfPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE) + != PackageManager.PERMISSION_GRANTED) { + requestPermissions(new String[]{Manifest.permission.WRITE_EXTERNAL_STORAGE}, REQUEST_EXPORT_LOGS_PERMISSION); + return; + } + preference.setEnabled(false); + logsRepository.load((entries, size) -> { + if (!isDestroyed()) { + logsEmailSender.export(entries, message -> { + if (!isDestroyed()) { + preference.setEnabled(true); + Toast.makeText(this, message, Toast.LENGTH_LONG).show(); + } + }); + } + return Unit.INSTANCE; + }); + } + private void setupRecommendPreference(PreferenceCategory preferenceCategoryMore) { boolean recommendEnabled = getResources().getBoolean(R.bool.recommend_enabled); @@ -1269,16 +1298,18 @@ protected void onPostCreate(Bundle savedInstanceState) { @Override public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { super.onRequestPermissionsResult(requestCode, permissions, grantResults); - if (logsExportController != null) { - logsExportController.onRequestPermissionsResult(requestCode, grantResults); + if (requestCode == REQUEST_EXPORT_LOGS_PERMISSION) { + if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) { + exportLogs(); + } else { + Toast.makeText(this, R.string.logs_export_storage_permission_required, Toast.LENGTH_LONG).show(); + } } } @Override protected void onDestroy() { - if (logsExportController != null) { - logsExportController.close(); - } + logsEmailSender.stop(); super.onDestroy(); getDelegate().onDestroy(); } diff --git a/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt b/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt index b13936b94699..638a33ff293d 100644 --- a/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt +++ b/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt @@ -7,9 +7,9 @@ */ package com.owncloud.android.utils -import android.annotation.SuppressLint import android.content.ContentResolver import android.content.ContentValues +import android.net.Uri import android.os.Build import android.os.Environment import android.provider.MediaStore @@ -18,133 +18,126 @@ import com.owncloud.android.datamodel.OCFile import com.owncloud.android.lib.common.utils.Log_OC import java.io.File import java.io.FileInputStream -import java.io.FileNotFoundException import java.io.FileOutputStream import java.io.IOException -import java.io.InputStream +import java.io.OutputStream class FileExportUtils { @Throws(IllegalStateException::class) - fun exportFile(fileName: String, mimeType: String, contentResolver: ContentResolver, ocFile: OCFile?, file: File?) { + fun exportFile( + fileName: String, + mimeType: String, + contentResolver: ContentResolver, + ocFile: OCFile?, + file: File?, + subdirectory: String = "" + ): Boolean = try { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { exportFileAndroid10AndAbove( fileName, mimeType, contentResolver, ocFile, - file + file, + subdirectory ) } else { exportFilesBelowAndroid10( fileName, contentResolver, ocFile, - file + file, + subdirectory ) } + true + } catch (e: IOException) { + Log_OC.e(this, "Cannot export file", e) + false + } catch (e: SecurityException) { + Log_OC.e(this, "Cannot access export destination", e) + false } - @SuppressLint("Recycle") // handled inside copy method @RequiresApi(Build.VERSION_CODES.Q) - private fun exportFileAndroid10AndAbove( + internal fun exportFileAndroid10AndAbove( fileName: String, mimeType: String, contentResolver: ContentResolver, ocFile: OCFile?, - file: File? + file: File?, + subdirectory: String ) { val cv = ContentValues().apply { put(MediaStore.MediaColumns.DISPLAY_NAME, fileName) put(MediaStore.MediaColumns.MIME_TYPE, mimeType) - put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS) + put(MediaStore.MediaColumns.RELATIVE_PATH, "${Environment.DIRECTORY_DOWNLOADS}/$subdirectory") + put(MediaStore.MediaColumns.IS_PENDING, 1) } - var uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) - - if (uri == null) { - var count = INITIAL_RENAME_COUNT - do { - val name = generateNewName(fileName, count) - cv.put(MediaStore.MediaColumns.DISPLAY_NAME, name) - uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) - - count++ - } while (uri == null) + val uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) + ?: throw IOException("Could not create export") + try { + writeMediaStoreExport(ocFile, file, contentResolver, uri) + } catch (e: Exception) { + runCatching { contentResolver.delete(uri, null, null) }.onFailure { + Log_OC.e(this, "Could not remove incomplete export", it) + } + throw e } + } - copy( - ocFile, - file, - contentResolver, - FileOutputStream(contentResolver.openFileDescriptor(uri, "w")?.fileDescriptor) - ) + @RequiresApi(Build.VERSION_CODES.Q) + private fun writeMediaStoreExport(ocFile: OCFile?, file: File?, contentResolver: ContentResolver, uri: Uri) { + val stream = contentResolver.openOutputStream(uri, "w") ?: throw IOException("Could not open export") + copy(ocFile, file, contentResolver, stream) + val published = ContentValues().apply { put(MediaStore.MediaColumns.IS_PENDING, 0) } + if (contentResolver.update(uri, published, null, null) != 1) { + throw IOException("Could not publish export") + } } - private fun exportFilesBelowAndroid10( + internal fun exportFilesBelowAndroid10( fileName: String, contentResolver: ContentResolver, ocFile: OCFile?, - file: File? + file: File?, + subdirectory: String ) { + val directory = File( + Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), + subdirectory + ) + if (!directory.isDirectory && !directory.mkdirs()) { + throw IOException("Could not create export directory") + } + var target = File(directory, fileName) + var count = INITIAL_RENAME_COUNT + while (!target.createNewFile()) { + target = File(directory, generateNewName(fileName, count++)) + } try { - var target = File( - Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), - fileName - ) - - if (target.exists()) { - var count = INITIAL_RENAME_COUNT - do { - val name = generateNewName(fileName, count) - target = File( - Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), - name - ) - - count++ - } while (target.exists()) - } - - copy( - ocFile, - file, - contentResolver, - FileOutputStream(target) - ) - } catch (e: FileNotFoundException) { - Log_OC.e(this, "File not found", e) - } catch (e: IOException) { - Log_OC.e(this, "Cannot write file", e) + copy(ocFile, file, contentResolver, FileOutputStream(target)) + } catch (e: Exception) { + target.delete() + throw e } } @Throws(IllegalStateException::class) - private fun copy(ocFile: OCFile?, file: File?, contentResolver: ContentResolver, outputStream: FileOutputStream) { + private fun copy(ocFile: OCFile?, file: File?, contentResolver: ContentResolver, outputStream: OutputStream) { outputStream.use { fos -> - try { - val inputStream = when { - ocFile != null -> contentResolver.openInputStream(ocFile.storageUri) - file != null -> FileInputStream(file) - else -> error("ocFile and file both may not be null") - }!! - - inputStream.use { fis -> - copyStream(fis, fos) - } - } catch (e: IOException) { - Log_OC.e(this, "Cannot write file", e) + val inputStream = when { + ocFile != null -> contentResolver.openInputStream(ocFile.storageUri) + file != null -> FileInputStream(file) + else -> error("ocFile and file both may not be null") + } ?: throw IOException("Could not open export source") + inputStream.use { fis -> + fis.copyTo(fos) } } } - private fun copyStream(inputStream: InputStream, outputStream: FileOutputStream) { - val buffer = ByteArray(COPY_BUFFER_SIZE) - var len: Int - while (inputStream.read(buffer).also { len = it } != -1) { - outputStream.write(buffer, 0, len) - } - } - private fun generateNewName(name: String, count: Int): String { val extPos = name.lastIndexOf('.') val suffix = " ($count)" @@ -161,6 +154,5 @@ class FileExportUtils { companion object { private const val INITIAL_RENAME_COUNT = 2 - private const val COPY_BUFFER_SIZE = 1024 } } diff --git a/app/src/test/java/com/nextcloud/client/logger/LogsExportTest.kt b/app/src/test/java/com/nextcloud/client/logger/LogsExportTest.kt new file mode 100644 index 000000000000..54c4ffbd2487 --- /dev/null +++ b/app/src/test/java/com/nextcloud/client/logger/LogsExportTest.kt @@ -0,0 +1,204 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.logger + +import android.annotation.SuppressLint +import android.content.ContentResolver +import android.content.ContentValues +import android.content.Context +import android.net.Uri +import android.os.Environment +import android.provider.MediaStore +import com.nextcloud.client.core.Clock +import com.nextcloud.client.core.ManualAsyncRunner +import com.nextcloud.client.logger.ui.LogsEmailSender +import com.owncloud.android.R +import com.owncloud.android.utils.FileExportUtils +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.mockito.Mockito.mockConstruction +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull +import org.mockito.kotlin.doThrow +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.IOException +import java.io.OutputStream +import java.util.Date +import java.util.TimeZone +import java.util.function.IntConsumer + +// Android storage APIs are mocked to exercise both paths on the host JVM. +@SuppressLint("NewApi") +class LogsExportTest { + @get:Rule + val directory = TemporaryFolder() + + private val resolver: ContentResolver = mock() + private val context: Context = mock { + on { contentResolver }.thenReturn(resolver) + on { cacheDir }.thenAnswer { File(directory.root, "cache").apply { mkdirs() } } + } + private val clock: Clock = mock { + on { tz }.thenReturn(TimeZone.getTimeZone("UTC")) + } + private val runner = ManualAsyncRunner() + private val sender = LogsEmailSender(context, clock, runner) + private val onComplete: IntConsumer = mock() + private val exporter = FileExportUtils() + private val uri: Uri = mock() + private val entries = listOf(LogEntry(Date(0), Level.ERROR, "test", "Failed to verify public key: 验证失败")) + private val content get() = entries.single().toString(clock.tz) + System.lineSeparator() + private val source by lazy { directory.newFile("source.txt").apply { writeText(content, Charsets.UTF_8) } } + + @Test + fun doesNotCreateAnEmptyExport() { + sender.export(emptyList(), onComplete) + + assertTrue(runner.isEmpty) + verify(onComplete).accept(R.string.logs_export_to_downloads_empty) + } + + @Test + fun publishesUtf8LogsInNextcloudDownloadsDirectoryAfterWriting() { + mockConstruction(ContentValues::class.java).use { values -> + val output = ByteArrayOutputStream() + whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) + whenever(resolver.openOutputStream(uri, "w")).thenReturn(output) + whenever(resolver.update(eq(uri), any(), anyOrNull(), anyOrNull())).thenReturn(1) + + exportToMediaStore() + + assertEquals(content, output.toString(Charsets.UTF_8)) + verify(values.constructed()[0]).put( + MediaStore.MediaColumns.RELATIVE_PATH, + "${Environment.DIRECTORY_DOWNLOADS}/Nextcloud" + ) + verify(values.constructed()[0]).put(MediaStore.MediaColumns.IS_PENDING, 1) + verify(values.constructed()[1]).put(MediaStore.MediaColumns.IS_PENDING, 0) + verify(resolver, never()).delete(any(), anyOrNull(), anyOrNull()) + } + } + + @Test + fun reportsFailureWhenMediaStoreCannotCreateAFile() { + assertTrue(runCatching { exportToMediaStore() }.exceptionOrNull() is IOException) + verify(resolver, never()).openOutputStream(any(), any()) + } + + @Test + fun removesPendingFileWhenOutputStreamIsUnavailable() { + whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) + + assertTrue(runCatching { exportToMediaStore() }.exceptionOrNull() is IOException) + verify(resolver).delete(uri, null, null) + verify(resolver, never()).update(any(), any(), anyOrNull(), anyOrNull()) + } + + @Test + fun removesIncompleteFileWhenWritingFails() { + val output: OutputStream = mock() + whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) + whenever(resolver.openOutputStream(uri, "w")).thenReturn(output) + doThrow(IOException("Disk full")).whenever(output).write(any(), any(), any()) + + assertTrue(runCatching { exportToMediaStore() }.exceptionOrNull() is IOException) + verify(resolver).delete(uri, null, null) + verify(resolver, never()).update(any(), any(), anyOrNull(), anyOrNull()) + verify(output).close() + } + + @Test + fun removesPendingFileWhenPublishingFails() { + whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) + whenever(resolver.openOutputStream(uri, "w")).thenReturn(ByteArrayOutputStream()) + + assertTrue(runCatching { exportToMediaStore() }.exceptionOrNull() is IOException) + verify(resolver).delete(uri, null, null) + } + + @Test + fun settingsAndLogViewerShareBackgroundExportWithoutOverwritingExistingLogs() = withDownloads { + sender.export(entries, onComplete) + sender.export(entries, onComplete) + assertEquals(1, runner.size) + assertFalse(File(directory.root, "Nextcloud").exists()) + runner.runAll() + sender.export(entries, onComplete) + runner.runAll() + + val files = File(directory.root, "Nextcloud").listFiles().orEmpty() + assertEquals(2, files.size) + files.forEach { assertEquals(content, it.readText(Charsets.UTF_8)) } + verify(onComplete, times(2)).accept(R.string.logs_export_to_downloads_success) + assertTrue(context.cacheDir.listFiles().isNullOrEmpty()) + } + + @Test + fun reportsFailureWithoutOverwritingAnExistingDestination() = withDownloads { + val existing = directory.newFile("Nextcloud").apply { writeText("existing data") } + + sender.export(entries, onComplete) + runner.runAll() + + verify(onComplete).accept(R.string.logs_export_to_downloads_failed) + verify(onComplete, never()).accept(R.string.logs_export_to_downloads_success) + assertEquals("existing data", existing.readText()) + } + + @Test + fun reportsSerializationFailureAndAllowsRetry() { + directory.newFile("cache") + sender.export(entries, onComplete) + runner.runAll() + verify(onComplete).accept(R.string.logs_export_to_downloads_failed) + + sender.export(entries, onComplete) + assertEquals(1, runner.size) + } + + @Test + fun stoppingCancelsQueuedExport() { + sender.export(entries, onComplete) + sender.stop() + + assertTrue(runner.isEmpty) + verify(onComplete, never()).accept(any()) + } + + @Test + fun otherExportsKeepTheDownloadsRootAndCleanUpFailedWrites() = withDownloads { + assertTrue(exporter.exportFile("other.txt", "text/plain", resolver, null, source)) + assertEquals(content, File(directory.root, "other.txt").readText(Charsets.UTF_8)) + assertFalse(exporter.exportFile("missing.txt", "text/plain", resolver, null, File(directory.root, "no-source"))) + assertFalse(File(directory.root, "missing.txt").exists()) + } + + private fun exportToMediaStore() = + exporter.exportFileAndroid10AndAbove("logs.txt", "text/plain", resolver, null, source, "Nextcloud") + + private fun withDownloads(block: () -> Unit) { + mockStatic(Environment::class.java).use { environment -> + environment.`when` { + Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) + }.thenReturn(directory.root) + block() + } + } +} diff --git a/app/src/test/java/com/nextcloud/client/logger/LogsExporterTest.kt b/app/src/test/java/com/nextcloud/client/logger/LogsExporterTest.kt deleted file mode 100644 index bf1539f7c4e6..000000000000 --- a/app/src/test/java/com/nextcloud/client/logger/LogsExporterTest.kt +++ /dev/null @@ -1,166 +0,0 @@ -/* - * Nextcloud - Android Client - * - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -package com.nextcloud.client.logger - -import android.annotation.SuppressLint -import android.content.ContentResolver -import android.content.ContentValues -import android.content.Context -import android.net.Uri -import android.os.Environment -import android.provider.MediaStore -import com.nextcloud.client.core.Clock -import kotlinx.coroutines.test.runTest -import org.junit.Assert.assertEquals -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Rule -import org.junit.Test -import org.junit.rules.TemporaryFolder -import org.mockito.Mockito.mockConstruction -import org.mockito.Mockito.mockStatic -import org.mockito.kotlin.any -import org.mockito.kotlin.anyOrNull -import org.mockito.kotlin.doThrow -import org.mockito.kotlin.eq -import org.mockito.kotlin.mock -import org.mockito.kotlin.never -import org.mockito.kotlin.verify -import org.mockito.kotlin.whenever -import java.io.ByteArrayOutputStream -import java.io.File -import java.io.IOException -import java.io.OutputStream -import java.util.Date -import java.util.TimeZone - -// Android storage APIs are mocked to exercise both paths on the host JVM. -@SuppressLint("NewApi") -class LogsExporterTest { - @get:Rule - val directory = TemporaryFolder() - - private val resolver: ContentResolver = mock() - private val context: Context = mock { - on { contentResolver }.thenReturn(resolver) - } - private val clock: Clock = mock { - on { tz }.thenReturn(TimeZone.getTimeZone("UTC")) - on { currentDate }.thenReturn(Date(0)) - } - private val repository: LogsRepository = mock() - private val exporter = LogsExporter(context, clock, repository) - private val uri: Uri = mock() - private val entries = listOf(LogEntry(Date(0), Level.ERROR, "test", "Failed to verify public key: 验证失败")) - - @Test - fun doesNotCreateAnEmptyExport() = runTest { - whenever(repository.load(any())).thenAnswer { - it.getArgument(0).invoke(emptyList(), 0) - } - - assertFalse(exporter.export()) - verify(resolver, never()).insert(anyOrNull(), any()) - } - - @Test - fun publishesUtf8LogsInNextcloudDownloadsDirectoryAfterWriting() = runTest { - mockConstruction(ContentValues::class.java).use { values -> - val output = ByteArrayOutputStream() - whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) - whenever(resolver.openOutputStream(uri, "w")).thenReturn(output) - whenever(resolver.update(eq(uri), any(), anyOrNull(), anyOrNull())).thenReturn(1) - - exporter.writeToMediaStore("logs.txt", entries) - - assertEquals(entries.single().toString(clock.tz) + "\n", output.toString(Charsets.UTF_8)) - verify(values.constructed()[0]).put( - MediaStore.MediaColumns.RELATIVE_PATH, - "${Environment.DIRECTORY_DOWNLOADS}/Nextcloud" - ) - verify(values.constructed()[0]).put(MediaStore.MediaColumns.IS_PENDING, 1) - verify(values.constructed()[1]).put(MediaStore.MediaColumns.IS_PENDING, 0) - verify(resolver, never()).delete(any(), anyOrNull(), anyOrNull()) - } - } - - @Test - fun reportsFailureWhenMediaStoreCannotCreateAFile() = runTest { - val result = runCatching { exporter.writeToMediaStore("logs.txt", entries) } - - assertTrue(result.exceptionOrNull() is IOException) - verify(resolver, never()).openOutputStream(any(), any()) - } - - @Test - fun removesPendingFileWhenOutputStreamIsUnavailable() = runTest { - whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) - - val result = runCatching { exporter.writeToMediaStore("logs.txt", entries) } - - assertTrue(result.exceptionOrNull() is IOException) - verify(resolver).delete(uri, null, null) - verify(resolver, never()).update(any(), any(), anyOrNull(), anyOrNull()) - } - - @Test - fun removesIncompleteFileWhenWritingFails() = runTest { - val output: OutputStream = mock() - whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) - whenever(resolver.openOutputStream(uri, "w")).thenReturn(output) - doThrow(IOException("Disk full")).whenever(output).write(any(), any(), any()) - - val result = runCatching { exporter.writeToMediaStore("logs.txt", entries) } - - assertTrue(result.exceptionOrNull() is IOException) - verify(resolver).delete(uri, null, null) - verify(resolver, never()).update(any(), any(), anyOrNull(), anyOrNull()) - } - - @Test - fun removesPendingFileWhenPublishingFails() = runTest { - whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) - whenever(resolver.openOutputStream(uri, "w")).thenReturn(ByteArrayOutputStream()) - - val result = runCatching { exporter.writeToMediaStore("logs.txt", entries) } - - assertTrue(result.exceptionOrNull() is IOException) - verify(resolver).delete(uri, null, null) - } - - @Test - fun legacyExportsUseNextcloudDirectoryWithoutOverwritingExistingLogs() = runTest { - mockStatic(Environment::class.java).use { environment -> - environment.`when` { - Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) - }.thenReturn(directory.root) - - exporter.writeToPublicDirectory("logs.txt", entries) - exporter.writeToPublicDirectory("logs.txt", entries) - - val files = File(directory.root, "Nextcloud").listFiles().orEmpty() - assertEquals(2, files.size) - files.forEach { assertEquals(entries.single().toString(clock.tz) + "\n", it.readText(Charsets.UTF_8)) } - } - } - - @Test - fun legacyExportReportsFailureWhenDestinationCannotBeCreated() = runTest { - mockStatic(Environment::class.java).use { environment -> - environment.`when` { - Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) - }.thenReturn(directory.root) - val existing = directory.newFile("Nextcloud").apply { writeText("existing data") } - - val result = runCatching { exporter.writeToPublicDirectory("logs.txt", entries) } - - assertTrue(result.exceptionOrNull() is IOException) - assertEquals("existing data", existing.readText()) - } - } -} From 3fff855635c68753f6e94d226905dbd0885ba218 Mon Sep 17 00:00:00 2001 From: Li Wei Date: Tue, 10 Nov 2026 14:46:15 +0800 Subject: [PATCH 3/5] refactor(logging): keep only settings log export Remove the debug logging switch, preferences, filtering and related tests. Preserve upstream logging behavior and reuse the existing export title. Assisted-by: Codex:GPT-6 Signed-off-by: Li Wei --- .../java/com/nextcloud/client/di/AppModule.kt | 5 +- .../com/nextcloud/client/logger/LoggerImpl.kt | 6 +- .../client/preferences/AppPreferences.java | 3 - .../preferences/AppPreferencesImpl.java | 11 --- .../android/ui/activity/SettingsActivity.java | 11 --- app/src/main/res/values/strings.xml | 3 - app/src/main/res/xml/preferences.xml | 7 +- .../client/logger/DebugLoggingTest.kt | 83 ------------------- .../com/nextcloud/client/logger/LoggerTest.kt | 2 +- .../DebugLoggingPreferencesTest.kt | 40 --------- 10 files changed, 5 insertions(+), 166 deletions(-) delete mode 100644 app/src/test/java/com/nextcloud/client/logger/DebugLoggingTest.kt delete mode 100644 app/src/test/java/com/nextcloud/client/preferences/DebugLoggingPreferencesTest.kt diff --git a/app/src/main/java/com/nextcloud/client/di/AppModule.kt b/app/src/main/java/com/nextcloud/client/di/AppModule.kt index a9ceb7a3a8a3..7dad415d7bc1 100644 --- a/app/src/main/java/com/nextcloud/client/di/AppModule.kt +++ b/app/src/main/java/com/nextcloud/client/di/AppModule.kt @@ -145,11 +145,10 @@ internal class AppModule { @Provides @Singleton - fun logger(context: Context, clock: Clock, preferences: AppPreferences): Logger { + fun logger(context: Context, clock: Clock): Logger { val logDir = File(context.filesDir, LOG_DIR_NAME) val handler = FileLogHandler(logDir, LOG_FILE_NAME, LOG_FILE_MAX_SIZE_BYTES) - return LoggerImpl(clock, handler, Handler(), LOGGER_QUEUE_CAPACITY, preferences::isDebugLoggingEnabled) - .apply { start() } + return LoggerImpl(clock, handler, Handler(), LOGGER_QUEUE_CAPACITY).apply { start() } } @Provides diff --git a/app/src/main/java/com/nextcloud/client/logger/LoggerImpl.kt b/app/src/main/java/com/nextcloud/client/logger/LoggerImpl.kt index 09e12cb8eab0..b4ab48bd0bf1 100644 --- a/app/src/main/java/com/nextcloud/client/logger/LoggerImpl.kt +++ b/app/src/main/java/com/nextcloud/client/logger/LoggerImpl.kt @@ -20,8 +20,7 @@ internal class LoggerImpl( private val clock: Clock, private val handler: FileLogHandler, private val mainThreadHandler: Handler, - queueCapacity: Int, - private val isDebugLoggingEnabled: () -> Boolean + queueCapacity: Int ) : Logger, LogsRepository { @@ -46,19 +45,16 @@ internal class LoggerImpl( } override fun v(tag: String, message: String) { - if (!isDebugLoggingEnabled()) return Log.v(tag, message) enqueue(Level.VERBOSE, tag, message) } override fun d(tag: String, message: String) { - if (!isDebugLoggingEnabled()) return Log.d(tag, message) enqueue(Level.DEBUG, tag, message) } override fun d(tag: String, message: String, t: Throwable) { - if (!isDebugLoggingEnabled()) return Log.d(tag, message, t) enqueue(Level.DEBUG, tag, message) } diff --git a/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java b/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java index 38452dc7fc27..4f4e85599480 100644 --- a/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java +++ b/app/src/main/java/com/nextcloud/client/preferences/AppPreferences.java @@ -62,9 +62,6 @@ default void onDarkThemeModeChanged(DarkMode mode) { boolean instantVideoUploadEnabled(); boolean isDarkModeEnabled(); - boolean isDebugLoggingEnabled(); - void setDebugLoggingEnabled(boolean enabled); - boolean isShowHiddenFilesEnabled(); void setShowHiddenFilesEnabled(boolean enabled); diff --git a/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java b/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java index 4c741c21941d..07d50e6f48c1 100644 --- a/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java +++ b/app/src/main/java/com/nextcloud/client/preferences/AppPreferencesImpl.java @@ -116,7 +116,6 @@ public final class AppPreferencesImpl implements AppPreferences { private static final String LOG_ENTRY = "log_entry"; - private static final String PREF__DEBUG_LOGGING = "record_debug_logs"; private final Context context; private final SharedPreferences preferences; @@ -227,16 +226,6 @@ public boolean instantVideoUploadEnabled() { return preferences.getBoolean(PREF__INSTANT_VIDEO_UPLOADING, false); } - @Override - public boolean isDebugLoggingEnabled() { - return preferences.getBoolean(PREF__DEBUG_LOGGING, false); - } - - @Override - public void setDebugLoggingEnabled(boolean enabled) { - preferences.edit().putBoolean(PREF__DEBUG_LOGGING, enabled).apply(); - } - @Override public boolean isShowHiddenFilesEnabled() { return preferences.getBoolean(PREF__SHOW_HIDDEN_FILES, false); diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java index 6028f78cc6c9..28430654a2e1 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java @@ -427,7 +427,6 @@ private void setupImprintPreference(PreferenceCategory preferenceCategoryMore) { private void setupLoggingPreference(PreferenceCategory preferenceCategoryMore) { Preference pLogger = findPreference("logger"); Preference pExport = findPreference("export_logs"); - ThemeableSwitchPreference pDebug = (ThemeableSwitchPreference) findPreference("record_debug_logs"); if (!MDMConfig.INSTANCE.isLogEnabled(this)) { if (pLogger != null) { preferenceCategoryMore.removePreference(pLogger); @@ -435,9 +434,6 @@ private void setupLoggingPreference(PreferenceCategory preferenceCategoryMore) { if (pExport != null) { preferenceCategoryMore.removePreference(pExport); } - if (pDebug != null) { - preferenceCategoryMore.removePreference(pDebug); - } return; } @@ -449,13 +445,6 @@ private void setupLoggingPreference(PreferenceCategory preferenceCategoryMore) { return true; }); } - if (pDebug != null) { - pDebug.setChecked(preferences.isDebugLoggingEnabled()); - pDebug.setOnPreferenceChangeListener((preference, enabled) -> { - preferences.setDebugLoggingEnabled((Boolean) enabled); - return true; - }); - } if (pExport != null) { pExport.setOnPreferenceClickListener(preference -> { exportLogs(); diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index a343b2ce356a..1862de191134 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -686,9 +686,6 @@ Secure connection redirected through an unsecured route. Logs - Record debug logs - Include debug and verbose messages in future logs - Export logs Save app logs to Download/Nextcloud Logs saved to Download/Nextcloud No logs to export. Reproduce the issue and try again. diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index 8a67dad21559..57c1f1307296 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -129,13 +129,8 @@ - Boolean): LoggerImpl { - val handler: Handler = mock() - whenever(handler.post(any())).thenAnswer { - it.getArgument(0).run() - true - } - val logHandler = FileLogHandler(directory.newFolder(), "log.txt", LOG_SIZE) - return LoggerImpl(ClockImpl(), logHandler, handler, QUEUE_CAPACITY, isDebugEnabled).apply { start() } - } - - private fun load(logger: LoggerImpl): List { - val loaded = CountDownLatch(1) - var entries = emptyList() - logger.load { logs, _ -> - entries = logs - loaded.countDown() - } - assertTrue("Log snapshot was not delivered", loaded.await(TIMEOUT_SECONDS, TimeUnit.SECONDS)) - return entries - } - - companion object { - private const val LOG_SIZE = 4096L - private const val QUEUE_CAPACITY = 100 - private const val TIMEOUT_SECONDS = 3L - } -} diff --git a/app/src/test/java/com/nextcloud/client/logger/LoggerTest.kt b/app/src/test/java/com/nextcloud/client/logger/LoggerTest.kt index 58bd0c90f51d..42114e551ebb 100644 --- a/app/src/test/java/com/nextcloud/client/logger/LoggerTest.kt +++ b/app/src/test/java/com/nextcloud/client/logger/LoggerTest.kt @@ -58,7 +58,7 @@ class LoggerTest { clock = ClockImpl() logHandler = spy(FileLogHandler(tempDir, "log.txt", FILE_SIZE)) osHandler = mock() - logger = LoggerImpl(clock, logHandler, osHandler, QUEUE_CAPACITY) { true } + logger = LoggerImpl(clock, logHandler, osHandler, QUEUE_CAPACITY) } @Test diff --git a/app/src/test/java/com/nextcloud/client/preferences/DebugLoggingPreferencesTest.kt b/app/src/test/java/com/nextcloud/client/preferences/DebugLoggingPreferencesTest.kt deleted file mode 100644 index d5a49da336bf..000000000000 --- a/app/src/test/java/com/nextcloud/client/preferences/DebugLoggingPreferencesTest.kt +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Nextcloud - Android Client - * - * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -package com.nextcloud.client.preferences - -import android.content.SharedPreferences -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Test -import org.mockito.kotlin.any -import org.mockito.kotlin.mock -import org.mockito.kotlin.whenever - -class DebugLoggingPreferencesTest { - @Test - fun defaultsToDisabledAndPersistsBothSwitchStates() { - val values = mutableMapOf() - val sharedPreferences: SharedPreferences = mock() - val editor: SharedPreferences.Editor = mock() - whenever(sharedPreferences.getBoolean(any(), any())).thenAnswer { - values[it.getArgument(0)] ?: it.getArgument(1) - } - whenever(sharedPreferences.edit()).thenReturn(editor) - whenever(editor.putBoolean(any(), any())).thenAnswer { - values[it.getArgument(0)] = it.getArgument(1) - editor - } - - val preferences = AppPreferencesImpl(mock(), sharedPreferences, mock()) - assertFalse(preferences.isDebugLoggingEnabled) - preferences.setDebugLoggingEnabled(true) - assertTrue(AppPreferencesImpl(mock(), sharedPreferences, mock()).isDebugLoggingEnabled) - preferences.setDebugLoggingEnabled(false) - assertFalse(AppPreferencesImpl(mock(), sharedPreferences, mock()).isDebugLoggingEnabled) - } -} From 578759088f0537eb4bcfbd4f6e396c3dff820ae0 Mon Sep 17 00:00:00 2001 From: Li Wei Date: Tue, 10 Nov 2026 14:56:31 +0800 Subject: [PATCH 4/5] refactor(logging): call the existing exporter from settings Keep the settings shortcut and legacy storage permission handling. Reuse the original export implementation with an optional Downloads subdirectory, removing the extra export behavior and its tests. Assisted-by: Codex:GPT-6 Signed-off-by: Li Wei --- .../client/logger/ui/LogsEmailSender.kt | 80 +++---- .../android/ui/activity/SettingsActivity.java | 73 +++---- .../owncloud/android/utils/FileExportUtils.kt | 155 +++++++------ app/src/main/res/values/strings.xml | 5 - app/src/main/res/xml/preferences.xml | 1 - .../nextcloud/client/logger/LogsExportTest.kt | 204 ------------------ 6 files changed, 139 insertions(+), 379 deletions(-) delete mode 100644 app/src/test/java/com/nextcloud/client/logger/LogsExportTest.kt diff --git a/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt b/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt index 658af28280a4..32c851c411fe 100644 --- a/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt +++ b/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt @@ -24,13 +24,11 @@ import com.nextcloud.client.core.Cancellable import com.nextcloud.client.core.Clock import com.nextcloud.client.logger.LogEntry import com.owncloud.android.R -import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.ui.notifications.NotificationUtils import com.owncloud.android.utils.FileExportUtils import java.io.File import java.security.SecureRandom import java.util.TimeZone -import java.util.function.IntConsumer import javax.inject.Inject class LogsEmailSender @Inject constructor( @@ -43,10 +41,14 @@ class LogsEmailSender @Inject constructor( const val LOGS_MIME_TYPE = "text/plain" } - private class Task(private val logs: List, private val file: File, private val tz: TimeZone) : - Function0 { + private class Task( + private val context: Context, + private val logs: List, + private val file: File, + private val tz: TimeZone + ) : Function0 { - override fun invoke(): File { + override fun invoke(): Uri? { file.parentFile?.mkdirs() file.outputStream().use { outputStream -> @@ -58,7 +60,7 @@ class LogsEmailSender @Inject constructor( } } - return file + return FileProvider.getUriForFile(context, context.getString(R.string.file_provider_authority), file) } } @@ -67,58 +69,21 @@ class LogsEmailSender @Inject constructor( fun send(logs: List) { if (task == null) { val outFile = File(context.cacheDir, "attachments/logs.txt") - task = runner.postQuickTask(Task(logs, outFile, clock.tz), onResult = { + task = runner.postQuickTask(Task(context, logs, outFile, clock.tz), onResult = { task = null - send(FileProvider.getUriForFile(context, context.getString(R.string.file_provider_authority), it)) + send(it) }) } } - @JvmOverloads - fun export( - logs: List, - onComplete: IntConsumer = IntConsumer { message -> - if (message == R.string.logs_export_to_downloads_success) { - showSuccessNotification(1) - } else { - Toast.makeText(context, message, Toast.LENGTH_LONG).show() - } - } - ) { - if (task != null) return - if (logs.isEmpty()) { - onComplete.accept(R.string.logs_export_to_downloads_empty) - return - } - task = runner.postQuickTask( - task = { - val outFile = File.createTempFile("logs-", ".txt", context.cacheDir) - try { - Task(logs, outFile, clock.tz).invoke() - FileExportUtils().exportFile( - "Nextcloud Android Files Logs.txt", - LOGS_MIME_TYPE, - context.contentResolver, - null, - outFile, - "Nextcloud" - ) - } finally { - outFile.delete() - } - }, - onResult = { success -> - task = null - onComplete.accept( - if (success) R.string.logs_export_to_downloads_success else R.string.logs_export_to_downloads_failed - ) - }, - onError = { + fun export(logs: List) { + if (task == null) { + val outFile = File(context.cacheDir, "attachments/logs.txt") + task = runner.postQuickTask(Task(context, logs, outFile, clock.tz), onResult = { task = null - Log_OC.e(this, "Could not export logs", it) - onComplete.accept(R.string.logs_export_to_downloads_failed) - } - ) + export(outFile) + }) + } } fun stop() { @@ -128,6 +93,17 @@ class LogsEmailSender @Inject constructor( } } + private fun export(file: File) { + FileExportUtils("Nextcloud").exportFile( + "Nextcloud Android Files Logs", + "text/plain", + context.contentResolver, + null, + file + ) + showSuccessNotification(1) + } + fun showSuccessNotification(successfulExports: Int) { showNotification( context.resources.getQuantityString( diff --git a/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java b/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java index 28430654a2e1..c2740ac0702b 100644 --- a/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java +++ b/app/src/main/java/com/owncloud/android/ui/activity/SettingsActivity.java @@ -43,7 +43,6 @@ import android.view.View; import android.view.ViewGroup; import android.webkit.URLUtil; -import android.widget.Toast; import com.google.android.material.dialog.MaterialAlertDialogBuilder; import com.nextcloud.client.account.User; @@ -425,54 +424,39 @@ private void setupImprintPreference(PreferenceCategory preferenceCategoryMore) { } private void setupLoggingPreference(PreferenceCategory preferenceCategoryMore) { - Preference pLogger = findPreference("logger"); Preference pExport = findPreference("export_logs"); - if (!MDMConfig.INSTANCE.isLogEnabled(this)) { - if (pLogger != null) { - preferenceCategoryMore.removePreference(pLogger); - } - if (pExport != null) { - preferenceCategoryMore.removePreference(pExport); - } - return; - } - - if (pLogger != null) { - pLogger.setOnPreferenceClickListener(preference -> { - Intent loggerIntent = new Intent(getApplicationContext(), LogsActivity.class); - startActivity(loggerIntent); - - return true; - }); - } - if (pExport != null) { + if (MDMConfig.INSTANCE.isLogEnabled(this)) { pExport.setOnPreferenceClickListener(preference -> { exportLogs(); return true; }); + } else { + preferenceCategoryMore.removePreference(pExport); + } + Preference pLogger = findPreference("logger"); + if (pLogger != null) { + if (MDMConfig.INSTANCE.isLogEnabled(this)) { + pLogger.setOnPreferenceClickListener(preference -> { + Intent loggerIntent = new Intent(getApplicationContext(), LogsActivity.class); + startActivity(loggerIntent); + + return true; + }); + } else { + preferenceCategoryMore.removePreference(pLogger); + } } } private void exportLogs() { - Preference preference = findPreference("export_logs"); - if (preference == null || !preference.isEnabled() || !MDMConfig.INSTANCE.isLogEnabled(this)) { - return; - } if (Build.VERSION.SDK_INT < Build.VERSION_CODES.Q - && ContextCompat.checkSelfPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE) - != PackageManager.PERMISSION_GRANTED) { + && !PermissionUtil.checkSelfPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE)) { requestPermissions(new String[]{Manifest.permission.WRITE_EXTERNAL_STORAGE}, REQUEST_EXPORT_LOGS_PERMISSION); return; } - preference.setEnabled(false); logsRepository.load((entries, size) -> { if (!isDestroyed()) { - logsEmailSender.export(entries, message -> { - if (!isDestroyed()) { - preference.setEnabled(true); - Toast.makeText(this, message, Toast.LENGTH_LONG).show(); - } - }); + logsEmailSender.export(entries); } return Unit.INSTANCE; }); @@ -1284,18 +1268,6 @@ protected void onPostCreate(Bundle savedInstanceState) { getDelegate().onPostCreate(savedInstanceState); } - @Override - public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { - super.onRequestPermissionsResult(requestCode, permissions, grantResults); - if (requestCode == REQUEST_EXPORT_LOGS_PERMISSION) { - if (grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) { - exportLogs(); - } else { - Toast.makeText(this, R.string.logs_export_storage_permission_required, Toast.LENGTH_LONG).show(); - } - } - } - @Override protected void onDestroy() { logsEmailSender.stop(); @@ -1303,6 +1275,15 @@ protected void onDestroy() { getDelegate().onDestroy(); } + @Override + public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) { + super.onRequestPermissionsResult(requestCode, permissions, grantResults); + if (requestCode == REQUEST_EXPORT_LOGS_PERMISSION + && grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) { + exportLogs(); + } + } + @Override protected void onStop() { super.onStop(); diff --git a/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt b/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt index 638a33ff293d..34e66b19ff18 100644 --- a/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt +++ b/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt @@ -7,9 +7,9 @@ */ package com.owncloud.android.utils +import android.annotation.SuppressLint import android.content.ContentResolver import android.content.ContentValues -import android.net.Uri import android.os.Build import android.os.Environment import android.provider.MediaStore @@ -18,126 +18,138 @@ import com.owncloud.android.datamodel.OCFile import com.owncloud.android.lib.common.utils.Log_OC import java.io.File import java.io.FileInputStream +import java.io.FileNotFoundException import java.io.FileOutputStream import java.io.IOException -import java.io.OutputStream +import java.io.InputStream -class FileExportUtils { +class FileExportUtils(private val subdirectory: String = "") { @Throws(IllegalStateException::class) - fun exportFile( - fileName: String, - mimeType: String, - contentResolver: ContentResolver, - ocFile: OCFile?, - file: File?, - subdirectory: String = "" - ): Boolean = try { + fun exportFile(fileName: String, mimeType: String, contentResolver: ContentResolver, ocFile: OCFile?, file: File?) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { exportFileAndroid10AndAbove( fileName, mimeType, contentResolver, ocFile, - file, - subdirectory + file ) } else { exportFilesBelowAndroid10( fileName, contentResolver, ocFile, - file, - subdirectory + file ) } - true - } catch (e: IOException) { - Log_OC.e(this, "Cannot export file", e) - false - } catch (e: SecurityException) { - Log_OC.e(this, "Cannot access export destination", e) - false } + @SuppressLint("Recycle") // handled inside copy method @RequiresApi(Build.VERSION_CODES.Q) - internal fun exportFileAndroid10AndAbove( + private fun exportFileAndroid10AndAbove( fileName: String, mimeType: String, contentResolver: ContentResolver, ocFile: OCFile?, - file: File?, - subdirectory: String + file: File? ) { val cv = ContentValues().apply { put(MediaStore.MediaColumns.DISPLAY_NAME, fileName) put(MediaStore.MediaColumns.MIME_TYPE, mimeType) put(MediaStore.MediaColumns.RELATIVE_PATH, "${Environment.DIRECTORY_DOWNLOADS}/$subdirectory") - put(MediaStore.MediaColumns.IS_PENDING, 1) } - val uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) - ?: throw IOException("Could not create export") - try { - writeMediaStoreExport(ocFile, file, contentResolver, uri) - } catch (e: Exception) { - runCatching { contentResolver.delete(uri, null, null) }.onFailure { - Log_OC.e(this, "Could not remove incomplete export", it) - } - throw e - } - } + var uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) - @RequiresApi(Build.VERSION_CODES.Q) - private fun writeMediaStoreExport(ocFile: OCFile?, file: File?, contentResolver: ContentResolver, uri: Uri) { - val stream = contentResolver.openOutputStream(uri, "w") ?: throw IOException("Could not open export") - copy(ocFile, file, contentResolver, stream) - val published = ContentValues().apply { put(MediaStore.MediaColumns.IS_PENDING, 0) } - if (contentResolver.update(uri, published, null, null) != 1) { - throw IOException("Could not publish export") + if (uri == null) { + var count = INITIAL_RENAME_COUNT + do { + val name = generateNewName(fileName, count) + cv.put(MediaStore.MediaColumns.DISPLAY_NAME, name) + uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) + + count++ + } while (uri == null) } + + copy( + ocFile, + file, + contentResolver, + FileOutputStream(contentResolver.openFileDescriptor(uri, "w")?.fileDescriptor) + ) } - internal fun exportFilesBelowAndroid10( + private fun exportFilesBelowAndroid10( fileName: String, contentResolver: ContentResolver, ocFile: OCFile?, - file: File?, - subdirectory: String + file: File? ) { - val directory = File( - Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), - subdirectory - ) - if (!directory.isDirectory && !directory.mkdirs()) { - throw IOException("Could not create export directory") - } - var target = File(directory, fileName) - var count = INITIAL_RENAME_COUNT - while (!target.createNewFile()) { - target = File(directory, generateNewName(fileName, count++)) - } try { - copy(ocFile, file, contentResolver, FileOutputStream(target)) - } catch (e: Exception) { - target.delete() - throw e + val directory = File( + Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), + subdirectory + ) + directory.mkdirs() + var target = File( + directory, + fileName + ) + + if (target.exists()) { + var count = INITIAL_RENAME_COUNT + do { + val name = generateNewName(fileName, count) + target = File( + directory, + name + ) + + count++ + } while (target.exists()) + } + + copy( + ocFile, + file, + contentResolver, + FileOutputStream(target) + ) + } catch (e: FileNotFoundException) { + Log_OC.e(this, "File not found", e) + } catch (e: IOException) { + Log_OC.e(this, "Cannot write file", e) } } @Throws(IllegalStateException::class) - private fun copy(ocFile: OCFile?, file: File?, contentResolver: ContentResolver, outputStream: OutputStream) { + private fun copy(ocFile: OCFile?, file: File?, contentResolver: ContentResolver, outputStream: FileOutputStream) { outputStream.use { fos -> - val inputStream = when { - ocFile != null -> contentResolver.openInputStream(ocFile.storageUri) - file != null -> FileInputStream(file) - else -> error("ocFile and file both may not be null") - } ?: throw IOException("Could not open export source") - inputStream.use { fis -> - fis.copyTo(fos) + try { + val inputStream = when { + ocFile != null -> contentResolver.openInputStream(ocFile.storageUri) + file != null -> FileInputStream(file) + else -> error("ocFile and file both may not be null") + }!! + + inputStream.use { fis -> + copyStream(fis, fos) + } + } catch (e: IOException) { + Log_OC.e(this, "Cannot write file", e) } } } + private fun copyStream(inputStream: InputStream, outputStream: FileOutputStream) { + val buffer = ByteArray(COPY_BUFFER_SIZE) + var len: Int + while (inputStream.read(buffer).also { len = it } != -1) { + outputStream.write(buffer, 0, len) + } + } + private fun generateNewName(name: String, count: Int): String { val extPos = name.lastIndexOf('.') val suffix = " ($count)" @@ -154,5 +166,6 @@ class FileExportUtils { companion object { private const val INITIAL_RENAME_COUNT = 2 + private const val COPY_BUFFER_SIZE = 1024 } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 1862de191134..f3d6f60bf3e1 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -686,11 +686,6 @@ Secure connection redirected through an unsecured route. Logs - Save app logs to Download/Nextcloud - Logs saved to Download/Nextcloud - No logs to export. Reproduce the issue and try again. - Could not save logs. Check available storage and try again. - Storage permission is needed to save logs to Download/Nextcloud. Refresh Send logs by email Export logs diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index 57c1f1307296..cc3b4a5745b2 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -131,7 +131,6 @@ android:key="logger" /> - val output = ByteArrayOutputStream() - whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) - whenever(resolver.openOutputStream(uri, "w")).thenReturn(output) - whenever(resolver.update(eq(uri), any(), anyOrNull(), anyOrNull())).thenReturn(1) - - exportToMediaStore() - - assertEquals(content, output.toString(Charsets.UTF_8)) - verify(values.constructed()[0]).put( - MediaStore.MediaColumns.RELATIVE_PATH, - "${Environment.DIRECTORY_DOWNLOADS}/Nextcloud" - ) - verify(values.constructed()[0]).put(MediaStore.MediaColumns.IS_PENDING, 1) - verify(values.constructed()[1]).put(MediaStore.MediaColumns.IS_PENDING, 0) - verify(resolver, never()).delete(any(), anyOrNull(), anyOrNull()) - } - } - - @Test - fun reportsFailureWhenMediaStoreCannotCreateAFile() { - assertTrue(runCatching { exportToMediaStore() }.exceptionOrNull() is IOException) - verify(resolver, never()).openOutputStream(any(), any()) - } - - @Test - fun removesPendingFileWhenOutputStreamIsUnavailable() { - whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) - - assertTrue(runCatching { exportToMediaStore() }.exceptionOrNull() is IOException) - verify(resolver).delete(uri, null, null) - verify(resolver, never()).update(any(), any(), anyOrNull(), anyOrNull()) - } - - @Test - fun removesIncompleteFileWhenWritingFails() { - val output: OutputStream = mock() - whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) - whenever(resolver.openOutputStream(uri, "w")).thenReturn(output) - doThrow(IOException("Disk full")).whenever(output).write(any(), any(), any()) - - assertTrue(runCatching { exportToMediaStore() }.exceptionOrNull() is IOException) - verify(resolver).delete(uri, null, null) - verify(resolver, never()).update(any(), any(), anyOrNull(), anyOrNull()) - verify(output).close() - } - - @Test - fun removesPendingFileWhenPublishingFails() { - whenever(resolver.insert(anyOrNull(), any())).thenReturn(uri) - whenever(resolver.openOutputStream(uri, "w")).thenReturn(ByteArrayOutputStream()) - - assertTrue(runCatching { exportToMediaStore() }.exceptionOrNull() is IOException) - verify(resolver).delete(uri, null, null) - } - - @Test - fun settingsAndLogViewerShareBackgroundExportWithoutOverwritingExistingLogs() = withDownloads { - sender.export(entries, onComplete) - sender.export(entries, onComplete) - assertEquals(1, runner.size) - assertFalse(File(directory.root, "Nextcloud").exists()) - runner.runAll() - sender.export(entries, onComplete) - runner.runAll() - - val files = File(directory.root, "Nextcloud").listFiles().orEmpty() - assertEquals(2, files.size) - files.forEach { assertEquals(content, it.readText(Charsets.UTF_8)) } - verify(onComplete, times(2)).accept(R.string.logs_export_to_downloads_success) - assertTrue(context.cacheDir.listFiles().isNullOrEmpty()) - } - - @Test - fun reportsFailureWithoutOverwritingAnExistingDestination() = withDownloads { - val existing = directory.newFile("Nextcloud").apply { writeText("existing data") } - - sender.export(entries, onComplete) - runner.runAll() - - verify(onComplete).accept(R.string.logs_export_to_downloads_failed) - verify(onComplete, never()).accept(R.string.logs_export_to_downloads_success) - assertEquals("existing data", existing.readText()) - } - - @Test - fun reportsSerializationFailureAndAllowsRetry() { - directory.newFile("cache") - sender.export(entries, onComplete) - runner.runAll() - verify(onComplete).accept(R.string.logs_export_to_downloads_failed) - - sender.export(entries, onComplete) - assertEquals(1, runner.size) - } - - @Test - fun stoppingCancelsQueuedExport() { - sender.export(entries, onComplete) - sender.stop() - - assertTrue(runner.isEmpty) - verify(onComplete, never()).accept(any()) - } - - @Test - fun otherExportsKeepTheDownloadsRootAndCleanUpFailedWrites() = withDownloads { - assertTrue(exporter.exportFile("other.txt", "text/plain", resolver, null, source)) - assertEquals(content, File(directory.root, "other.txt").readText(Charsets.UTF_8)) - assertFalse(exporter.exportFile("missing.txt", "text/plain", resolver, null, File(directory.root, "no-source"))) - assertFalse(File(directory.root, "missing.txt").exists()) - } - - private fun exportToMediaStore() = - exporter.exportFileAndroid10AndAbove("logs.txt", "text/plain", resolver, null, source, "Nextcloud") - - private fun withDownloads(block: () -> Unit) { - mockStatic(Environment::class.java).use { environment -> - environment.`when` { - Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) - }.thenReturn(directory.root) - block() - } - } -} From 4af81f3e5021d82ce8b4b10c6522a13a0a9922d0 Mon Sep 17 00:00:00 2001 From: Li Wei Date: Thu, 12 Nov 2026 11:30:04 +0800 Subject: [PATCH 5/5] feat(logging): show log export result and file size Show the export directory and formatted file size in a toast and expandable notification after copying succeeds. Report export errors through the existing background runner instead of announcing success when writing fails. Assisted-by: Codex:GPT-6 Signed-off-by: Li Wei --- .../client/logger/ui/LogsEmailSender.kt | 45 +++++++---- .../owncloud/android/utils/FileExportUtils.kt | 30 +++----- app/src/main/res/values/strings.xml | 2 + .../client/logger/LogExportFailureTest.kt | 77 +++++++++++++++++++ 4 files changed, 119 insertions(+), 35 deletions(-) create mode 100644 app/src/test/java/com/nextcloud/client/logger/LogExportFailureTest.kt diff --git a/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt b/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt index 32c851c411fe..14dfbe6ff0fe 100644 --- a/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt +++ b/app/src/main/java/com/nextcloud/client/logger/ui/LogsEmailSender.kt @@ -16,6 +16,7 @@ import android.content.Intent.FLAG_ACTIVITY_NEW_TASK import android.content.Intent.FLAG_GRANT_READ_URI_PERMISSION import android.net.Uri import android.os.Build +import android.text.format.Formatter import android.widget.Toast import androidx.core.app.NotificationCompat import androidx.core.content.FileProvider @@ -24,6 +25,7 @@ import com.nextcloud.client.core.Cancellable import com.nextcloud.client.core.Clock import com.nextcloud.client.logger.LogEntry import com.owncloud.android.R +import com.owncloud.android.lib.common.utils.Log_OC import com.owncloud.android.ui.notifications.NotificationUtils import com.owncloud.android.utils.FileExportUtils import java.io.File @@ -79,10 +81,27 @@ class LogsEmailSender @Inject constructor( fun export(logs: List) { if (task == null) { val outFile = File(context.cacheDir, "attachments/logs.txt") - task = runner.postQuickTask(Task(context, logs, outFile, clock.tz), onResult = { - task = null - export(outFile) - }) + task = runner.postQuickTask( + task = { + Task(context, logs, outFile, clock.tz).invoke() + export(outFile) + outFile.length() + }, + onResult = { size -> + task = null + val message = context.getString( + R.string.logs_export_success, + Formatter.formatFileSize(context, size) + ) + showExportResult(message) + }, + onError = { error -> + task = null + Log_OC.e(this, "Could not export logs", error) + val reason = error.localizedMessage ?: context.getString(R.string.common_error_unknown) + showExportResult(context.getString(R.string.logs_export_failed, reason)) + } + ) } } @@ -101,20 +120,10 @@ class LogsEmailSender @Inject constructor( null, file ) - showSuccessNotification(1) - } - - fun showSuccessNotification(successfulExports: Int) { - showNotification( - context.resources.getQuantityString( - R.plurals.export_successful, - successfulExports, - successfulExports - ) - ) } - private fun showNotification(message: String) { + private fun showExportResult(message: String) { + Toast.makeText(context, message, Toast.LENGTH_LONG).show() val notificationId = SecureRandom().nextInt() val notificationBuilder = NotificationCompat.Builder( @@ -122,7 +131,9 @@ class LogsEmailSender @Inject constructor( NotificationUtils.NOTIFICATION_CHANNEL_DOWNLOAD ) .setSmallIcon(R.drawable.notification_icon) - .setContentTitle(message) + .setContentTitle(context.getString(R.string.logs_menu_export)) + .setContentText(message) + .setStyle(NotificationCompat.BigTextStyle().bigText(message)) .setAutoCancel(true) val actionIntent = Intent(DownloadManager.ACTION_VIEW_DOWNLOADS).apply { diff --git a/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt b/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt index 34e66b19ff18..a7a0a1ac0dc0 100644 --- a/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt +++ b/app/src/main/java/com/owncloud/android/utils/FileExportUtils.kt @@ -22,9 +22,10 @@ import java.io.FileNotFoundException import java.io.FileOutputStream import java.io.IOException import java.io.InputStream +import java.io.OutputStream class FileExportUtils(private val subdirectory: String = "") { - @Throws(IllegalStateException::class) + @Throws(IllegalStateException::class, IOException::class) fun exportFile(fileName: String, mimeType: String, contentResolver: ContentResolver, ocFile: OCFile?, file: File?) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { exportFileAndroid10AndAbove( @@ -59,24 +60,14 @@ class FileExportUtils(private val subdirectory: String = "") { put(MediaStore.MediaColumns.RELATIVE_PATH, "${Environment.DIRECTORY_DOWNLOADS}/$subdirectory") } - var uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) - - if (uri == null) { - var count = INITIAL_RENAME_COUNT - do { - val name = generateNewName(fileName, count) - cv.put(MediaStore.MediaColumns.DISPLAY_NAME, name) - uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) - - count++ - } while (uri == null) - } + val uri = contentResolver.insert(MediaStore.Downloads.EXTERNAL_CONTENT_URI, cv) + ?: throw IOException("Could not create export in Downloads") copy( ocFile, file, contentResolver, - FileOutputStream(contentResolver.openFileDescriptor(uri, "w")?.fileDescriptor) + contentResolver.openOutputStream(uri, "w") ?: throw IOException("Could not open export in Downloads") ) } @@ -118,31 +109,34 @@ class FileExportUtils(private val subdirectory: String = "") { ) } catch (e: FileNotFoundException) { Log_OC.e(this, "File not found", e) + throw e } catch (e: IOException) { Log_OC.e(this, "Cannot write file", e) + throw e } } - @Throws(IllegalStateException::class) - private fun copy(ocFile: OCFile?, file: File?, contentResolver: ContentResolver, outputStream: FileOutputStream) { + @Throws(IllegalStateException::class, IOException::class) + private fun copy(ocFile: OCFile?, file: File?, contentResolver: ContentResolver, outputStream: OutputStream) { outputStream.use { fos -> try { val inputStream = when { ocFile != null -> contentResolver.openInputStream(ocFile.storageUri) file != null -> FileInputStream(file) else -> error("ocFile and file both may not be null") - }!! + } ?: throw IOException("Could not open export source") inputStream.use { fis -> copyStream(fis, fos) } } catch (e: IOException) { Log_OC.e(this, "Cannot write file", e) + throw e } } } - private fun copyStream(inputStream: InputStream, outputStream: FileOutputStream) { + private fun copyStream(inputStream: InputStream, outputStream: OutputStream) { val buffer = ByteArray(COPY_BUFFER_SIZE) var len: Int while (inputStream.read(buffer).also { len = it } != -1) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index f3d6f60bf3e1..199e31e0cf8e 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -689,6 +689,8 @@ Refresh Send logs by email Export logs + Logs saved to Download/Nextcloud/ (%1$s) + Could not save logs to Download/Nextcloud/: %1$s Delete logs No app for sending logs found. Please install an email client. %1$s Android app logs diff --git a/app/src/test/java/com/nextcloud/client/logger/LogExportFailureTest.kt b/app/src/test/java/com/nextcloud/client/logger/LogExportFailureTest.kt new file mode 100644 index 000000000000..732a0c03ec8a --- /dev/null +++ b/app/src/test/java/com/nextcloud/client/logger/LogExportFailureTest.kt @@ -0,0 +1,77 @@ +/* + * Nextcloud - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +package com.nextcloud.client.logger + +import android.content.ContentResolver +import android.os.Environment +import com.owncloud.android.utils.FileExportUtils +import org.junit.Assert.assertArrayEquals +import org.junit.Assert.assertEquals +import org.junit.Assert.assertThrows +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.mockito.Mockito.mockConstruction +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any +import org.mockito.kotlin.doThrow +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import java.io.File +import java.io.FileOutputStream +import java.io.IOException + +class LogExportFailureTest { + @get:Rule + val directory = TemporaryFolder() + + private val resolver: ContentResolver = mock() + private val exporter = FileExportUtils("Nextcloud") + + @Test + fun exportedBytesAndSizeMatchTheSerializedLog() = withDownloads { + val source = directory.newFile("source.txt").apply { writeText("ERROR: key verification failed 验证失败\n") } + + exporter.exportFile("logs.txt", "text/plain", resolver, null, source) + + val exported = File(directory.root, "Nextcloud/logs.txt") + assertArrayEquals(source.readBytes(), exported.readBytes()) + assertEquals(source.length(), exported.length()) + } + + @Test + fun missingSourceIsReportedAsFailure() = withDownloads { + assertThrows(IOException::class.java) { + exporter.exportFile("logs.txt", "text/plain", resolver, null, File(directory.root, "missing")) + } + } + + @Test + fun diskWriteErrorIsReportedAndOutputIsClosed() = withDownloads { + val source = directory.newFile("source.txt").apply { writeText("ERROR: verification failed\n") } + mockConstruction(FileOutputStream::class.java) { output, _ -> + doThrow(IOException("Disk full")).whenever(output).write(any(), any(), any()) + }.use { outputs -> + val error = assertThrows(IOException::class.java) { + exporter.exportFile("logs.txt", "text/plain", resolver, null, source) + } + assertEquals("Disk full", error.message) + verify(outputs.constructed().single()).close() + } + } + + private fun withDownloads(block: () -> Unit) { + mockStatic(Environment::class.java).use { environment -> + environment.`when` { + Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) + }.thenReturn(directory.root) + block() + } + } +}