Skip to content

Commit

Permalink
Item15206: more decruftification of MetaCache
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelDaum committed Aug 23, 2023
1 parent 8ff7693 commit ae6be61
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 59 deletions.
80 changes: 39 additions & 41 deletions core/lib/Foswiki/Meta.pm
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,6 @@ sub load {
$this = $proto->new( $session, $web, $topic );
}

my $session = $this->{_session};

my $useMetaCache = 0;
if (USE_META_CACHE) {
if ( defined( $this->topic )
Expand All @@ -460,7 +458,8 @@ sub load {
{
$useMetaCache = 1;

my $meta = $session->metaCache->getMeta( $this->web, $this->topic );
my $meta =
$this->session->metaCache->getMeta( $this->web, $this->topic );

#print STDERR "found meta in cache".$this->getPath()."\n";
return $meta if defined $meta;
Expand All @@ -485,7 +484,7 @@ sub load {
ASSERT( defined( $this->{_latestIsLoaded} ) ) if DEBUG;
}

$session->metaCache->addMeta( $this->web, $this->topic, $this )
$this->session->metaCache->addMeta( $this->web, $this->topic, $this )
if $useMetaCache;

return $this;
Expand All @@ -504,8 +503,10 @@ which may have surprising effects on other code that shares the object.
sub unload {
my $this = shift;

$this->{_session}->metaCache->removeMeta( $this->web, $this->topic )
if $this->{_session};
return
unless defined $this->{_session}; # trying to unload the same thing twice?

$this->session->metaCache->removeMeta( $this->web, $this->topic );
$this->{_loadedRev} = undef;
$this->{_latestIsLoaded} = undef;
$this->{_text} = undef;
Expand Down Expand Up @@ -570,7 +571,10 @@ it was created.
=cut

sub session {
return $_[0]->{_session};
my $this = shift;

$this->{_session} ||= $Foswiki::Plugins::SESSION;
return $this->{_session};
}

# Assert helpers
Expand Down Expand Up @@ -662,10 +666,10 @@ sub isSessionTopic {
return 0
unless defined $this->{_web}
&& defined $this->{_topic}
&& defined $this->{_session}->{webName}
&& defined $this->{_session}->{topicName};
return $this->{_web} eq $this->{_session}->{webName}
&& $this->{_topic} eq $this->{_session}->{topicName};
&& defined $this->session->{webName}
&& defined $this->session->{topicName};
return $this->{_web} eq $this->session->{webName}
&& $this->{_topic} eq $this->session->{topicName};
}

=begin TML
Expand All @@ -685,13 +689,12 @@ sub getPreference {
my ( $this, $key ) = @_;

unless ( $this->{_web} || $this->{_topic} ) {
return $this->{_session}->{prefs}->getPreference($key);
return $this->session->{prefs}->getPreference($key);
}

# make sure the preferences are parsed and cached
unless ( $this->{_preferences} ) {
$this->{_preferences} =
$this->{_session}->{prefs}->loadPreferences($this);
$this->{_preferences} = $this->session->{prefs}->loadPreferences($this);
}
return unless $this->{_preferences};
return $this->{_preferences}->get($key);
Expand All @@ -709,10 +712,10 @@ sub getContainer {
my $this = shift;

if ( $this->{_topic} ) {
return Foswiki::Meta->new( $this->{_session}, $this->{_web} );
return Foswiki::Meta->new( $this->session, $this->{_web} );
}
if ( $this->{_web} ) {
return Foswiki::Meta->new( $this->{_session} );
return Foswiki::Meta->new( $this->session );
}
ASSERT( 0, 'no container for this object type' ) if DEBUG;
return;
Expand All @@ -735,11 +738,11 @@ sub existsInStore {
# only checking for a topic existence already establishes a dependency
$this->addDependency();

return $this->{_session}->{store}
return $this->session->{store}
->topicExists( $this->{_web}, $this->{_topic} );
}
elsif ( defined $this->{_web} ) {
return $this->{_session}->{store}->webExists( $this->{_web} );
return $this->session->{store}->webExists( $this->{_web} );
}
else {
return 1; # the root always exists
Expand Down Expand Up @@ -782,7 +785,7 @@ See Foswiki::PageCache::addDependency().
=cut

sub addDependency {
my $cache = $_[0]->{_session}->{cache};
my $cache = $_[0]->session->{cache};
return unless $cache;
return $cache->addDependency( $_[0]->{_web}, $_[0]->{_topic} );
}
Expand All @@ -797,7 +800,7 @@ within the Foswiki::PageCache. See Foswiki::PageCache::fireDependency().
=cut

sub fireDependency {
my $cache = $_[0]->{_session}->{cache};
my $cache = $_[0]->session->{cache};
return unless $cache;
return $cache->fireDependency( $_[0]->{_web}, $_[0]->{_topic} );
}
Expand All @@ -815,7 +818,7 @@ sub isCacheable {

return 0 unless $Foswiki::cfg{Cache}{Enabled};

my $cache = $this->{_session}->{cache};
my $cache = $this->session->{cache};
return 0 unless $cache;

return $cache->isCacheable( $this->{_web}, $this->{_topic}, $value );
Expand Down Expand Up @@ -848,8 +851,6 @@ sub populateNewWeb {
my ( $this, $templateWeb, $opts ) = @_;
_assertIsWeb($this) if DEBUG;

my $session = $this->{_session};

my ( $parent, $new ) = $this->{_web} =~ m/^(.*)\/([^\.\/]+)$/;

if ($parent) {
Expand All @@ -859,14 +860,14 @@ sub populateNewWeb {
. ' - Hierarchical webs are disabled' );
}

unless ( $session->webExists($parent) ) {
unless ( $this->session->webExists($parent) ) {
throw Error::Simple( 'Parent web ' . $parent . ' does not exist' );
}
}

# Validate that template web exists, or error should be thrown
if ($templateWeb) {
unless ( $session->webExists($templateWeb) ) {
unless ( $this->session->webExists($templateWeb) ) {
throw Error::Simple(
'Template web ' . $templateWeb . ' does not exist' );
}
Expand All @@ -875,7 +876,7 @@ sub populateNewWeb {
# Make sure there is a preferences topic; this is how we know it's a web
my $prefsTopicObject;
if (
!$session->topicExists(
!$this->session->topicExists(
$this->{_web}, $Foswiki::cfg{WebPrefsTopicName}
)
)
Expand All @@ -888,10 +889,11 @@ sub populateNewWeb {
}

if ($templateWeb) {
my $tWebObject = $this->new( $session, $templateWeb );
my $tWebObject = $this->new( $this->session, $templateWeb );
require Foswiki::WebFilter;
my $sys =
Foswiki::WebFilter->new('template')->ok( $session, $templateWeb );
Foswiki::WebFilter->new('template')
->ok( $this->session, $templateWeb );
my $it = $tWebObject->eachTopic();
while ( $it->hasNext() ) {
my $topic = $it->next();
Expand All @@ -906,7 +908,7 @@ sub populateNewWeb {
$attfh{ $sfa->{name} } = {
fh => $fh,
date => $sfa->{date},
user => $sfa->{user} || $session->{user},
user => $sfa->{user} || $this->session->{user},
comment => $sfa->{comment}
};
}
Expand All @@ -918,7 +920,7 @@ sub populateNewWeb {

# copy fileattachments
while ( my ( $fa, $sfa ) = each %attfh ) {
my $arev = $session->{store}->saveAttachment(
my $arev = $this->session->{store}->saveAttachment(
$to, $fa,
$sfa->{fh},
$sfa->{user},
Expand Down Expand Up @@ -1937,10 +1939,8 @@ sub haveAccess {
$mode ||= 'VIEW';
$cUID ||= $this->{_session}->{user};

my $session = $this->{_session};

my $ok = $session->access->haveAccess( $mode, $cUID, $this );
$reason = $session->access->getReason();
my $ok = $this->session->access->haveAccess( $mode, $cUID, $this );
$reason = $this->session->access->getReason();
return $ok;
}

Expand Down Expand Up @@ -3560,8 +3560,7 @@ sub _summariseTextSimple {
sub _makeSummaryTextSafe {
my ( $this, $text ) = @_;

my $session = $this->session();
my $renderer = $session->renderer();
my $renderer = $this->session->renderer();

# We do not want the summary to contain any $variable that formatted
# searches can interpret to anything (Item3489).
Expand Down Expand Up @@ -3699,8 +3698,7 @@ In non-tml, lines are truncated to 70 characters. Differences are shown using +
sub summariseChanges {
my ( $this, $orev, $nrev, $tml, $nochecks ) = @_;
my $summary = '';
my $session = $this->session();
my $renderer = $session->renderer();
my $renderer = $this->session->renderer();

_assertIsTopic($this) if DEBUG;
$nrev = $this->getLatestRev() unless $nrev;
Expand Down Expand Up @@ -3740,12 +3738,12 @@ sub summariseChanges {
#print "SSSSSS ntext\n($ntext)\nSSSSSS\n\n";

my $oldTopicObject =
Foswiki::Meta->load( $session, $this->web, $this->topic, $orev );
Foswiki::Meta->load( $this->session, $this->web, $this->topic, $orev );
unless ( $nochecks || $oldTopicObject->haveAccess('VIEW') ) {

# No access to old rev, make a blank topic object
$oldTopicObject =
Foswiki::Meta->new( $session, $this->web, $this->topic, '' );
Foswiki::Meta->new( $this->session, $this->web, $this->topic, '' );
}

my $ostring = $oldTopicObject->stringify();
Expand Down Expand Up @@ -3782,7 +3780,7 @@ sub summariseChanges {
$block =~ s/^-(.*)$/CGI::del( {}, $1 )/se;
$block =~ s/^\+(.*)$/CGI::ins( {}, $1 )/se;
}
elsif ( $session->inContext('rss') ) {
elsif ( $this->session->inContext('rss') ) {
$block =~ s/^-/REMOVED: /;
$block =~ s/^\+/INSERTED: /;
}
Expand Down
19 changes: 1 addition & 18 deletions core/lib/Foswiki/MetaCache.pm
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,8 @@ sub get {
$meta = $m if ( defined($m) );
ASSERT( defined($meta) ) if DEBUG;

my $info = { tom => $meta };
my $info = $this->{cache}{$web}{$topic} // { tom => $meta };

$info = $this->{cache}{$web}{$topic}
if defined( $this->{cache}{$web}{$topic} );
if ( not defined( $info->{editby} ) ) {

#TODO: extract this to the Meta Class, or remove entirely
Expand All @@ -194,21 +192,6 @@ sub get {
$info->{editby} = $ri->{author} || '';
$info->{modified} = $ri->{date};
$info->{revNum} = $ri->{version};

#TODO: this is _not_ actually sufficient.. as there are other things that appear to be evaluated in turn
#Ideally, the Store2::Meta object will _not_ contain any session info, and anything that is session / user oriented gets stored in another object that links to the 'database' object.
#it'll probably be better to make the MetaCache know what
#Item10097: make the cache multi-user safe by storing the haveAccess on a per user basis
if ( not defined( $info->{ $this->{session}->{user} } ) ) {
$info->{ $this->{session}->{user} } = ();
}
if ( not defined( $info->{ $this->{session}->{user} }{allowView} ) ) {
$info->{ $this->{session}->{user} }{allowView} =
$info->{tom}->haveAccess('VIEW');
}

#use the cached permission
$info->{allowView} = $info->{ $this->{session}->{user} }{allowView};
}

return $info;
Expand Down

0 comments on commit ae6be61

Please sign in to comment.