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

Add EventSource to the public API #10739

Merged
merged 5 commits into from Sep 8, 2014
Merged

Add EventSource to the public API #10739

merged 5 commits into from Sep 8, 2014

Conversation

icewind1991
Copy link
Contributor

Also fixes EventSource to not start sending output the moment we construct it but wait until we start sending data.

cc @PVince81 @DeepDiver1975 @MorrisJobke

@ghost
Copy link

ghost commented Aug 29, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/6992/

@MorrisJobke
Copy link
Contributor

@icewind1991 A test case would be really nice

@DeepDiver1975
Copy link
Member

@icewind1991 A test case would be really nice

yes - please - besides that: looks good 👍

@DeepDiver1975
Copy link
Member

@icewind1991 squash and rebase please

@MorrisJobke
Copy link
Contributor

test case means how to test this change (manually)

@ghost
Copy link

ghost commented Sep 3, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/7063/

@icewind1991
Copy link
Contributor Author

Basic manual test: run scanFiles(true) in the js console while in the files app, if the progress is shown in the console then the event source is working

@@ -492,4 +492,13 @@ function getCertificateManager($user = null) {
}
return new CertificateManager($user);
}

/**
* Returns a search instance
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the comment.
It's important to state that this will CREATE a new event source, as the class itself doesn't have any "open()" method.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd almost even prefer calling it createEventSource(), because when seeing getEventSource() used somewhere it is not clear that we are getting new instance instead of a possibly cached one.

If you compare with other services above they all seem to have singletons.

@icewind1991
Copy link
Contributor Author

@PVince81 switched to createEventSource()

@ghost
Copy link

ghost commented Sep 3, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/7082/

@MorrisJobke
Copy link
Contributor

👎

{"reqId":"54080001a86f8","app":"index","message":"BadMethodCallException: Type needs to be alphanumeric","level":4,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}
{"reqId":"54080001a86f8","app":"index","message":"Exception: #1 \/home\/mjob\/Projekte\/owncloud\/master\/apps\/files\/ajax\/scan.php(34): OC_EventSource->close()","level":4,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}
{"reqId":"54080001a86f8","app":"index","message":"Exception: #2 \/home\/mjob\/Projekte\/owncloud\/master\/lib\/private\/route\/route.php(135) : runtime-created function(1): require_once('\/home\/mjob\/Proj...')","level":4,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}
{"reqId":"54080001a86f8","app":"index","message":"Exception: #3 [internal function]: __lambda_func(Array)","level":4,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}
{"reqId":"54080001a86f8","app":"index","message":"Exception: #4 \/home\/mjob\/Projekte\/owncloud\/master\/lib\/private\/route\/router.php(227): call_user_func('\\x00lambda_1506', Array)","level":4,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}
{"reqId":"54080001a86f8","app":"index","message":"Exception: #5 \/home\/mjob\/Projekte\/owncloud\/master\/lib\/base.php(725): OC\\Route\\Router->match('\/apps\/files\/aja...')","level":4,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}
{"reqId":"54080001a86f8","app":"index","message":"Exception: #6 \/home\/mjob\/Projekte\/owncloud\/master\/index.php(28): OC::handleRequest()","level":4,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}
{"reqId":"54080001a86f8","app":"index","message":"Exception: #7 {main}","level":4,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}
{"reqId":"54080001a86f8","app":"PHP","message":"Cannot modify header information - headers already sent by (output started at \/home\/mjob\/Projekte\/owncloud\/master\/lib\/private\/eventsource.php:81) at \/home\/mjob\/Projekte\/owncloud\/master\/lib\/private\/response.php#83","level":0,"time":"2014-09-04T06:00:33+00:00","method":"GET","url":"\/master\/index.php\/apps\/files\/ajax\/scan.php?force=false&dir=&requesttoken=IvcDSomqR2KLX6smuSDpD.YVraGr2o"}

@MorrisJobke
Copy link
Contributor

This appears in my log once the filescan is triggered.

@MorrisJobke
Copy link
Contributor

and reappears on every file scan

@scrutinizer-notifier
Copy link

A new inspection was created.

@icewind1991
Copy link
Contributor Author

@MorrisJobke should be fixed now

@ghost
Copy link

ghost commented Sep 4, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/7088/

@MorrisJobke
Copy link
Contributor


Testing with sqlite ...
PHP Fatal error:  Class 'OC_Mount_Config' not found in /var/lib/jenkins/jobs/pull-request-analyser/workspace/apps/files_external/tests/mountconfig.php on line 225
PHP Stack trace:
PHP   1. {main}() /usr/bin/phpunit:0
PHP   2. PHPUnit_TextUI_Command::main() /usr/bin/phpunit:46
PHP   3. PHPUnit_TextUI_Command->run() /usr/share/php/PHPUnit/TextUI/Command.php:129
PHP   4. PHPUnit_TextUI_Command->handleArguments() /usr/share/php/PHPUnit/TextUI/Command.php:138
PHP   5. PHPUnit_Util_Configuration->getTestSuiteConfiguration() /usr/share/php/PHPUnit/TextUI/Command.php:657
PHP   6. PHPUnit_Util_Configuration->getTestSuite() /usr/share/php/PHPUnit/Util/Configuration.php:789
PHP   7. PHPUnit_Framework_TestSuite->addTestFile() /usr/share/php/PHPUnit/Util/Configuration.php:912
PHP   8. PHPUnit_Framework_TestSuite->addTestSuite() /usr/share/php/PHPUnit/Framework/TestSuite.php:389
PHP   9. PHPUnit_Framework_TestSuite->__construct() /usr/share/php/PHPUnit/Framework/TestSuite.php:315
PHP  10. PHPUnit_Framework_TestSuite->addTestMethod() /usr/share/php/PHPUnit/Framework/TestSuite.php:212
PHP  11. PHPUnit_Framework_TestSuite::createTest() /usr/share/php/PHPUnit/Framework/TestSuite.php:834
PHP  12. PHPUnit_Util_Test::getProvidedData() /usr/share/php/PHPUnit/Framework/TestSuite.php:481
PHP  13. ReflectionMethod->invoke() /usr/share/php/PHPUnit/Util/Test.php:263
PHP  14. Test_Mount_Config->applicableConfigProvider() /usr/share/php/PHPUnit/Util/Test.php:263
+ ./autotest.sh mysql
Using database oc_autotest1
Setup environment for mysql testing ...
INDEX
END INDEX
Testing with mysql ...
PHP Fatal error:  Class 'OC_Mount_Config' not found in /var/lib/jenkins/jobs/pull-request-analyser/workspace/apps/files_external/tests/mountconfig.php on line 225
PHP Stack trace:
PHP   1. {main}() /usr/bin/phpunit:0
PHP   2. PHPUnit_TextUI_Command::main() /usr/bin/phpunit:46
PHP   3. PHPUnit_TextUI_Command->run() /usr/share/php/PHPUnit/TextUI/Command.php:129
PHP   4. PHPUnit_TextUI_Command->handleArguments() /usr/share/php/PHPUnit/TextUI/Command.php:138
PHP   5. PHPUnit_Util_Configuration->getTestSuiteConfiguration() /usr/share/php/PHPUnit/TextUI/Command.php:657
PHP   6. PHPUnit_Util_Configuration->getTestSuite() /usr/share/php/PHPUnit/Util/Configuration.php:789
PHP   7. PHPUnit_Framework_TestSuite->addTestFile() /usr/share/php/PHPUnit/Util/Configuration.php:912
PHP   8. PHPUnit_Framework_TestSuite->addTestSuite() /usr/share/php/PHPUnit/Framework/TestSuite.php:389
PHP   9. PHPUnit_Framework_TestSuite->__construct() /usr/share/php/PHPUnit/Framework/TestSuite.php:315
PHP  10. PHPUnit_Framework_TestSuite->addTestMethod() /usr/share/php/PHPUnit/Framework/TestSuite.php:212
PHP  11. PHPUnit_Framework_TestSuite::createTest() /usr/share/php/PHPUnit/Framework/TestSuite.php:834
PHP  12. PHPUnit_Util_Test::getProvidedData() /usr/share/php/PHPUnit/Framework/TestSuite.php:481
PHP  13. ReflectionMethod->invoke() /usr/share/php/PHPUnit/Util/Test.php:263
PHP  14. Test_Mount_Config->applicableConfigProvider() /usr/share/php/PHPUnit/Util/Test.php:263
+ ./autotest.sh pgsql
Using database oc_autotest1
Setup environment for pgsql testing ...
INDEX
END INDEX
Testing with pgsql ...
PHP Fatal error:  Class 'OC_Mount_Config' not found in /var/lib/jenkins/jobs/pull-request-analyser/workspace/apps/files_external/tests/mountconfig.php on line 225
PHP Stack trace:
PHP   1. {main}() /usr/bin/phpunit:0
PHP   2. PHPUnit_TextUI_Command::main() /usr/bin/phpunit:46
PHP   3. PHPUnit_TextUI_Command->run() /usr/share/php/PHPUnit/TextUI/Command.php:129
PHP   4. PHPUnit_TextUI_Command->handleArguments() /usr/share/php/PHPUnit/TextUI/Command.php:138
PHP   5. PHPUnit_Util_Configuration->getTestSuiteConfiguration() /usr/share/php/PHPUnit/TextUI/Command.php:657
PHP   6. PHPUnit_Util_Configuration->getTestSuite() /usr/share/php/PHPUnit/Util/Configuration.php:789
PHP   7. PHPUnit_Framework_TestSuite->addTestFile() /usr/share/php/PHPUnit/Util/Configuration.php:912
PHP   8. PHPUnit_Framework_TestSuite->addTestSuite() /usr/share/php/PHPUnit/Framework/TestSuite.php:389
PHP   9. PHPUnit_Framework_TestSuite->__construct() /usr/share/php/PHPUnit/Framework/TestSuite.php:315
PHP  10. PHPUnit_Framework_TestSuite->addTestMethod() /usr/share/php/PHPUnit/Framework/TestSuite.php:212
PHP  11. PHPUnit_Framework_TestSuite::createTest() /usr/share/php/PHPUnit/Framework/TestSuite.php:834
PHP  12. PHPUnit_Util_Test::getProvidedData() /usr/share/php/PHPUnit/Framework/TestSuite.php:481
PHP  13. ReflectionMethod->invoke() /usr/share/php/PHPUnit/Util/Test.php:263
PHP  14. Test_Mount_Config->applicableConfigProvider() /usr/share/php/PHPUnit/Util/Test.php:263

@MorrisJobke
Copy link
Contributor

@owncloud-bot retest this please

@ghost
Copy link

ghost commented Sep 4, 2014

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/7089/

@LukasReschke
Copy link
Member

👍

LukasReschke added a commit that referenced this pull request Sep 8, 2014
@LukasReschke LukasReschke merged commit 70abce0 into master Sep 8, 2014
@LukasReschke LukasReschke deleted the eventsource-public branch September 8, 2014 16:46
@lock lock bot locked as resolved and limited conversation to collaborators Aug 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants