Mailing List Archive: Bricolage: devel

Patch to fix accessors in Bric::Biz::Element::Field

Index | Next | Previous | View Threaded

adrian at gossamer-threads

Feb 6, 2009, 10:05 AM

Post #1 of 8 (1997 views)
Permalink

Patch to fix accessors in Bric::Biz::Element::Field

Hi,

While figuring things out for the in-place editor, I ran into what seems like a
bug where Bric::Biz::Element::Field->get_max_length was returning $self instead
of the max_length. I took a look at the SVN log, but there wasn't a message
relating to why it was added:

http://viewsvn.bricolage.cc/bricolage/trunk/lib/Bric/Biz/Element/Field.pm?view=diff&r1=6698&r2=6699

Here's the patch, essentially removing the bit of code and relying on the built
in accessors:

***** start patch *****
Index: lib/Bric/Biz/Element/Field.pm
===================================================================
--- lib/Bric/Biz/Element/Field.pm (revision 8406)
+++ lib/Bric/Biz/Element/Field.pm (working copy)
@@ -731,12 +731,6 @@

=cut

-sub get_max_length {
- my $self = shift;
- return $self if $self->_get('max_length');
- return;
-}
-
##############################################################################

=item my $sql_type = $data->get_sql_type
***** end patch *****

There are a couple other methods which do the same thing that may want to be
removed/fixed as well: is_autopopulated, is_multiple

Adrian

david at kineticode

Feb 6, 2009, 12:46 PM

Post #2 of 8 (1905 views)
Permalink

Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

On Feb 6, 2009, at 10:05 AM, Adrian Yee wrote:

> Hi,
>
> While figuring things out for the in-place editor, I ran into what
> seems like a
> bug where Bric::Biz::Element::Field->get_max_length was returning
> $self instead
> of the max_length. I took a look at the SVN log, but there wasn't a
> message
> relating to why it was added:
>
> http://viewsvn.bricolage.cc/bricolage/trunk/lib/Bric/Biz/Element/Field.pm?view=diff&r1=6698&r2=6699
>
> Here's the patch, essentially removing the bit of code and relying
> on the built
> in accessors:
>
> ***** start patch *****
> Index: lib/Bric/Biz/Element/Field.pm
> ===================================================================
> --- lib/Bric/Biz/Element/Field.pm (revision 8406)
> +++ lib/Bric/Biz/Element/Field.pm (working copy)
> @@ -731,12 +731,6 @@
>
> =cut
>
> -sub get_max_length {
> - my $self = shift;
> - return $self if $self->_get('max_length');
> - return;
> -}
> -
> ##############################################################################
>
> =item my $sql_type = $data->get_sql_type
> ***** end patch *****
>
> There are a couple other methods which do the same thing that may
> want to be
> removed/fixed as well: is_autopopulated, is_multiple

Those should work that way; they're booleans.

Are you running the test suite as you do this stuff? See Bric::Hacker
for details. You should be adding new tests, too, eh?

Thanks,

David

david at kineticode

Mar 11, 2009, 10:32 AM

Post #3 of 8 (1759 views)
Permalink

Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

Hi Adrian,

Sorry, this fell through the cracks, though I know we did discuss it
when I reviewed your patch. Please do feel free to change these
methods, provided all tests continue to pass. Have you managed to get
tests passing yet?

Thanks,

David

On Feb 6, 2009, at 10:05 AM, Adrian Yee wrote:

