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

--- Comment #18 from David Cook <[email protected]> ---
> > (In reply to Jonathan Druart from comment #12)
> > > > Koha::Context shouldn't depend on Koha::Middleware::Context. In fact, I
> > > > don't think that we really need Koha::Middleware::Context. All
> > > > Koha::Context->current() needs to do is return a singleton. So check 
> > > > for the
> > > > $ENV{"koha.context"}. If it's set, return its contents. If it's not 
> > > > set, set
> > > > it. That's it.
> > > 
> > > We cannot store structures in ENV, only strings. That's why I need to 
> > > reuse
> > > the psgi_env var from the middleware.
> > 
> > I don't understand this. We are already storing structures in a PSGI
> > environment. Why would we need to store a structure in an "ENV"
> > environmental variable? Is it because of the CGI scripts?
> 
> What do we store already there?

In your patch:
47          # Create request-scoped context
48          my $context = Koha::Context->new;
49       
50          # Store context in environment
51          $env->{'koha.context'} = $context;

> > Note that https://metacpan.org/pod/Plack::Component#OBJECT-LIFECYCLE warns
> > against saving per-request data in the object (and that would be especially
> > true for an "our" variable as well).
> > 
> > --
> > 
> > Overall, I think that we're mixing up a number of things here still, and
> > it's probably because of the CGI scripts. If/when the app is 100% a Plack
> > app or 100% a Mojo app, then we'll be able to easily handle things at the
> > beginning and ending of the request cycle.
> 
> Obviously we need to mix and workaround, as usual. But we don't need to wait
> to switch to a 100% mojo app to make this kind of code improvements.

But where would we use Koha::Middleware::Context::psgi_env? I don't understand
that part. I'm not seeing where it's used in the code?

> IMO the solution is quite good and elegant. I would be happy to receive
> follow-up patches for any further improvements you can find. Certainly a
> good area to iterate together.

I think it's pretty good overall. There's just a couple of fine points I'm not
sure about. 

--

I'm also not sure about the dependency graph? Is the idea that we complete Bug
42663 and then redo the patches for the removal of C4::Context? Or are those
separate?

-- 
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