Item13897: Working on bugs.

foswiki · Feb 10, 2016 · 6c5d460 · 6c5d460
1 parent 901a48c
commit 6c5d460
Show file tree

Hide file tree

Showing 16 changed files with 257 additions and 219 deletions.
diff --git a/core/lib/Foswiki/Form.pm b/core/lib/Foswiki/Form.pm
@@ -115,8 +115,8 @@ sub _validateWebTopic {
         Foswiki::OopsException->throw(
             template => 'attention',
             def      => 'invalid_form_name',
-            web      => $session->{webName},
-            topic    => $session->{topicName},
+            web      => $session->webName,
+            topic    => $session->topicName,
             params   => [ $web, $form ]
         );
     }
@@ -134,7 +134,7 @@ sub loadCached {
     my ( $vweb, $vtopic ) = _validateWebTopic( $session, $web, $form );
 
     my $this;
-    if ( defined( $this = $session->{forms}->{"$vweb.$vtopic"} ) ) {
+    if ( defined( $this = $session->forms->{"$vweb.$vtopic"} ) ) {
         unless ( $this->isa('Foswiki::Form') ) {
 
             #recast if we have to - allowing the cache to work its magic
@@ -177,8 +177,8 @@ around BUILDARGS => sub {
         Foswiki::OopsException->throw(
             'attention',
             def    => 'no_form_def',
-            web    => $session->{webName},
-            topic  => $session->{topicName},
+            web    => $session->webName,
+            topic  => $session->topicName,
             params => [ $vweb, $vtopic ]
         );
     }
@@ -194,13 +194,13 @@ sub BUILD {
     my $topic   = $this->topic;
 
     unless ( $this->has_def || $this->haveAccess('VIEW') ) {
-        Foswiki::AccessControlException->throw( 'VIEW', $session->{user},
+        Foswiki::AccessControlException->throw( 'VIEW', $session->user,
             $web, $topic, $Foswiki::Meta::reason );
     }
 
     # cache the object before we've parsed it to prevent recursion
     #when there are SEARCH / INCLUDE macros in the form definition
-    $session->{forms}->{"$web.$topic"} = $this;
+    $session->forms->{"$web.$topic"} = $this;
 
     unless ( $this->has_def ) {
         $this->fields( $this->_parseFormDefinition() );
@@ -610,7 +610,7 @@ sub renderForEdit {
             # Give plugin field types a chance first (but no chance to add to
             # col 0 :-(
             # SMELL: assumes that the field value is a string
-            my $output = $session->{plugins}->dispatch(
+            my $output = $session->plugins->dispatch(
                 'renderFormFieldForEditHandler', $fieldDef->{name},
                 $fieldDef->{type},               $fieldDef->{size},
                 $value,                          $fieldDef->{attributes},

diff --git a/core/lib/Foswiki/Iterator/FilterIterator.pm b/core/lib/Foswiki/Iterator/FilterIterator.pm
@@ -27,17 +27,16 @@ BEGIN {
 has iterator => (
     is       => 'rw',
     required => 1,
-    weak_ref => 1,
     isa      => Foswiki::Object::isaCLASS(
         'iterator', 'Foswiki::Object', does => 'Foswiki::Iterator',
     ),
 );
-has data    => ( is => 'rw', required => 1, );
-has pending => ( is => 'rw', default  => 0, );
+has data => ( is => 'rw', );
+has pending => ( is => 'rw', default => 0, );
 
 =begin TML
 
----++ ClassMethod new( iterator => $iter, filter => $sub, data => $data )
+---++ ClassMethod new( iterator => $iter, filter => $sub, [data => $data] )
 Construct a new iterator that will filter $iter based on the results from
 $sub. $sub should return 0 if the next() from $iter should be filtered and
 1 if it should be treated as the next item in the sequence.

diff --git a/core/lib/Foswiki/Iterator/PagerIterator.pm b/core/lib/Foswiki/Iterator/PagerIterator.pm
@@ -26,7 +26,6 @@ BEGIN {
 has iterator => (
     is       => 'rw',
     required => 1,
-    weak_ref => 1,
     isa      => Foswiki::Object::isaCLASS(
         'iterator', 'Foswiki::Object', does => 'Foswiki::Iterator',
     ),

diff --git a/core/lib/Foswiki/Meta.pm b/core/lib/Foswiki/Meta.pm
@@ -952,8 +952,7 @@ sub populateNewWeb {
     }
 
     if ($templateWeb) {
-        my $tWebObject =
-          $this->new( sesssion => $session, web => $templateWeb );
+        my $tWebObject = $this->new( session => $session, web => $templateWeb );
         require Foswiki::WebFilter;
         my $sys =
           Foswiki::WebFilter->new('template')->ok( $session, $templateWeb );
@@ -1102,7 +1101,7 @@ sub eachTopic {
 
         # Root
         require Foswiki::ListIterator;
-        return new Foswiki::ListIterator( [] );
+        return Foswiki::ListIterator->new( list => [] );
     }
     return $this->session->store->eachTopic($this);
 }
@@ -2047,7 +2046,7 @@ sub save {
         if ( $text ne $pretext ) {
 
             # Create a new object to parse the changed text
-            my $after = new Foswiki::Meta(
+            my $after = Foswiki::Meta->new(
                 session => $this->session,
                 web     => $this->web,
                 topic   => $this->topic,

diff --git a/core/lib/Foswiki/Prefs/Stack.pm b/core/lib/Foswiki/Prefs/Stack.pm
@@ -296,14 +296,9 @@ sub clone {
 
     my $clone = $this->new;
 
-    # SMELL vrurg Not sure but Storable would do the job faster and more
-    # reliable with respect to avoiding of circular references.
-    #$clone->map( { %{ $this->map } } );
-    #$clone->levels( [ @{ $this->levels } ] );
-    #$clone->final( { %{ $this->final } } );
-    $clone->map( dclone( $this->map ) );
-    $clone->levels( dclone( $this->levels ) );
-    $clone->final( dclone( $this->final ) );
+    $clone->map( { %{ $this->map } } );
+    $clone->levels( [ @{ $this->levels } ] );
+    $clone->final( { %{ $this->final } } );
     $clone->restore($level) if defined $level;
 
     return $clone;

diff --git a/core/lib/Foswiki/Search.pm b/core/lib/Foswiki/Search.pm
@@ -266,7 +266,8 @@ sub searchWeb {
     ASSERT( defined $session->webName ) if DEBUG;
     my %params = @_;
 
-    my $baseWebObject = Foswiki::Meta->new( $session, $session->webName );
+    my $baseWebObject =
+      Foswiki::Meta->new( session => $session, web => $session->webName );
 
     my ( $callback, $cbdata ) = setup_callback( \%params, $baseWebObject );
 
@@ -772,8 +773,8 @@ sub formatResults {
     my $nhits      = 0;         # number of hits (if multiple=on) in current web
     my $headerDone = $noHeader;
 
-    my $web              = $baseWeb;
-    my $webObject        = new Foswiki::Meta( $session, $web );
+    my $web = $baseWeb;
+    my $webObject = Foswiki::Meta->new( session => $session, web => $web );
     my $lastWebProcessed = '';
 
     #total number of topics and hits - not reset when we swap webs
@@ -810,7 +811,11 @@ sub formatResults {
 
 #TODO: OMG! Search.pm relies on Meta::load (in the metacache) returning a meta object even when the topic does not exist.
 #lets change that
-                $topicMeta = new Foswiki::Meta( $session, $web, $topic );
+                $topicMeta = Foswiki::Meta->new(
+                    session => $session,
+                    web     => $web,
+                    topic   => $topic
+                );
             }
             $info = $this->metacache->get( $web, $topic, $topicMeta );
             ASSERT( defined( $info->{tom} ) ) if DEBUG;
@@ -941,7 +946,8 @@ sub formatResults {
             }
 
             if ( $lastWebProcessed ne $web ) {
-                $webObject = new Foswiki::Meta( $session, $web );
+                $webObject =
+                  Foswiki::Meta->new( session => $session, web => $web );
                 $lastWebProcessed = $web;
 
                 #reset our web partitioned legacy counts

diff --git a/core/lib/Foswiki/Search/InfoCache.pm b/core/lib/Foswiki/Search/InfoCache.pm
@@ -56,10 +56,11 @@ Because this Iterator can be created and filled dynamically, once the Iterator h
 #TODO: or..... make reset() make the object mutable again, so we can change the elements in the list, but re-use the meta cache??
 #CONSIDER: convert the internals to a hash[tomAddress] = {matches->[list of resultint text bits], othermeta...} - except this does not give us order :/
 
-has _session => (
+has session => (
     is       => 'rw',
     weak_ref => 1,
     init_arg => 'session',
+    required => 1,
 );
 has _defaultWeb => (
     is       => 'rw',
@@ -124,10 +125,10 @@ sub addTopic {
 
     my ( $w, $t ) = Foswiki::Func::normalizeWebTopicName( $web, $topic );
     my $webtopic = "$w.$t";
-    push( @{ $this->list }, $webtopic );
     $this->count( $this->count + 1 );
+    push( @{ $this->list }, $webtopic );
     if ( defined($meta) ) {
-        $this->_session->search->metacache->addMeta( $web, $topic, $meta );
+        $this->session->search->metacache->addMeta( $web, $topic, $meta );
     }
     $this->clear_sorted;
 }
@@ -163,7 +164,7 @@ sub sortResults {
     return if ( $this->sorted );
     $this->sorted(1);
 
-    my $session = $this->_session;
+    my $session = $this->session;
 
     my $sortOrder = $params->{order} || '';
     my $revSort   = Foswiki::isTrue( $params->{reverse} );
@@ -508,8 +509,10 @@ sub getTopicListIterator {
         $it = $webObject->eachTopic();
     }
 
-    return Foswiki::Iterator::FilterIterator->new( $it,
-        getOptionFilter($options) );
+    return Foswiki::Iterator::FilterIterator->new(
+        iterator => $it,
+        filter   => getOptionFilter($options)
+    );
 }
 
 sub convertTopicPatternToRegex {

diff --git a/core/lib/Foswiki/Search/Node.pm b/core/lib/Foswiki/Search/Node.pm
@@ -1,5 +1,6 @@
 # See bottom of file for license and copyright information
 package Foswiki::Search::Node;
+use v5.14;
 
 =begin TML
 
@@ -9,11 +10,8 @@ Refactoring mid-step that contains a set of SEARCH tokens and options.
 
 =cut
 
-use strict;
-use warnings;
-
 use Assert;
-use Error qw( :try );
+use Try::Tiny;
 
 BEGIN {
     if ( $Foswiki::cfg{UseLocale} ) {
@@ -22,6 +20,10 @@ BEGIN {
     }
 }
 
+use Moo;
+use namespace::clean;
+extends qw(Foswiki::Object);
+
 # Some day this may usefully be an infix node
 #use Foswiki::Infix::Node ();
 #our @ISA = ('Foswiki::Infix::Node');
@@ -34,18 +36,19 @@ Construct a search token container.
 
 =cut
 
-sub new {
-    my ( $class, $search, $tokens, $options ) = @_;
-    my $this = bless(
-        {
-            tokens  => $tokens,
-            search  => $search,
-            options => $options,
-        },
-        $class
-    );
-    return $this;
-}
+has tokens => (
+    is       => 'ro',
+    required => 1,
+    default  => sub { [] },
+);
+has search => (
+    is       => 'ro',
+    required => 1,
+);
+has options => (
+    is       => 'ro',
+    required => 1,
+);
 
 =begin TML
 
@@ -55,11 +58,11 @@ Return a ref to a list of tokens that are ANDed to perform the search.
 
 =cut
 
-sub tokens {
-    my $this = shift;
-    return [] unless $this->{tokens};
-    return $this->{tokens};
-}
+#sub tokens {
+#    my $this = shift;
+#    return [] unless $this->tokens;
+#    return $this->tokens;
+#}
 
 =begin TML
 
@@ -71,16 +74,17 @@ Return true if this search is empty (has no tokens)
 
 sub isEmpty {
     my $this = shift;
-    return !( $this->{tokens} && scalar( @{ $this->{tokens} } ) > 0 );
+    return !( $this->tokens && scalar( @{ $this->tokens } ) > 0 );
 }
 
 sub stringify {
-    my $this = shift;
+    my $this    = shift;
+    my $options = $this->options;
     return
-      join( ' ', @{ $this->{tokens} } ) . ' {'
+      join( ' ', @{ $this->tokens } ) . ' {'
       . join( ',',
-        map  { "$_=>$this->{options}->{$_}" }
-        grep { !/^_/ } keys %{ $this->{options} } )
+        map  { "$_=>$options->{$_}" }
+        grep { !/^_/ } keys %{ $this->options } )
       . '}';
 }
 

diff --git a/core/lib/Foswiki/Search/Parser.pm b/core/lib/Foswiki/Search/Parser.pm
@@ -140,7 +140,11 @@ sub parse {
           split( /\s+/, $searchString ); # split on spaces
     }
 
-    my $result = new Foswiki::Search::Node( $searchString, \@tokens, $options );
+    my $result = Foswiki::Search::Node->new(
+        search  => $searchString,
+        tokens  => \@tokens,
+        options => $options
+    );
     return $result;
 }