> Hi,
>
> While figuring things out for the in-place editor, I ran into what
> seems like a
> bug where Bric::Biz::Element::Field->get_max_length was returning
> $self instead
> of the max_length. I took a look at the SVN log, but there wasn't a
> message
> relating to why it was added:
>
> http://viewsvn.bricolage.cc/bricolage/trunk/lib/Bric/Biz/Element/Field.pm?view=diff&r1=6698&r2=6699
>
> Here's the patch, essentially removing the bit of code and relying
> on the built
> in accessors:
>
> ***** start patch *****
> Index: lib/Bric/Biz/Element/Field.pm
> ===================================================================
> --- lib/Bric/Biz/Element/Field.pm (revision 8406)
> +++ lib/Bric/Biz/Element/Field.pm (working copy)
> @@ -731,12 +731,6 @@
>
> =cut
>
> -sub get_max_length {
> - my $self = shift;
> - return $self if $self->_get('max_length');
> - return;
> -}
> -
> ##############################################################################
>
> =item my $sql_type = $data->get_sql_type
> ***** end patch *****
>
> There are a couple other methods which do the same thing that may
> want to be
> removed/fixed as well: is_autopopulated, is_multiple
>
> Adrian

adrian at gossamer-threads

Mar 12, 2009, 6:50 PM

Post #4 of 8 (1765 views)
Permalink

Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

> Sorry, this fell through the cracks, though I know we did discuss it
> when I reviewed your patch. Please do feel free to change these methods,
> provided all tests continue to pass. Have you managed to get tests
> passing yet?

Yup, got a dev copy installed and got it going, thanks.

Got some failed tests though:

t/Bric/Test/Runner....NOK 9403/11815

# Failed test 'And it should be the right thing'
# at t/Bric/Util/ApacheReq/Test.pm line 23.
# (in Bric::Util::ApacheReq::Test->test_url)
# got: 'http://www.example.com:8080/'
# expected: 'http://www.example.com/'

Running Bricolage on a different port...

t/Bric/Test/Runner....NOK 10792/11815

# Failed test 'Check parse error payload'
# at t/Bric/Dist/Action/DTDValidate/DevTest.pm line 68.
# (in Bric::Dist::Action::DTDValidate::DevTest->test_do_it)
# '/foo/bad.html:14: parser error : Premature end of
data in tag html line 5
# '
# doesn't match '(?-xism:^/foo/bad\.html:11: (parser )?error ?:
Opening and ending tag mismatch: br line 10 and p)'
t/Bric/Test/Runner....NOK 10806/11815

# Failed test 'Check parse error payload'
# at t/Bric/Dist/Action/DTDValidate/DevTest.pm line 99.
# (in Bric::Dist::Action::DTDValidate::DevTest->test_do_it)
# '/foo/bad.html:14: parser error : Premature end of
data in tag html line 5
# '
# doesn't match '(?-xism:^/foo/bad\.html:11: (parser )?error ?:
Opening and ending tag mismatch: br line 10 and p)'

No clue what these are about.

t/Bric/Test/Runner....NOK 11470/11815

# Failed test 'test_href died (Unable to execute SQL statement:
DBD::Pg::st execute failed: ERROR: duplicate key value violates unique
constraint "udx_server_type__name_site" [for Statement "
# INSERT INTO server_type (id, name, description, site__id,
copyable, publish, preview, active, class__id)
# VALUES (NEXTVAL('seq_server_type'), ?, ?, ?, ?, ?, ?, ?,
(SELECT id FROM class WHERE LOWER(disp_name) = LOWER(?)))
# " with ParamValues: 1='Bogus1', 2='Bogus ServerType1',
3='100', 4='0', 5='1', 6='0', 7='1', 8='File System'] at
lib/Bric/Util/DBI.pm line 1136, <F> line 28.
#
# [lib/Bric/Util/DBI.pm:1136]
# [lib/Bric/Dist/ServerType.pm:2030]
# [t/Bric/Dist/ServerType/DevTest.pm:201]
# [/usr/share/perl5/Test/Class.pm:253]
# [/usr/share/perl5/Test/Class.pm:345]
# [t/Bric/Test/Runner.pm:101]
#
# [lib/Bric/Util/DBI.pm:1137]
# [lib/Bric/Dist/ServerType.pm:2030]
# [t/Bric/Dist/ServerType/DevTest.pm:201]
# [/usr/share/perl5/Test/Class.pm:253]
# [/usr/share/perl5/Test/Class.pm:345]
# [t/Bric/Test/Runner.pm:101])'
# at t/Bric/Test/Runner.pm line 101.
# (in Bric::Dist::ServerType::DevTest->test_href)

These didn't happen the first time around. There's a few more of this
variety.

I'm pretty sure these aren't linked to my accessor change, so I've
committed those changes.

Adrian

david at kineticode

Mar 12, 2009, 7:26 PM

Post #5 of 8 (1748 views)
Permalink

Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

On Mar 12, 2009, at 6:50 PM, Adrian Yee wrote:

>> Sorry, this fell through the cracks, though I know we did discuss
>> it when I reviewed your patch. Please do feel free to change these
>> methods, provided all tests continue to pass. Have you managed to
>> get tests passing yet?
>
> Yup, got a dev copy installed and got it going, thanks.
>
> Got some failed tests though:
>
> t/Bric/Test/Runner....NOK 9403/11815
> # Failed test 'And it should be the right thing'
> # at t/Bric/Util/ApacheReq/Test.pm line 23.
> # (in Bric::Util::ApacheReq::Test->test_url)
> # got: 'http://www.example.com:8080/'
> # expected: 'http://www.example.com/'
>
> Running Bricolage on a different port...

Don't do that. ;-P

> t/Bric/Test/Runner....NOK 10806/11815
> # Failed test 'Check parse error payload'
> # at t/Bric/Dist/Action/DTDValidate/DevTest.pm line 99.
> # (in Bric::Dist::Action::DTDValidate::DevTest->test_do_it)
> # '/foo/bad.html:14: parser error : Premature end
> of data in tag html line 5
> # '
> # doesn't match '(?-xism:^/foo/bad\.html:11: (parser )?error ?:
> Opening and ending tag mismatch: br line 10 and p)'
>
> No clue what these are about.

Make sure you're up-to-date with libxml2 and XML::LibXML.

> t/Bric/Test/Runner....NOK 11470/11815
> # Failed test 'test_href died (Unable to execute SQL statement:
> DBD::Pg::st execute failed: ERROR: duplicate key value violates
> unique constraint "udx_server_type__name_site" [for Statement "
> # INSERT INTO server_type (id, name, description,
> site__id, copyable, publish, preview, active, class__id)
> # VALUES
> (NEXTVAL('seq_server_type'), ?, ?, ?, ?, ?, ?, ?, (SELECT id FROM
> class WHERE LOWER(disp_name) = LOWER(?)))
> # " with ParamValues: 1='Bogus1', 2='Bogus ServerType1',
> 3='100', 4='0', 5='1', 6='0', 7='1', 8='File System'] at lib/Bric/
> Util/DBI.pm line 1136, <F> line 28.
> #
> # [lib/Bric/Util/DBI.pm:1136]
> # [lib/Bric/Dist/ServerType.pm:2030]
> # [t/Bric/Dist/ServerType/DevTest.pm:201]
> # [/usr/share/perl5/Test/Class.pm:253]
> # [/usr/share/perl5/Test/Class.pm:345]
> # [t/Bric/Test/Runner.pm:101]
> #
> # [lib/Bric/Util/DBI.pm:1137]
> # [lib/Bric/Dist/ServerType.pm:2030]
> # [t/Bric/Dist/ServerType/DevTest.pm:201]
> # [/usr/share/perl5/Test/Class.pm:253]
> # [/usr/share/perl5/Test/Class.pm:345]
> # [t/Bric/Test/Runner.pm:101])'
> # at t/Bric/Test/Runner.pm line 101.
> # (in Bric::Dist::ServerType::DevTest->test_href)
>
> These didn't happen the first time around. There's a few more of
> this variety.

Huh. That's not good. Sounds like there are some records that aren't
getting deleted from the database. Which is odd. Please look and see
if you have any records in the server_type table.

