Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/automate windows file name compatibility #7850

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mgallien
Copy link
Collaborator

No description provided.

enforce trailing and leading space rules for new files and only for new
files but on all platforms, not only windows

Signed-off-by: Matthieu Gallien <[email protected]>
to ensure compatibility with Widnows, we will remove automatically the
leading space characters in file name of new files or folders

Signed-off-by: Matthieu Gallien <[email protected]>
for now simple rule to guess if the server has windows naming enforced

if windows naming is enforced, we enforce it for new files

if not, we do not care

for now limited to spaces removal

more to come

Signed-off-by: Matthieu Gallien <[email protected]>
@mgallien mgallien added this to the 3.16.0 milestone Feb 13, 2025
Copy link

Artifact containing the AppImage: nextcloud-appimage-pr-7850.zip

SHA256 checksum: fe00eb397955257d2da748975070005bb3f6337b1d9cf74d9b36e6fddcc1d3c4

To test this change/fix you can download the above artifact file, unzip it, and run it.

Please make sure to quit your existing Nextcloud app and backup your data.

Copy link
Member

@nilsding nilsding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good overall, tests are failing though (also on windows)

QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::FileNameInvalid);
#else
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::Success);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did these get removed?
running the tests on e.g. linux these items still end up with a status of Success

2025-02-13T07:43:29.5350432Z FAIL!  : TestSyncVirtualFiles::testCreateFileWithTrailingSpaces_acceptAndRejectInvalidFileName() Compared values are not the same
2025-02-13T07:43:29.5350569Z    Actual   (completeSpy.findItem(fileWithSpaces1)->_status): Success
2025-02-13T07:43:29.5350719Z    Expected (SyncFileItem::Status::FileNameInvalid)         : FileNameInvalid
2025-02-13T07:43:29.5350832Z    Loc: [/__w/desktop/desktop/test/testsyncvirtualfiles.cpp(838)]

Comment on lines +2075 to +2081
QCOMPARE(completeSpy.findItem(fileWithSpaces1)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces2)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces3)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces4)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces5)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(fileWithSpaces6)->_status, SyncFileItem::Status::FileNameInvalid);
QCOMPARE(completeSpy.findItem(extraFileNameWithSpaces)->_status, SyncFileItem::Status::FileNameInvalid);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these end up as Success too on the Linux tests even with the setLocalDiscoveryEnforceWindowsFileNameCompatibility option enabled on the syncEngine -- in the SyncEngine::startSync method this property gets set on the DiscoveryPhase object that is used by the discovery jobs only depending on the account caps...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants