Skip to content

Feature uploadsubfolders #35

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.

Sign up for GitHub

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

Merged
AndyScherzinger merged 11 commits into from
Jun 27, 2016
Merged

Feature uploadsubfolders #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f68b53b
introduce upload subfolders configuration variable
Spacefish Jun 19, 2016
bd8ab10
make InstantUploader honor the SubFolder setting
Spacefish Jun 19, 2016
89ae5bf
use long for timestamp and don´t fail if timestamp is empty
Spacefish Jun 19, 2016
3c388ad
implement the same for videos and do some code cleanup
Spacefish Jun 19, 2016
e231caa
tidy up code based on feedback
Spacefish Jun 20, 2016
23e1071
introduce upload subfolders configuration variable
Spacefish Jun 19, 2016
7f02bb0
make InstantUploader honor the SubFolder setting
Spacefish Jun 19, 2016
f85cf5d
use long for timestamp and don´t fail if timestamp is empty
Spacefish Jun 19, 2016
6401731
implement the same for videos and do some code cleanup
Spacefish Jun 19, 2016
bed4644
tidy up code based on feedback
Spacefish Jun 20, 2016
9b4a4d2
Merge branch 'feature_uploadsubfolders' of https://github.com/nextclo…
Spacefish Jun 20, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@

<string name="error__upload__local_file_not_copied">%1$s could not be copied to %2$s local folder</string>
<string name="prefs_instant_upload_path_title">Upload path</string>
<string name="prefs_instant_upload_path_use_subfolders_title">Use subfolders</string>
<string name="prefs_instant_upload_path_use_subfolders_summary">Store in subfolders based on year and month</string>

@przybylski przybylski Jun 20, 2016

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.

year & month: plural or singular ?

@Spacefish Spacefish Jun 20, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

both, as year and month refers to a single subfolder, but multiple subfolders are created.


<string name="share_link_no_support_share_api">Sorry, sharing is not enabled on your server. Please contact your
administrator.</string>
Expand Down
10 changes: 9 additions & 1 deletion res/xml/preferences.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,13 @@
<com.owncloud.android.ui.CheckBoxPreferenceWithLongTitle android:key="instant_uploading"
android:title="@string/prefs_instant_upload"
android:summary="@string/prefs_instant_upload_summary"/>
<com.owncloud.android.ui.PreferenceWithLongSummary
<com.owncloud.android.ui.PreferenceWithLongSummary
android:title="@string/prefs_instant_upload_path_title"
android:key="instant_upload_path" />
<com.owncloud.android.ui.CheckBoxPreferenceWithLongTitle
android:title="@string/prefs_instant_upload_path_use_subfolders_title"

@tobiasKaminsky tobiasKaminsky Jun 19, 2016

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.

Please only lines with 80 chars, this makes review on github a lot easier.

@AndyScherzinger AndyScherzinger Jun 19, 2016

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.

@tobiasKaminsky 120 is fine with me ;) Since this is still enough for the files changed tab :D So I would vote to go for 120 line length. @przybylski

@przybylski przybylski Jun 20, 2016

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.

Longer line would be great I would vote to stick to how android code is written, which is 100 lines.
https://source.android.com/source/code-style.html#limit-line-length

@AndyScherzinger AndyScherzinger Jun 20, 2016

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.

@tobiasKaminsky what do you think? I am with @przybylski on this, so I also vote for 100 💯

@tobiasKaminsky tobiasKaminsky Jun 21, 2016

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.

100 is also fine for me. I do not know who said 80 back on owncloud, but if 100 is still visible in changes tab on github everything is fine.

@przybylski przybylski Jun 21, 2016

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.

I think I introduced 80 chars limit. I don't recall why :D

@AndyScherzinger AndyScherzinger Jun 22, 2016

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.

I think Github would also work with 120, not sure though...

android:summary="@string/prefs_instant_upload_path_use_subfolders_summary"
android:key="instant_upload_path_use_subfolders" />
<com.owncloud.android.ui.CheckBoxPreferenceWithLongTitle
android:title="@string/instant_upload_on_wifi"
android:key="instant_upload_on_wifi"/>
Expand All @@ -37,6 +41,10 @@
<com.owncloud.android.ui.PreferenceWithLongSummary
android:title="@string/prefs_instant_video_upload_path_title"
android:key="instant_video_upload_path" />
<com.owncloud.android.ui.CheckBoxPreferenceWithLongTitle
android:title="@string/prefs_instant_upload_path_use_subfolders_title"
android:summary="@string/prefs_instant_upload_path_use_subfolders_summary"
android:key="instant_video_upload_path_use_subfolders" />
<com.owncloud.android.ui.CheckBoxPreferenceWithLongTitle
android:title="@string/instant_video_upload_on_wifi"
android:key="instant_video_upload_on_wifi"/>
Expand Down
10 changes: 10 additions & 0 deletions src/com/owncloud/android/db/PreferenceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@ public abstract class PreferenceManager {
private static final String AUTO_PREF__SORT_ASCENDING = "sort_ascending";
private static final String PREF__INSTANT_UPLOADING = "instant_uploading";
private static final String PREF__INSTANT_VIDEO_UPLOADING = "instant_video_uploading";
private static final String PREF__INSTANT_UPLOAD_PATH_USE_SUBFOLDERS = "instant_upload_path_use_subfolders";
private static final String PREF__INSTANT_UPLOAD_ON_WIFI = "instant_upload_on_wifi";
private static final String PREF__INSTANT_VIDEO_UPLOAD_ON_WIFI = "instant_video_upload_on_wifi";
private static final String PREF__INSTANT_VIDEO_UPLOAD_PATH_USE_SUBFOLDERS = "instant_video_upload_path_use_subfolders";

public static boolean instantPictureUploadEnabled(Context context) {
return getDefaultSharedPreferences(context).getBoolean(PREF__INSTANT_UPLOADING, false);
Expand All @@ -48,10 +50,18 @@ public static boolean instantVideoUploadEnabled(Context context) {
return getDefaultSharedPreferences(context).getBoolean(PREF__INSTANT_VIDEO_UPLOADING, false);
}

public static boolean instantPictureUploadPathUseSubfolders(Context context) {
return getDefaultSharedPreferences(context).getBoolean(PREF__INSTANT_UPLOAD_PATH_USE_SUBFOLDERS, false);
}

public static boolean instantPictureUploadViaWiFiOnly(Context context) {
return getDefaultSharedPreferences(context).getBoolean(PREF__INSTANT_UPLOAD_ON_WIFI, false);
}

public static boolean instantVideoUploadPathUseSubfolders(Context context) {
return getDefaultSharedPreferences(context).getBoolean(PREF__INSTANT_VIDEO_UPLOAD_PATH_USE_SUBFOLDERS, false);
}

public static boolean instantVideoUploadViaWiFiOnly(Context context) {
return getDefaultSharedPreferences(context).getBoolean(PREF__INSTANT_VIDEO_UPLOAD_ON_WIFI, false);
}
Expand Down
8 changes: 6 additions & 2 deletions src/com/owncloud/android/files/InstantUploadBroadcastReceiver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ private void handleNewPictureAction(Context context, Intent intent) {
String file_path = null;
String file_name = null;
String mime_type = null;
long date_taken = 0;

Log_OC.i(TAG, "New photo received");

Expand Down Expand Up @@ -116,6 +117,7 @@ private void handleNewPictureAction(Context context, Intent intent) {
file_path = c.getString(c.getColumnIndex(Images.Media.DATA));
file_name = c.getString(c.getColumnIndex(Images.Media.DISPLAY_NAME));
mime_type = c.getString(c.getColumnIndex(Images.Media.MIME_TYPE));
date_taken = System.currentTimeMillis();

@przybylski przybylski Jun 20, 2016

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.

why do you separate declaration and initialization?

@Spacefish Spacefish Jun 20, 2016 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

cause it´s done like that in the code above it, and i don´t want to break the codeflow for better readability

@przybylski przybylski Jun 20, 2016

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.

IMO declarasion should be also initialization. I would even fix wrong other stuff. @AndyScherzinger @tobiasKaminsky you agree?

@AndyScherzinger AndyScherzinger Jun 21, 2016

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.

I would leave it like that until we are done with most of the open PRs and then open refactoring tasks

c.close();

if (file_path.equals(lastUploadedPhotoPath)) {
Expand All @@ -134,7 +136,7 @@ private void handleNewPictureAction(Context context, Intent intent) {
context,
account,
file_path,
FileStorageUtils.getInstantUploadFilePath(context, file_name),
FileStorageUtils.getInstantUploadFilePath(context, file_name, date_taken),
behaviour,
mime_type,
true, // create parent folder if not existent
Expand All @@ -161,6 +163,7 @@ private void handleNewVideoAction(Context context, Intent intent) {
String file_path = null;
String file_name = null;
String mime_type = null;
long date_taken = 0;

Log_OC.i(TAG, "New video received");

Expand All @@ -186,6 +189,7 @@ private void handleNewVideoAction(Context context, Intent intent) {
file_name = c.getString(c.getColumnIndex(Video.Media.DISPLAY_NAME));
mime_type = c.getString(c.getColumnIndex(Video.Media.MIME_TYPE));
c.close();
date_taken = System.currentTimeMillis();
Log_OC.d(TAG, file_path + "");

int behaviour = getUploadBehaviour(context);
Expand All @@ -194,7 +198,7 @@ private void handleNewVideoAction(Context context, Intent intent) {
context,
account,
file_path,
FileStorageUtils.getInstantVideoUploadFilePath(context, file_name),
FileStorageUtils.getInstantVideoUploadFilePath(context, file_name, date_taken),
behaviour,
mime_type,
true, // create parent folder if not existent
Expand Down
12 changes: 10 additions & 2 deletions src/com/owncloud/android/ui/activity/Preferences.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,11 @@ public class Preferences extends PreferenceActivity
private Preference mPrefInstantUpload;
private Preference mPrefInstantUploadBehaviour;
private Preference mPrefInstantUploadPath;
private Preference mPrefInstantUploadUseSubfolders;
private Preference mPrefInstantUploadPathWiFi;
private Preference mPrefInstantVideoUpload;
private Preference mPrefInstantVideoUploadPath;
private Preference mPrefInstantVideoUploadUseSubfolders;
private Preference mPrefInstantVideoUploadPathWiFi;
private String mUploadVideoPath;

Expand Down Expand Up @@ -391,7 +393,8 @@ public boolean onPreferenceClick(Preference preference) {

mPrefInstantUploadCategory =
(PreferenceCategory) findPreference("instant_uploading_category");


mPrefInstantUploadUseSubfolders = findPreference("instant_upload_path_use_subfolders");
mPrefInstantUploadPathWiFi = findPreference("instant_upload_on_wifi");
mPrefInstantUpload = findPreference("instant_uploading");

Expand Down Expand Up @@ -426,7 +429,8 @@ public boolean onPreferenceClick(Preference preference) {
}
});
}


mPrefInstantVideoUploadUseSubfolders = findPreference("instant_video_upload_path_use_subfolders");
mPrefInstantVideoUploadPathWiFi = findPreference("instant_video_upload_on_wifi");
mPrefInstantVideoUpload = findPreference("instant_video_uploading");
toggleInstantVideoOptions(((CheckBoxPreference) mPrefInstantVideoUpload).isChecked());
Expand Down Expand Up @@ -477,19 +481,23 @@ private void toggleInstantPictureOptions(Boolean value){
if (value){
mPrefInstantUploadCategory.addPreference(mPrefInstantUploadPathWiFi);
mPrefInstantUploadCategory.addPreference(mPrefInstantUploadPath);
mPrefInstantUploadCategory.addPreference(mPrefInstantUploadUseSubfolders);
} else {
mPrefInstantUploadCategory.removePreference(mPrefInstantUploadPathWiFi);
mPrefInstantUploadCategory.removePreference(mPrefInstantUploadPath);
mPrefInstantUploadCategory.removePreference(mPrefInstantUploadUseSubfolders);
}
}

private void toggleInstantVideoOptions(Boolean value){
if (value){
mPrefInstantUploadCategory.addPreference(mPrefInstantVideoUploadPathWiFi);
mPrefInstantUploadCategory.addPreference(mPrefInstantVideoUploadPath);
mPrefInstantUploadCategory.addPreference(mPrefInstantVideoUploadUseSubfolders);
} else {
mPrefInstantUploadCategory.removePreference(mPrefInstantVideoUploadPathWiFi);
mPrefInstantUploadCategory.removePreference(mPrefInstantVideoUploadPath);
mPrefInstantUploadCategory.removePreference(mPrefInstantVideoUploadUseSubfolders);
}
}

Expand Down
56 changes: 50 additions & 6 deletions src/com/owncloud/android/utils/FileStorageUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,19 @@
package com.owncloud.android.utils;

import java.io.File;
import java.text.SimpleDateFormat;
import java.util.Collections;
import java.util.Comparator;
import java.util.Date;
import java.util.Locale;
import java.util.Vector;

import third_parties.daveKoeller.AlphanumComparator;

import com.owncloud.android.MainApp;
import com.owncloud.android.R;
import com.owncloud.android.datamodel.OCFile;
import com.owncloud.android.lib.common.utils.Log_OC;
import com.owncloud.android.lib.resources.files.RemoteFile;

import android.accounts.Account;
Expand All @@ -47,6 +51,8 @@
* Static methods to help in access to local file system.
*/
public class FileStorageUtils {
private static final String TAG = FileStorageUtils.class.getSimpleName();

public static final Integer SORT_NAME = 0;
public static final Integer SORT_DATE = 1;
public static final Integer SORT_SIZE = 2;
Expand Down Expand Up @@ -117,12 +123,46 @@ public static final String getLogPath() {
return Environment.getExternalStorageDirectory() + File.separator + MainApp.getDataFolder() + File.separator + "log";
}

public static String getInstantUploadFilePath(Context context, String fileName) {
/**
* Returns the a string like 2016/08/ for the passed date. If date is 0 an empty
* string is returned
*
* @param date: date in microseconds since 1st January 1970
* @return
*/
private static String getSubpathFromDate(long date) {

@przybylski przybylski Jun 22, 2016

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.

I think that passing date as a parameter isn't needed. You can get current time inside of this function. Pushing it here all the way doesn't makes too much of a sense.

@Spacefish Spacefish Jun 23, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I introduced the parameter, i might later on want to upload existing photos (upload all media on the device, which hasn´t been uploaded already). This would require us to read the timestamp from the pictures and use this timestamp for folderpath. Thats why i made it into a parameter on the method.

if (date == 0) {
return "";
}
try {
SimpleDateFormat formatter = new SimpleDateFormat(
"yyyy" + OCFile.PATH_SEPARATOR + "MM" + OCFile.PATH_SEPARATOR, Locale.ENGLISH);

@przybylski przybylski Jun 22, 2016

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.

IMO it should be Locale.getDefault()

return formatter.format(new Date(date));
}
catch(RuntimeException ex) {
Log_OC.w(TAG, "could not extract date from timestamp");
return "";

@przybylski przybylski Jun 20, 2016

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.

Please log what went wrong

}
}

/**
* Returns the InstantUploadFilePath on the owncloud instance
*
* @param context
* @param fileName
* @param dateTaken: Time in milliseconds since 1970 when the picture was taken.
* @return
*/
public static String getInstantUploadFilePath(Context context, String fileName, long dateTaken) {
SharedPreferences pref = PreferenceManager.getDefaultSharedPreferences(context);
String uploadPathdef = context.getString(R.string.instant_upload_path);
String uploadPath = pref.getString("instant_upload_path", uploadPathdef);
String value = uploadPath + OCFile.PATH_SEPARATOR + (fileName == null ? "" : fileName);
return value;
String subPath = "";
if (com.owncloud.android.db.PreferenceManager.instantPictureUploadPathUseSubfolders(context)) {
subPath = getSubpathFromDate(dateTaken);
}
return uploadPath + OCFile.PATH_SEPARATOR + subPath
+ (fileName == null ? "" : fileName);
}

/**
Expand All @@ -131,12 +171,16 @@ public static String getInstantUploadFilePath(Context context, String fileName)
* @param fileName: video file name
* @return String: video file path composed
*/
public static String getInstantVideoUploadFilePath(Context context, String fileName) {
public static String getInstantVideoUploadFilePath(Context context, String fileName, long dateTaken) {
SharedPreferences pref = PreferenceManager.getDefaultSharedPreferences(context);
String uploadVideoPathdef = context.getString(R.string.instant_upload_path);
String uploadVideoPath = pref.getString("instant_video_upload_path", uploadVideoPathdef);
String value = uploadVideoPath + OCFile.PATH_SEPARATOR + (fileName == null ? "" : fileName);
return value;
String subPath = "";
if (com.owncloud.android.db.PreferenceManager.instantVideoUploadPathUseSubfolders(context)) {
subPath = getSubpathFromDate(dateTaken);
}
return uploadVideoPath + OCFile.PATH_SEPARATOR + subPath
+ (fileName == null ? "" : fileName);
}

public static String getParentPath(String remotePath) {
Expand Down