Skip to content

use local id from webdav#10011

Closed
tobiasKaminsky wants to merge 1 commit into
masterfrom
localId
Closed

use local id from webdav#10011
tobiasKaminsky wants to merge 1 commit into
masterfrom
localId

Conversation

@tobiasKaminsky

@tobiasKaminsky tobiasKaminsky commented Mar 23, 2022

Copy link
Copy Markdown
Member

Fixes: #8566
Library: nextcloud/android-library#843

Signed-off-by: tobiasKaminsky tobias@kaminsky.me

  • Tests written, or not not needed

@github-actions

github-actions Bot commented Mar 23, 2022

Copy link
Copy Markdown

blue-Light-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/10011-Screenshot-blue-Light-06-47

@nextcloud-android-bot

nextcloud-android-bot commented Mar 23, 2022

Copy link
Copy Markdown
Collaborator

stable-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/4379-IT-stable-06-54

@tobiasKaminsky @AlvaroBrey
use local id from webdav
2492e15 
Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
AlvaroBrey
AlvaroBrey approved these changes Mar 23, 2022
View reviewed changes
Comment thread app/src/main/java/com/owncloud/android/datamodel/OCFile.java
}

@VisibleForTesting
public String getLocalIdDirect() {

@AlvaroBrey AlvaroBrey Mar 23, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

getInternalLocalId, or something like that, may be a better name.

Comment thread app/src/main/java/com/owncloud/android/providers/FileContentProvider.java
Comment on lines +2476 to +2478
if (!upgraded) {
Log_OC.i(SQL, String.format(Locale.ENGLISH, UPGRADE_VERSION_MSG, oldVersion, newVersion));
}

@AlvaroBrey AlvaroBrey Mar 23, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's the point of adding this between each upgrade if? It will print the same every time. Also, some upgrades set upgraded=true, but others don't. Looks like this needs cleanup, I will do it after this PR is merged

@github-actions

github-actions Bot commented Mar 23, 2022

Copy link
Copy Markdown

APK file: https://www.kaminsky.me/nc-dev/android-artifacts/10011.apk

qrcode

To test this change/fix you can simply download above APK file and install and test it in parallel to your existing Nextcloud app.

@nextcloud-android-bot

nextcloud-android-bot commented Mar 23, 2022

Copy link
Copy Markdown
Collaborator

Codacy

Lint

TypemasterPR
Warnings9292
Errors00

SpotBugs (new)

Warning Type Number
Bad practice Warnings 28
Correctness Warnings 78
Experimental Warnings 1
Internationalization Warnings 9
Malicious code vulnerability Warnings 57
Multithreaded correctness Warnings 9
Performance Warnings 67
Security Warnings 29
Dodgy code Warnings 350
Total 628

SpotBugs (master)

Warning Type Number
Bad practice Warnings 28
Correctness Warnings 78
Experimental Warnings 1
Internationalization Warnings 9
Malicious code vulnerability Warnings 57
Multithreaded correctness Warnings 9
Performance Warnings 67
Security Warnings 29
Dodgy code Warnings 350
Total 628

Android-library is not set to master branch in build.gradle

@nextcloud-android-bot

nextcloud-android-bot commented Mar 23, 2022

Copy link
Copy Markdown
Collaborator

master-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/4392-IT-master-10-16

@github-actions

github-actions Bot commented Mar 23, 2022

Copy link
Copy Markdown

blue-Light-Screenshot test failed, but no output was generated. Maybe a preliminary stage failed.

@github-actions

github-actions Bot commented Mar 23, 2022

Copy link
Copy Markdown

blue-Light-IT test failed: https://www.kaminsky.me/nc-dev/android-integrationTests/10011-Screenshot-blue-Light-10-20

@AlvaroBrey

AlvaroBrey commented Mar 23, 2022

Copy link
Copy Markdown
Member

Similar trace in all failing tests (including screenshot)

java.lang.StringIndexOutOfBoundsException: length=3; index=8
at java.lang.String.substring(String.java:1971)
at com.owncloud.android.datamodel.OCFile.getLocalId(OCFile.java:560)
at com.owncloud.android.datamodel.FileDataStorageManager.saveFile(FileDataStorageManager.java:238)
at com.owncloud.android.datamodel.FileDataStorageManager.saveNewFile(FileDataStorageManager.java:348)
at com.owncloud.android.datamodel.FileDataStorageManagerIT.testSaveNewFile(FileDataStorageManagerIT.java:349)

AlvaroBrey
AlvaroBrey requested changes Mar 30, 2022
View reviewed changes

@AlvaroBrey AlvaroBrey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

failing tests, see previous comment

@joshtrichards

joshtrichards commented Jan 30, 2025

Copy link
Copy Markdown
Member

Superceded by #8643

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

Reviewers

@AlvaroBrey AlvaroBrey AlvaroBrey requested changes

Assignees

No one assigned

Labels

2. developing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use fileId instead of relying on remoteID

4 participants

@tobiasKaminsky @nextcloud-android-bot @AlvaroBrey @joshtrichards