> I'm pretty sure these aren't linked to my accessor change, so I've
> committed those changes.

No, likely they're not.

Best,

David

adrian at gossamer-threads

Mar 12, 2009, 7:42 PM

Post #6 of 8 (1759 views)
Permalink

Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

>> t/Bric/Test/Runner....NOK 11470/11815
>> # Failed test 'test_href died (Unable to execute SQL statement:
>> DBD::Pg::st execute failed: ERROR: duplicate key value violates
>> unique constraint "udx_server_type__name_site" [for Statement "
>> # INSERT INTO server_type (id, name, description,
>> site__id, copyable, publish, preview, active, class__id)
>> # VALUES (NEXTVAL('seq_server_type'), ?, ?, ?, ?, ?, ?, ?,
>> (SELECT id FROM class WHERE LOWER(disp_name) = LOWER(?)))
>> # " with ParamValues: 1='Bogus1', 2='Bogus ServerType1',
>> 3='100', 4='0', 5='1', 6='0', 7='1', 8='File System'] at
>> lib/Bric/Util/DBI.pm line 1136, <F> line 28.
>> #
>> # [lib/Bric/Util/DBI.pm:1136]
>> # [lib/Bric/Dist/ServerType.pm:2030]
>> # [t/Bric/Dist/ServerType/DevTest.pm:201]
>> # [/usr/share/perl5/Test/Class.pm:253]
>> # [/usr/share/perl5/Test/Class.pm:345]
>> # [t/Bric/Test/Runner.pm:101]
>> #
>> # [lib/Bric/Util/DBI.pm:1137]
>> # [lib/Bric/Dist/ServerType.pm:2030]
>> # [t/Bric/Dist/ServerType/DevTest.pm:201]
>> # [/usr/share/perl5/Test/Class.pm:253]
>> # [/usr/share/perl5/Test/Class.pm:345]
>> # [t/Bric/Test/Runner.pm:101])'
>> # at t/Bric/Test/Runner.pm line 101.
>> # (in Bric::Dist::ServerType::DevTest->test_href)
>>
>> These didn't happen the first time around. There's a few more of this
>> variety.
>
> Huh. That's not good. Sounds like there are some records that aren't
> getting deleted from the database. Which is odd. Please look and see if
> you have any records in the server_type table.

Yup, the test entries were left around from the last run:

bricolage=> select * from server_type;
id | class__id | name | description | site__id | copyable |
------+-----------+--------+-------------------+----------+----------+
1223 | 12 | Bogus1 | Bogus ServerType1 | 100 | t |

1224 | 12 | Bogus2 | Bogus ServerType | 100 | f |
1225 | 12 | Bogus3 | Bogus ServerType3 | 100 | t |
1226 | 12 | Bogus4 | Bogus ServerType | 100 | f |
1227 | 12 | Bogus5 | Bogus ServerType5 | 100 | t |

lannings at who

Mar 13, 2009, 1:29 AM

Post #7 of 8 (1743 views)
Permalink

Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

On Thu, 12 Mar 2009, Adrian Yee wrote:
> Yup, the test entries were left around from the last run:

Have to clear out the DB before running make devtest.
In perldoc lib/Bric/Hacker.pod , section "TESTING":

You can get a clean database without reinstalling Bricolage by doing

sudo make db
sudo make db_grant

david at kineticode

Mar 13, 2009, 9:34 AM

Post #8 of 8 (1749 views)
Permalink

Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

On Mar 13, 2009, at 1:29 AM, Scott Lanning wrote:

> On Thu, 12 Mar 2009, Adrian Yee wrote:
>> Yup, the test entries were left around from the last run:
>
> Have to clear out the DB before running make devtest.
> In perldoc lib/Bric/Hacker.pod , section "TESTING":
>
> You can get a clean database without reinstalling Bricolage by doing

Yeah, but test data should not have been stuck in there�

David

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads