Dear Bernhard Dear list members Today I released version: 0.1.0 from my App: https://github.com/jmeile/agreedisclaimer/tree/0.1.0
I implemented lots of the suggestions given by Bernhard. I really appreciate your time. The comments were really helpful. The only thing I haven't done yet, was automatic injection. For me it is somehow a complicated concept, which I won't implement for now. Best regards Josef -----Original Message----- From: Josef Meile [mailto:[email protected]] Sent: Donnerstag, 6. August 2015 18:42 To: 'List for Developers of ownCloud' Subject: RE: [owncloud-devel] Injecting services and variables to App constructors and calling them Dear Bernhard Ok, I just setup the github: https://github.com/jmeile/agreedisclaimer There are two branches there: * 0.0.1: The original code as it is posted in the app store. This code is working; however, it not using injected translator nor the logger. * 0.1.0a: Alpha release where I tried to inject the services and get rid of some ugly static stuff. The problem with the alpha release is that it still uses: \OCP\Util::writeLog and \OCP\Util::getL10N($appId)->t in some places. Other thing I didn't succeed was removing the class constant: Application::APP_ID and inject it on the app. Somehow I suspect that my main problem is that I have a router called: "settings#get_files" / SettingsController->getFiles. Inside that function, I call other controller functions, but when I tried to use some class members like "$this->trans" and "$this->logger" I see messages like: "Call to a member function t() on a non-object" "Call to a member function error() on a non-object" This happens most of the time when the route: "settings#get_files" gets called through ajax. Although the function "SettingsController->getFiles" is a class method, I would say that the route is some kind of static call and no instance to the class can be determined, so, "$this" isn't recognized. You will see all parts where I got the errors with a "Fix it" comment on the code. There you can uncomment the next lines and you will get the errors. Another problem is with the "Application::APP_ID" constant. I want to remove it by injecting the application's name into the constructors of: Application and SettingsController, using a private variable called $appId. The problem here is that if I use: $this->appId, sometimes it will be evaluated to the empty string and no error will be logged, so, strange things will happened, ie: the application won't appear on the admin page. I would really appreciate if you have some suggestion. Best regards Josef -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Bernhard Posselt Sent: Mittwoch, 5. August 2015 20:18 To: [email protected] Subject: Re: [owncloud-devel] Injecting services and variables to App constructors and calling them The Application class is not defined in the tutorial ;) There are a few reasons to still use an Application class, for instance if you want to overwrite how a class is instantiated or if you want to resolve an interface to a class. I will take a look when the code that is not working for you is on GitHub and provide a PR ;) On 08/05/2015 08:12 PM, Josef Meile wrote: > Dear Bernhard > > Thanks for your feedback. > > I saw that part about automatic injection too, but didn't use it since > it was somehow confusing for me: that's the only part on the tutorial > where it is mentioned, then on the rest of it, the "Application class" > will be defined. So, I think this is somehow inconsistence and in my > opinion, if something is recommended, then it should be used for the > rest of the tutorial; you can always see the previous versions of the > tutorial and see the old way. I also choose this way because most code > of the apps I saw used it too. Anyway, I will give it a try too. > > Regarding Github, I haven't setup it yet, but I will do it soon. Right > now, the app is only in the store: > https://apps.owncloud.com/content/show.php/AgreeDisclaimer?content=170 > 844 > > Best regards > Josef > > -----Original Message----- > From: [email protected] [mailto:[email protected]] > On Behalf Of Bernhard Posselt > Sent: Mittwoch, 5. August 2015 19:44 > To: [email protected] > Subject: Re: [owncloud-devel] Injecting services and variables to App > constructors and calling them > > Full file? Is your project on Github? > > Apart from that I can only highlight the tutorial at > https://doc.owncloud.org/server/8.1/developer_manual/app/tutorial.html > > My advice: Use automatic injection (ownCloud 8+), that is remove > everything in the registerServices function and let ownCloud figure > out everything else. The way this works is that it uses reflection to > get the type hints in your constructor (and variable names for scalar > types) and passes the stuff in that is needed. Also controllers are resolved automatically as in: > settings#index instantiates OCA\YourApp\SettingsController by default, > no need to write extra code like in versions prior to 7. > > More information on how it works in detail is here > https://doc.owncloud.org/server/8.1/developer_manual/app/container.htm > l#use- automatic-dependency-assembly-recommended > > On 08/05/2015 07:18 PM, Josef Meile wrote: >> Sorry, I saw an error in the constructor of my Application class, it >> should >> be: >> >> public function __construct($appId, array $urlParams=array()) { >> parent::__construct($appId, $urlParams); >> >> $this->appId = $appId; >> $this->registerServices(); >> $this->registerHooks(); >> $this->registerSettings(); >> } >> >> Anyway, the problems are still there. I would appreciate if somebody >> gives me a clue. >> >> -----Original Message----- >> From: Josef Meile [mailto:[email protected]] >> Sent: Mittwoch, 5. August 2015 19:15 >> To: [email protected] >> Subject: Injecting services and variables to App constructors and >> calling them >> >> Dear all >> >> I have developed the AgreeDisclaimer App and it is working as it >> should; however, I got the feedback from another developer that I >> should avoid this >> things: >> 1) \OC::$server calls >> 2) Using "\OCP\Util::writeLog" for the logger or "\OCP\Util::getL10N" >> for translating things >> >> The recommendation was to inject them into the constructors. I have >> tried to do this, and in some cases I succeeded, but in others don't. >> First, I tried to inject the L10N service in my controller: >> >> class SettingsController extends Controller { >> private $trans; >> >> public function __construct($AppName, IRequest $request, IL10N >> $trans) > { >> parent::__construct($AppName, $request); >> $this->trans = $trans; >> } >> >> /* Some more code comes here */ >> >> } >> >> Then I called the constructor as follows in Application.php: >> class Application extends App { >> private $appId; >> >> public function __construct($appId, array $urlParams=array()) { >> parent::__construct('agreedisclaimer', $urlParams); >> >> $this->appId; >> $this->registerServices(); >> $this->registerHooks(); >> $this->registerSettings(); >> } >> >> public function registerServices() { >> $container = $this->getContainer(); >> >> $container->registerService('L10N', function($c) { >> return >> $c->query('ServerContainer')->getL10N($c->query('AppName')); >> }); >> >> $container->registerService('SettingsController', function($c) { >> return new SettingsController ( >> $c->query('AppName'), >> $c->query('Request'), >> $c->query('L10N') >> ); >> }); >> } >> >> /* Some more code comes here */ >> } >> >> Then I tried to use the $trans variable in a method of my controller >> as >> follows: >> public function getFile($userLang, $defaultLang, $basePath, $fileExt, >> $getContent = false, $maxFileSize = 2, $getFallbackLang = >> true) { >> >> /* Some code comes here */ >> >> $errorMsg = $this->trans->t('%s doesn\'t exist.', >> $userLangFile . '<br/>') . ' ' . >> $this->trans->t('Please contact the webmaster'); >> >> /* Some more code comes here */ >> } >> >> I got an error exactly on that line: >> "Call to a member function t() on a non-object at >> \var\www\html\owncloud\apps\agreedisclaimer\controller\settingscontro >> l >> ler.ph >> p#227" >> >> The function getFile gets called from another function called >> "getFiles", which is a route called through an ajax request: >> function getFiles($isAdminForm = false, $defaultLang = null) { >> >> /* Some code comes here */ >> >> $fileInfo = $this->getFile($userLang, $defaultLang, >> $txtFileBasePath, >> 'txt', true, $maxTxtFileSize, $getFallbackLang); >> >> /* Some more code comes here */ } >> >> And here the routes: >> <?php >> return ['routes' => [ >> ['name' => 'settings#get_settings', >> 'url' => '/settings/get_all', 'verb' => 'GET'], >> ['name' => 'settings#get_files', >> 'url' => '/settings/get_files', 'verb' => 'GET'], ]]; >> >> I think that the problem is that "getFiles is a route, so it doesn't >> really needs an object instance to the SettingsController class. It >> is some kind of static function, so, that's why "$this" can't be >> used. My original code is >> using: >> \OCP\Util::getL10N($appId)->t('%s doesn\'t exist.', $userLangFile . >> '<br/>') >> >> and it works. But it isn't the way of doing it according to the >> Developers manual, so, how can I solve this problem? >> >> The second problem I'm facing: on the class "Application", I defined >> a private variable called "$appId". I would like to inject it to the >> admin page, but I don't know how to do this. First I registered the >> admin page on the "Application" class: >> >> \OCP\App::registerAdmin($this->appId, 'admin'); >> >> Then in the "agreedisclaimer/admin.php" file, I render the template: >> agreedisclaimer/templates/admin.php >> >> \OCP\User::checkAdminUser(); >> $data = []; >> $templateResponse = new TemplateResponse($appId, 'admin', $data, >> 'blank'); return $templateResponse->render(); >> >> How can I get the Application object from "agreedisclaimer/admin.php" >> and push the $appId variable into the $data array? Here again, my >> workaround was a static class variable instead of a private one: >> >> class Application extends App { >> const APP_ID = 'agreedisclaimer'; >> >> /* Some more code comes here */ } >> >> Then I called it like: Application::APP_ID, but this isn't the way it >> should be. How can I avoid using this static constant? >> >> Thanks in advanced. >> >> Best regards >> Josef >> >> _______________________________________________ >> Devel mailing list >> [email protected] >> http://mailman.owncloud.org/mailman/listinfo/devel > _______________________________________________ > Devel mailing list > [email protected] > http://mailman.owncloud.org/mailman/listinfo/devel > > _______________________________________________ > Devel mailing list > [email protected] > http://mailman.owncloud.org/mailman/listinfo/devel _______________________________________________ Devel mailing list [email protected] http://mailman.owncloud.org/mailman/listinfo/devel _______________________________________________ Devel mailing list [email protected] http://mailman.owncloud.org/mailman/listinfo/devel
