Skip to content

Commit

Permalink
Item13897: Intermidiate commit
Browse files Browse the repository at this point in the history 
Many parts are to be Moo-fied before tests would run again.

Most notable change is that Foswiki::Form object has now be created by
means of method load() instead of new(). This is because generally it's not
a very good idea to overload Moo's new() and there is no other way to
bypass the object construction and fetch a form from the cache if it's been
loaded beforehand. Generally it's makes sense as previously
Foswiki::Form->new() call was not necessarily creating a new object after
all. Whereas load() does load a form either from disk or from the cache.
  • Loading branch information
@vrurg
vrurg committed Jan 27, 2016
1 parent 2897df2 commit f7b7053
Show file tree
Hide file tree
Showing 9 changed files with 234 additions and 148 deletions.
3 changes: 2 additions & 1 deletion UnitTestContrib/test/unit/FormDefTests.pm
View file
Expand Up @@ -245,7 +245,8 @@ FORM
FORM
$topicObject->save();
$topicObject->finish();
my $def = new Foswiki::Form( $this->session, $this->test_web, 'TestForm' );
my $def =
Foswiki::Form->load( $this->session, $this->test_web, 'TestForm' );

$this->assert_equals( 1, scalar @{ $def->getFields() } );
my $f = $def->getField('Ecks');
Expand Down
2 changes: 1 addition & 1 deletion core/lib/Foswiki/Compatibility.pm
View file
Expand Up @@ -178,7 +178,7 @@ sub upgradeCategoryTable {
}

require Foswiki::Form;
my $def = new Foswiki::Form( $session, $web, $defaultFormTemplate );
my $def = Foswiki::Form->load( $session, $web, $defaultFormTemplate );
$meta->put( 'FORM', { name => $defaultFormTemplate } );

