https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42663

Jonathan Druart <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |In Discussion

--- Comment #3 from Jonathan Druart <[email protected]> ---
I think there are several good things in this approach:

1. Code is elegant and easy to read!
my $context = Koha::Context->current;
my $preferences = $context->preferences;
my $value = $preferences->get_value("MyPref");

my $config = $context->config;
my $value = $config->get("entry");

my $userenv = $context->userenv;
my $number = $userenv->get("number");

2. Remove circular deps and lazy-load "components" (Koha::Context)

3. Need only one middleware (Koha::Middleware::Context replaces
Koha::Context::UserEnv, but could also replace RealIP and SetEnv)

4. We set the env from the Plack's middleware and retrieve it from
Koha::Context. This might feel weird at first but I think it's a good solution
for our hybride PSGI situation (Koha::Context->current).

Koha/Context.pm
 70     # Try to get from Plack environment first                               
 71     my $env = $class->is_psgi_env;                                          
 72     if ($env) {
 73                                                                             
 74         # Some modules pre-loaded in plack.psgi access the context before
we set it in the middleware 
 75         # FIXME Is it an (existing) security issue?                         
 76         my $existing_context =
$Koha::Middleware::Context::psgi_env->{'koha.context'};                
 77         return $existing_context || $class->new;                            
 78     }

Koha/Middleware/Context.pm
 50     # Store context in environment
 51     $env->{'koha.context'} = $context;
 52             
 53     # Initialize remote address is done in Koha::Middleware::RealIP
 54     # (for non-Plack CGI scripts this is done in BEGIN block)
 55 
 56     $context->set_current($context);
 57 
 58     # Initialize userenv through context
 59     my $userenv = $context->userenv;
 60     $userenv->attach( {} );
 61 
 62     local $psgi_env = $env;
 63     return $self->app->($env);

We can move this to memory cache if not considered nice/robust enough.

5. Bad code is highlighted here however, some preloaded modules (from
plack.psgi) uses the context at compile time (mostly from BEGIN blocks), which
is obviously not set yet. IMO this should be remove ASAP anyway, and as
follow-up of this bug. That's why `use C4::XSLT` is (temporarily) removed from
plack.psgi.

The whole tree is available on my gitlab, branch bug_42663
https://gitlab.com/joubu/Koha/-/commits/bug_42663

Next steps:
* Apply to C4::Context
* Koha::Context->Preferences should have use_syspref_cache defaults to true
(currently false)
* Write missing POD and tests
* Remove `use C4::Context` where it's used only for config, syspref, userenv.
[...]
* Kill C4::Context

This is either awesome and we need to move forward, or I am missing something
silly. Please provide feedback, I would be more than happy to continue working
on this!

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to