Item13897: Implement ImproveOOModel proposal.
Priority: Enhancement
Current State: Being Worked On
Released In: 3.0.0
Target Release: major
This task is for supporting branch with implementation of
ImproveOOModel.
Major changes not yet reflected in any documentation
Foswiki::Macro role and macro objects.
Few macros are having a bad habit of storing their data on a session object. This way they pollute session namespace and have no control over the data during session shutdown. To overcome this deficiency
Foswiki::Macro
role has been introduced and a macro object handling mechanism implemented in
Fosiki::_expandMacroOnTopicRendering()
method. Upon loading of a macro module it's been checked against availability of
new()
class method. If it's available then macro's module name is been stored on the
%macros
hash instead of a coderef. The macro object is then created and stored on session's
_macros
attribute. The object is expected to do
expand()
method which is enforced by
Fosiki::Macro
role.
Foswiki::Macro
role predefines
session
attribute which is weakened to avoid circular dependencies.
FORMFIELD
is the first macro converted to make use of this technique thus making the session object free of
_ffCache
key which is now
Foswiki::Macros::FORMFIELD
attribute.
$Foswiki::PLUGINS::SESSION is weakened. Experimental.
This is done by the session constructor and nowhere else. So far tests are not affected by this change.
Notes for development
TODO
- Organize exceptions into a meaningful tree.
- Consider a generic Cache object which would not care about what kind of data is stored and only keeps refcount of add/remove requests and simply deletes an object when the count drops to 0.
To be considered/planned
Implement a Foswiki::Exception::Web
Should be a common ancestor to Oops,
AccessControl. Containts
web
and
topic
attributes.
Resolve uncertainties over Error::Simple exceptions.
It is necessary to find out if Error::Simple is always interchangeable with Foswiki::Exception or there're some special cases to handle.
Three failing tests seemingly are not covered with
expect_failure
but still considered passed. The very same tests are failing on the master branch too. So, it's either intended behavior and I miss something or it's a bug.
Fn_LANGUAGES test
Doesn't do any testing with Foswiki::I18N::Fallback class.
save()
method relies on rather unreliable method of preserving an exception object in a
catch {}
clause and rethrowing it later. The code between the
catch {}
and
throw
could be covered in a
finally {}
block including the
throw
itself.
Foswiki::Prefs
Consider a possibility of converting this class into a role and apply it to entity-family classes like Web, Topic, Plugin, etc.
Foswiki
- Reimplement ImplementationClasses inheritance chain into a simple queue of instantiated objects. Something similar to Plugins.
- Idea for a proposal: Consider unicode-enabled macro names.
Foswiki::UserMapping
Needs its own exception to replace the default Foswiki::Exception (formerly Error::Simple).
Seriously reconsider interaction of
MetaCache and Meta objecs within Moo object destruction model. Now when a newly created
Foswiki::Meta
object goes out of its scope and gets destroyed it removes itself from
MetaCache (methods sequence
DEMOLISH
->
finish
->
unload
->
Foswiki::MetaCache::removeMeta
). For a web it means that all topics from this web are getting wiped out of the cache! Even though they're still needed somewhere (by search in particular).
I have workarounded this problem by adding
inMetaCache
attribute to the
Foswiki::Meta
class. The attribute is set and reset by
Foswiki::MetaCache
only (methods
addMeta
and
removeMeta
). It's been checked by
Foswiki::Meta::unload
to determine if removal from the cache should be initiated. But there must be better solution. For example, for each key in metacache there might be a counter of object loads which would be increased with each call to
Foswiki::Meta::load
and decreased as upon
unload
.
%Foswiki::cfg to OO
Replace
%Foswiki::cfg
with a OO implementation and instantiate it in
Foswiki::Object
. Advantages:
- Future PSGI compatibility - DONE.
- Better flexibility of tests – ease of config localization depending on current test needs - DONE.
- Separate config implementation from usage allowing multiple config sources like DBI, JSON, etc...
Status: Partially done: all but the last item.
Foswiki::Config
Bootstrapping is better get use of engine's API instead of referring directly to the environment whenever possible.
Almost done
Extract bootstrap code into a separate module to be loaded only when really needed.
CRITICAL BUGS/PROBLEMS
-
Foswiki::UI::Rest
is using pathInfo
attribute directly – insecure.
-
Foswiki::Request::Rest
must not use it's SUPER class for parsing request because it may fetch web name from pathInfo
which is totally incorrect.
- Web interface failing with
The method 'BUILDARGS' is not found in the inheritance hierarchy for class Foswiki::Exception::Engine
Stack trace:
The method 'BUILDARGS' is not found in the inheritance hierarchy for class Foswiki::Exception::Engine at /usr/share/perl5/Class/Method/Modifiers.pm line 46.
Class::Method::Modifiers::install_modifier("Foswiki::Exception::Engine", "around", "BUILDARGS") called at /usr/share/perl5/Moo/_Utils.pm line 39
Moo::_Utils::_install_modifier("Foswiki::Exception::Engine", "around", "BUILDARGS", CODE(0x16d1a48)) called at /usr/share/perl5/Moo.pm line 65
Moo::around("BUILDARGS", CODE(0x16d1a48)) called at /var/www/foswiki/distro/core/lib/Foswiki/Exception.pm line 507
require Foswiki/Exception.pm called at /var/www/foswiki/distro/core/lib/Foswiki/Object.pm line 25
Foswiki::Object::BEGIN() called at /var/www/foswiki/distro/core/lib/Foswiki/Exception.pm line 0
-
FIXED by replacing use Foswiki::Exception
in Foswiki::Object
with require Foswiki::Exception
.
- LoginManaqer::complete() is called after the Config hash has been destroyed, fails with
view: Use of uninitialized value in numeric gt (>) at /var/www/foswiki/distro/core/lib/Foswiki/LoginManager.pm line 761 during global destruction.
FIXED by calling LoginManager complete() method in Foswiki::App DEMOLISH. Not sure if it's a good thing to do, would need to be reviewed.
- See Security task Item14090. The userInitializationHandler is called after Loginmanager has established identity. This needs some architectural work.
--
VadimBelman - 15 Dec 2015
Excellent work.
--
MichaelDaum - 11 Jan 2016
Thanks Michael, but these are just basics. I'm more worried about future merges from master. So far they're conflict-less but that's won't last long. And still I'll be looking for a lot of help on reworking the internals.
--
VadimBelman - 11 Jan 2016
By me, Foswiki could have one common cache handler, let call it as
Foswiki::Cache
and (IMHO) it should be
a subclass of the
CPAN:CHI .
CPAN:CHI is one very nice and fast caching system (best on the CPAN - by me), which could use various backends like: plain files, memory, memory mapped files, DBI, memcached, Redis etc... It even could be used (by using the
CPAN:CHI::Memoize) as replacement for the
CPAN:Memoize (which is extremely useful by itself and could be used to speed up some repeated method-calls). CHI has even built-in logging and much more.
Using
CPAN:CHI we could move another subsystem into some (well tested and more powerful) CPAN module and lowering the our source-code maintenance needs.
BTW, logging - another subsystem, for which we could use more CPAN - e.g. by refactoring the
Foswiki::Logger
to have something like
$logger = $foswiki->get_logger( ... )
and it based on configuration could return an logger for example: our inhouse logger(s) /aka,
PlainFile or Compatibility/, but also
CPAN:Log::Any based logger - and use them all (any) by an unified logging interface... (the
CPAN:Log::Any could by using
CPAN:Log::Any::Adapter use many other logging routines, like
CPAN:Log::Dispatch or
CPAN:Log::Log4perl)
Of course, all the above - as usually - is just an another view - for brainstorming...
--
JozefMojzis - 13 Feb 2016
We have
LogDispatchContrib which was supposed to be part of Foswiki 2.0, but was dropped because of dependency issues and missing packages on some platforms.
--
GeorgeClark - 13 Feb 2016
The main idea is the
get_logger
method which should return the logger and this logger should use an unified logging interface... (e.g. the Log::Dispatch is only one of the more possibilities... - with other words, the Log::Any is one level "above" of the Log::Dispatch.) The
CPAN:Log::Any doesn't have any
required logger dependecies. (But The user could use any of many adapters). I could imagine an -==Log::Any::Adapter::Foswiki::Plainfile== module.
--
JozefMojzis - 13 Feb 2016
Jozef, we're talking about different kind of caches here, I'm afraid.
MetaCache is about keeping pre-fetched objects in memory to spare on construction time which often involves slow disk operations.
Other caches are to be considered separately. Though I would presume that as soon as lifespan of most of Foswiki objects (not only in OO terms but in wide sense of this term) is limited to request processing use of different kind of caches is quite doubtful. But I don't have enough info yet to make well-grounded judgments.
--
VadimBelman - 13 Feb 2016
Vadim, CHI also could caching in the memory. (driver=>memory). With other words, "keeps prefetched data in the memory". So, it is perfectly usable to
MetaCache (or if you want - caching any objects) and also for any other caches - but, with
unified caching interface (which is alway good thing). And also could persisting the data - if need - using the serialised objects, with the same caching interface. Please read
CPAN:CHI::Driver::Memory and
CPAN:CHI::Driver::RawMemory. Implememting many different "pseudo-caches" (as object attribues) could seem be an good idea, but it isn't - the result is usually and overcomplicated mess - instead of having an simple unified interface for all caching needs. Also, implementing an sole caching role - you can bring (in-memory and any other) caching capabality to ANY object - using a simple
with Foswiki::Cache
. But - as i told - just an another view.
--
JozefMojzis - 14 Feb 2016
I will need to read the docs more thoroughly when have time. I've got an impression that it requires object serialization/deserialization what makes its use overcomplicated. But perhaps I'm wrong.
--
VadimBelman - 14 Feb 2016
Everybody has his own view to the solutions. That's ok and it is a base of any brainstorming. This was just one "another idea"...
Use it or not - doesn't matter - the decision is on you - because you're the developer.
--
JozefMojzis - 14 Feb 2016
Regarding Meta and MetaCache, there are several other proposals that may be worth reviewing regarding design of these.
There are a lot of other discussions here and there about the Meta design beyond the above. I'm not suggesting that any of these get wrapped into your work, but they may explain better some of the thinking behind what's currently implemented and what others see as areas for improvement.
--
GeorgeClark - 14 Feb 2016