foreach my $fieldDef ( @{ $def->getFields() } ) {
Expand Down
211 changes: 141 additions & 70 deletions core/lib/Foswiki/Form.pm
View file
Expand Up @@ -64,9 +64,24 @@ my %valid_columns = map { $_ => 1 } @default_columns;

our @_newParameters = qw(session web form def);

has def => (
is => 'ro',
lazy => 1,
predicate => 1,
);
has fields => (
is => 'rw',
lazy => 1,
);
has mandatoryFieldsPresent => (
is => 'rw',
lazy => 1,
default => 0,
);

=begin TML
---++ ClassMethod new ( $session, $web, $topic, \@def )
---++ ClassMethod load ( $session, $web, $topic, \@def )
Looks up a form in the session object or, if it hasn't been read yet,
reads it from the form definition topic on disc.
Expand All @@ -84,71 +99,115 @@ in the database is protected against view.
=cut

sub new {
my ( $class, $session, $web, $form, $def ) = @_;
sub _validateWebTopic {
my ( $session, $web, $form ) = @_;

my ( $vweb, $vtopic ) = $session->normalizeWebTopicName( $web, $form );
my $this = $session->{forms}->{"$vweb.$vtopic"};

unless ($this) {

# A form name has to be a valid topic name after normalisation
$vweb = Foswiki::Sandbox::untaint( $vweb,
\&Foswiki::Sandbox::validateWebName );
$vtopic = Foswiki::Sandbox::untaint( $vtopic,
\&Foswiki::Sandbox::validateTopicName );
unless ( $vweb && $vtopic ) {
throw Foswiki::OopsException(
'attention',
def => 'invalid_form_name',
web => $session->{webName},
topic => $session->{topicName},
params => [ $web, $form ]
);
}

# Got to have either a def or a topic
unless ( $def || $session->topicExists( $vweb, $vtopic ) ) {
throw Foswiki::OopsException(
'attention',
def => 'no_form_def',
web => $session->{webName},
topic => $session->{topicName},
params => [ $vweb, $vtopic ]
);
}
# Validating web/topic before usage.
$vweb =
Foswiki::Sandbox::untaint( $vweb, \&Foswiki::Sandbox::validateWebName );
$vtopic = Foswiki::Sandbox::untaint( $vtopic,
\&Foswiki::Sandbox::validateTopicName );

unless ( $vweb && $vtopic ) {
Foswiki::OopsException->throw(
template => 'attention',
def => 'invalid_form_name',
web => $session->{webName},
topic => $session->{topicName},
params => [ $web, $form ]
);
}

$this = $class->SUPER::new( $session, $vweb, $vtopic );
return ( $vweb, $vtopic );
}

unless ( $def || $this->haveAccess('VIEW') ) {
throw Foswiki::AccessControlException( 'VIEW', $session->{user},
$vweb, $vtopic, $Foswiki::Meta::reason );
}
# XXX vrurg ClassMethod load() is supposed to replace the old new() method and
# become a new constructor. Required to stay in compliance with Moo architecture
# and avoid replacing of the standard new() method.
sub load {
my ( $class, $session, $web, $form, $def ) = @_;

my ( $vweb, $vtopic ) = _validateWebTopic( $session, $web, $form );

if ( ref($this) ne 'Foswiki::Form' ) {
my $this;
if ( defined( $this = $session->{forms}->{"$vweb.$vtopic"} ) ) {
unless ( $this->isa('Foswiki::Form') ) {

#recast if we have to - allowing the cache to work its magic
$this = bless( $this, 'Foswiki::Form' );
$this = bless $this, $class;
}
}

# cache the object before we've parsed it to prevent recursion
#when there are SEARCH / INCLUDE macros in the form definition
$session->{forms}->{"$vweb.$vtopic"} = $this;
return $this // $class->new(
session => $session,
web => $vweb,
topic => $vtopic,
_via_load => 1,
( defined $def ? ( def => $def ) : () ),
);
}

unless ($def) {
$this->{fields} = $this->_parseFormDefinition();
}
elsif ( ref($def) eq 'ARRAY' ) {
$this->{fields} = $def;
}
else {
around BUILDARGS => sub {
my $orig = shift;
my $class = shift;

# Foswiki::Meta object
$this->{fields} = $this->_extractPseudoFieldDefs($def);
}
my $params = $orig->( $class, @_ );

my $session = $params->{session};
my ( $vweb, $vtopic ) =
_validateWebTopic( $session, $params->{web}, $params->{form} );

# Avoid direct calls to $class::new().
ASSERT( $params->{_via_load},
"${class}::new() has been use directly. Use ${class}::load() instead."
);

# No more need to pollute properties with this key.
delete $params->{_via_load};

# Got to have either a def or a topic
unless ( $params->{def} || $session->topicExists( $vweb, $vtopic ) ) {
Foswiki::OopsException->throw(
'attention',
def => 'no_form_def',
web => $session->{webName},
topic => $session->{topicName},
params => [ $vweb, $vtopic ]
);
}

return $params;
};

sub BUILD {
my $this = shift;

my $session = $this->session;
my $web = $this->web;
my $topic = $this->topic;

unless ( $this->has_def || $this->haveAccess('VIEW') ) {
Foswiki::AccessControlException->throw( 'VIEW', $session->{user},
$web, $topic, $Foswiki::Meta::reason );
}

return $this;
# 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;

unless ( $this->has_def ) {
$this->fields( $this->_parseFormDefinition() );
}
elsif ( ref( $this->def ) eq 'ARRAY' ) {
$this->fields( $this->def );
}
else {

# Foswiki::Meta object
$this->fields( $this->_extractPseudoFieldDefs( $this->def ) );
}
}

=begin TML
Expand All @@ -161,14 +220,18 @@ Break circular references.
# Note to developers; please undef *all* fields in the object explicitly,
# whether they are references or not. That way this method is "golden
# documentation" of the live fields in the object.
sub finish {
around finish => sub {
my $orig = shift;
my $this = shift;
foreach ( @{ $this->{fields} } ) {
$_->finish();
}
undef $this->{fields};
$this->SUPER::finish();
}

# vrurg The following commented out code shouldn't be needed anymore. Though
# I'd keep it here as a reminder.
#foreach ( @{ $this->fields } ) {
# $_->finish();
#}
$this->clear_fields;
$orig->($this);
};

=begin TML
Expand Down Expand Up @@ -370,16 +433,24 @@ sub _parseFormDefinition {
push( @fields, $fieldDef );
%field = ();

$this->{mandatoryFieldsPresent} ||= $fieldDef->isMandatory();
$this->mandatoryFieldsPresent( $this->mandatoryFieldsPresent
|| $fieldDef->isMandatory() );
}
};

try {
Foswiki::Tables::Parser::parse( $text, $handler );
}
catch Foswiki::Form::ParseFinished with {
catch {

if ( $_->isa('Foswiki::Form::ParseFinished ') ) {

# clean exit, fired when first table has been parsed
}
else {
Foswiki::Exception->rethrow($_);
}

# clean exit, fired when first table has been parsed
};

return \@fields;
Expand Down Expand Up @@ -470,7 +541,7 @@ sub _link {
sub stringify {
my $this = shift;
my $fs = "| *Name* | *Type* | *Size* | *Attributes* |\n";
foreach my $fieldDef ( @{ $this->{fields} } ) {
foreach my $fieldDef ( @{ $this->fields } ) {
$fs .= $fieldDef->stringify();
}
return $fs;
Expand All @@ -493,7 +564,7 @@ sub renderForEdit {
require CGI;
my $session = $this->session;

if ( $this->{mandatoryFieldsPresent} ) {
if ( $this->mandatoryFieldsPresent ) {
$session->enterContext('mandatoryfields');
}
my $tmpl = $session->templates->readTemplate('form');
Expand All @@ -503,7 +574,7 @@ sub renderForEdit {
my ( $text, $repeatTitledText, $repeatUntitledText, $afterText ) =
split( /%REPEAT%/, $tmpl );

foreach my $fieldDef ( @{ $this->{fields} } ) {
foreach my $fieldDef ( @{ $this->fields } ) {

my $value;
my $tooltip = $fieldDef->{description};
Expand Down Expand Up @@ -586,7 +657,7 @@ sub renderHidden {

my $text = '';

foreach my $field ( @{ $this->{fields} } ) {
foreach my $field ( @{ $this->fields } ) {
$text .= $field->renderHidden($topicObject);
}

Expand Down Expand Up @@ -622,7 +693,7 @@ sub getFieldValuesFromQuery {
# order in the previous meta object. See Item1982.
my @old = $topicObject->find('FIELD');
$topicObject->remove('FIELD');
foreach my $fieldDef ( @{ $this->{fields} } ) {
foreach my $fieldDef ( @{ $this->fields } ) {
my ( $set, $present ) =
$fieldDef->populateMetaFromQueryData( $query, $topicObject, \@old );
if ($present) {
Expand Down Expand Up @@ -674,7 +745,7 @@ define the field.

sub getField {
my ( $this, $name ) = @_;
foreach my $fieldDef ( @{ $this->{fields} } ) {
foreach my $fieldDef ( @{ $this->fields } ) {
return $fieldDef if ( $fieldDef->{name} && $fieldDef->{name} eq $name );
}
return;
Expand All @@ -693,7 +764,7 @@ returned list should be treated as *read only* (must not be written to).

sub getFields {
my $this = shift;
return $this->{fields};
return $this->fields;
}

sub renderForDisplay {
Expand All @@ -707,7 +778,7 @@ sub renderForDisplay {

my $rowTemplate = $templates->expandTemplate('FORM:display:row');
my $hasAllFieldsHidden = 1;
foreach my $fieldDef ( @{ $this->{fields} } ) {
foreach my $fieldDef ( @{ $this->fields } ) {
my $fm = $topicObject->get( 'FIELD', $fieldDef->{name} );
next unless $fm;
my $fa = $fieldDef->{attributes} || '';
Expand Down

0 comments on commit f7b7053

Please sign in to comment.