[stable9] lenz1111's fantastic share download range requests support #24830

Merged
merged 4 commits into from May 30, 2016

Projects

None yet

3 participants

@PVince81
Collaborator

Backport of #24750 to stable9

Please review and test @lenz1111 @michaelstingl @davitol @owncloud/sharing @owncloud/filesystem

Had to revert the PSR-4 namespace of the new exception.

lenz1111 and others added some commits May 20, 2016
@lenz1111 @PVince81 lenz1111 Http Range requests support in downloads
Http range requests support is required for video preview
f29f857
@lenz1111 @PVince81 lenz1111 Code style and doc fix 7a791c5
@PVince81 PVince81 Lowercase class name for unseekableexception
Because 9.1 was PSR-4 and the backport goes back to the old style
8c43964
@PVince81 PVince81 added this to the 9.0.3-current-maintenance milestone May 25, 2016
@lenz1111 lenz1111 and 1 other commented on an outdated diff May 25, 2016
lib/public/files/unseekableexception.php
+ * along with this program. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+/**
+ * Public interface of ownCloud for apps to use.
+ * Files/UnseekableException class
+ */
+
+// use OCP namespace for all classes that are considered public.
+// This means that they should be used by apps instead of the internal ownCloud classes
+namespace OCP\Files;
+
+/**
+ * Exception for seek problem
+ * @since 9.1.0
@lenz1111
lenz1111 May 25, 2016 Member

Should be here @since 9.1.0?

@PVince81
PVince81 May 25, 2016 Collaborator

good point... maybe we need to use another approach that doesn't require an exception 😦

@lenz1111
Member

WFM 👍

@davitol
Contributor
davitol commented May 25, 2016 edited

WFM too using Safari as browser for reproducing share via link videos 👍

@PVince81
Collaborator

👎 we need to find a solution for the newly added exception.
Since it's added to the public namespace it qualifies as "API change" or "API addition".

@lenz1111 is there a way to make the code work without the exception ?

@lenz1111
Member

@PVince81 We could return false by readFilePart whenfseek fail - but easiest workaround is to change thrown exception (by readFilePart) to for example NotPermittedException - what you think?

@PVince81
Collaborator

Let's do the NotPermittedException one, because return false is for when fopen fails, in which case it doesn't make sense to retry.

@lenz1111
Member

OK - I'll fix it

@lenz1111 lenz1111 replaced UnseekableException => NotPermittedException
002a7f8
@PVince81
Collaborator

@lenz1111 protip: after committing stuff, please ping people for review because we don't get a notification from Github automatically 😄

@PVince81
Collaborator

Retested, works. 👍

@PVince81 PVince81 merged commit 0ec4098 into stable9 May 30, 2016

14 of 15 checks passed

core-ci-linux-swift-primary-storage/database=mysql,label=SLAVE Build #56704 failed in 1 min 10 sec
Details
cla-bot-core Build #4432 succeeded in 1 min 42 sec
Details
continuous-integration/php-5.4 Build #4252 succeeded in 4 min 14 sec
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
core-ci-linux-jsunit/database=sqlite,label=SLAVE Build #62488 succeeded in 3 min 4 sec
Details
ocs-api-integration-tests-ci Build #11108 succeeded in 1 hr 17 min
Details
server-master-linux-externals-ci/database=sqlite,external=webdav-ownCloud,label=SLAVE Build #10650 succeeded in 15 min
Details
server-master-linux-php7-ci/database=sqlite,label=SLAVE Build #39527 succeeded in 4 min 33 sec
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=litmus,mirallBranch=v2.0.2,slave=SMASH Build #15027 succeeded in 8 min 15 sec
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_basicSync@0,mirallBranch=v2.0.2,slave=SMASH Build #15027 succeeded in 15 min
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_basicSync@1,mirallBranch=v2.0.2,slave=SMASH Build #15027 succeeded in 49 min
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_shareLink,mirallBranch=v2.0.2,slave=SMASH Build #15027 succeeded in 33 min
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_sharePermissions,mirallBranch=v2.0.2,slave=SMASH Build #15027 succeeded in 47 min
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_sharePropagationGroups,mirallBranch=v2.0.2,slave=SMASH Build #15027 succeeded in 18 min
Details
smashbox-on-docker-ci/DOCKER_IMAGE=ubuntu_oc_lamp-git,TEST_NAME=test_sharePropagationInsideGroups,mirallBranch=v2.0.2,slave=SMASH Build #15027 succeeded in 12 min
Details
@PVince81 PVince81 deleted the stable9-lenz1111-share_download_range_requests_support branch May 30, 2016
@lenz1111
Member
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment