Auto-upload: with "delete from camera roll" active, photos that are already present on the server are not deleted from the camera roll

オープン
  • 何か問題がありますか？
#4,335 コメント 2 件 リアクション 1 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

  • #4395 @mvanhorn による — マージされずにクローズ

評価

難易度
3/5
見積もり時間
1〜2日
初心者へのやさしさ
72/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
swift
領域
mobile

調査の方向性

Start in NCAutoUpload.swift around autoUploadBackgroundSync at line 376, then trace removeUploadedAssetsIfNeeded and the handling of a successful PROPFIND. Reproduce with existing server photos and remove_photo_CameraRoll enabled; done means those already-present assets are removed from the camera roll without changing the handling of upload errors.

索引モデルが issue の本文から書いたものです。

説明

bug
How to use GitHub
  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.
Steps to reproduce
  1. reinstall app with pictures (previously uploaded to the server) still present on the camera roll (for whatever reason, see #4307, #4306 for instance)
  2. setup app for automatic uploading, "new_photos_only" not set, and "remove_photo_CameraRoll" set
  3. maintain app in foreground with a good connection (test setup: iPhone and server on the same LAN)
  4. wait for upload termination
Expected behaviour

I believe these files should be deleted from the camera roll (at least that's the exact reason I reinstalled the app)

Actual behaviour

Out of more than 600 files, only 58 got deleted (including videos that were not uploaded because of #4304 ).

Analysis

When a photo is found in the camera roll, autoUploadBackgroundSync (NCAutoUpload.swift:376) will PROPFIND the server, with 3 outcomes

  • .errorCode == 404: extract and upload file
  • other errors (503, 401, 423, ...): do nothing, we'll see on next pass
  • .success: file exists, deletes the local asset row -> this is where the decision is made to not delete the file

Note: the existence check is only based on the name of the file. In my case, with default _filenamemask_ and _autoupload_create_subfolder_ set, there is a very thin chance this could cause collisions, so it's fine. Perhaps other users may encounter issues -- and perhaps changing this is way too costly..

A remark: a few more logs could help better diagnose the auto-upload system, such as upload successes, photos deletion requests (removeUploadedAssetsIfNeeded), logging getCameraRollAssets filtered vs unfiltered counts, etc.

            let unfilteredCount = collections.reduce(0) { $0 + PHAsset.fetchAssets(in: $1, options: nil).count }
            nkLog(tag: self.global.logTagBgSync,
                  message: "getCameraRollAssets: collections=\(collections.count), filteredCount=\(allAssets.count), unfilteredLibraryCount=\(unfilteredCount)",
                  minimumLogLevel: .verbose)
Logs

issue-existing-photos-not-deleted-1.txt

Reasoning or why should it be changed/implemented?

Like I said, IMHO the app should not make the difference between a file that is already there and one that has just been uploaded, so the setting "remove_photo_CameraRoll" should apply to both. Plus, with current auto-upload issues, that would have given me an workaround to clean my camera roll ;). Right now, though, I can safely clear my camera roll

Environment data

iOS version: iOS 18.7

Nextcloud iOS app version: 35.0.0.8

Server operating system: Nextcloud AIO 14.1.1

Web server: Nextcloud AIO 14.1.1

Database: Nextcloud AIO 14.1.1

PHP version: Nextcloud AIO 14.1.1

Nextcloud version: 34.0.4

主要言語
Swift
スター
2.5k
フォーク
1k
平均マージ
1日 18時間
マージ済み PR（30日）
25

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

nextcloud/ios のほかの issue

nextcloud/ios の issue をすべて見る

似ている issue

Swift の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。