Bug 17425

Summary: Koha::Object should raise exceptions
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, josef.moravec, kyle, lari.taskula, mirko
Version: MainKeywords: dependency, nowheezy
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13995
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 17091    
Bug Blocks: 17084, 17428, 17495    
Attachments: Bug 17425: Make Koha::Object raise exceptions
Bug 17425: Make Koha::Object raise exceptions
Bug 17425: Add Try::Tiny dependency
Bug 17425: Make Koha::Object raise exceptions
Bug 17425: Add Try::Tiny dependency
Bug 17425: Make Koha::Object raise exceptions
Bug 17425: Add Try::Tiny dependency
Bug 17425: Make Koha::Object raise exceptions
Bug 17425: Add Try::Tiny dependency
Bug 17425: Add the Object::MethodNotCoveredByTests exception
Bug 17425: Make Koha::Object raise exceptions
Bug 17425: Add Try::Tiny dependency
Bug 17425: Add the Object::MethodNotCoveredByTests exception

Description Tomás Cohen Arazi 2016-10-11 11:38:27 UTC
Koha::Objects should raise exceptions when invalid accessors are called, and when invalid properties are updated using ->set.
Comment 1 Tomás Cohen Arazi 2016-10-11 11:50:55 UTC
Created attachment 56168 [details] [review]
Bug 17425: Make Koha::Object raise exceptions

This patch makes Koha::Object raise exceptions in the following
situations:
- When a non existent accessor is called
- When a non existent property is tried to be updated using ->set

On implementing this change, we introduce Koha::Exceptions::Object class
to contain all Koha::Object-specific exception definitions.

Unit tests for this change are introduced in
t/db_dependent/Koha/Objects.t

To test:
- Apply the patches on master
- Run:
  $ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests return green
- Sign off

Note: A followup introduces the dependency for Try::Tiny. It needs to be
present for running the tests.
Comment 2 Tomás Cohen Arazi 2016-10-11 11:58:08 UTC
Created attachment 56169 [details] [review]
Bug 17425: Make Koha::Object raise exceptions

This patch makes Koha::Object raise exceptions in the following
situations:
- When a non existent accessor is called
- When a non existent property is tried to be updated using ->set

On implementing this change, we introduce Koha::Exceptions::Object class
to contain all Koha::Object-specific exception definitions.

Unit tests for this change are introduced in
t/db_dependent/Koha/Objects.t

To test:
- Apply the patches on master
- Run:
  $ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests return green
- Sign off

Note: A followup introduces the dependency for Try::Tiny. It needs to be
present for running the tests.
Comment 3 Tomás Cohen Arazi 2016-10-11 11:59:04 UTC
Created attachment 56170 [details] [review]
Bug 17425: Add Try::Tiny dependency

This patch just introduces Try::Tiny as a core Koha dependency.
Comment 4 Josef Moravec 2016-10-11 13:44:31 UTC
Created attachment 56187 [details] [review]
Bug 17425: Make Koha::Object raise exceptions

This patch makes Koha::Object raise exceptions in the following
situations:
- When a non existent accessor is called
- When a non existent property is tried to be updated using ->set

On implementing this change, we introduce Koha::Exceptions::Object class
to contain all Koha::Object-specific exception definitions.

Unit tests for this change are introduced in
t/db_dependent/Koha/Objects.t

To test:
- Apply the patches on master
- Run:
  $ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests return green
- Sign off

Note: A followup introduces the dependency for Try::Tiny. It needs to be
present for running the tests.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Josef Moravec 2016-10-11 13:44:36 UTC
Created attachment 56188 [details] [review]
Bug 17425: Add Try::Tiny dependency

This patch just introduces Try::Tiny as a core Koha dependency.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 6 Mirko Tietgen 2016-10-12 06:04:37 UTC
Created attachment 56218 [details]
Bug 17425 Try::Tiny Control file update (master)
Comment 7 Kyle M Hall 2016-10-12 07:07:16 UTC
Created attachment 56220 [details] [review]
Bug 17425: Make Koha::Object raise exceptions

This patch makes Koha::Object raise exceptions in the following
situations:
- When a non existent accessor is called
- When a non existent property is tried to be updated using ->set

On implementing this change, we introduce Koha::Exceptions::Object class
to contain all Koha::Object-specific exception definitions.

Unit tests for this change are introduced in
t/db_dependent/Koha/Objects.t

To test:
- Apply the patches on master
- Run:
  $ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests return green
- Sign off

Note: A followup introduces the dependency for Try::Tiny. It needs to be
present for running the tests.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall 2016-10-12 07:07:33 UTC
Created attachment 56221 [details] [review]
Bug 17425: Add Try::Tiny dependency

This patch just introduces Try::Tiny as a core Koha dependency.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Lari Taskula 2016-10-13 13:06:07 UTC
After Bug 17226

Applying: Bug 17425: Make Koha::Object raise exceptions
Using index info to reconstruct a base tree...
M       Koha/Object.pm
Falling back to patching base and 3-way merge...
error: The following untracked working tree files would be overwritten by merge:
        Koha/Exceptions/Object.pm
Please move or remove them before you can merge.
Aborting
Failed to merge in the changes.
Patch failed at 0001 Bug 17425: Make Koha::Object raise exceptions
Comment 10 Jonathan Druart 2016-10-13 13:10:11 UTC
Please rebase
Comment 11 Jonathan Druart 2016-10-13 13:18:17 UTC
Created attachment 56393 [details] [review]
Bug 17425: Make Koha::Object raise exceptions

This patch makes Koha::Object raise exceptions in the following
situations:
- When a non existent accessor is called
- When a non existent property is tried to be updated using ->set

On implementing this change, we introduce Koha::Exceptions::Object class
to contain all Koha::Object-specific exception definitions.

Unit tests for this change are introduced in
t/db_dependent/Koha/Objects.t

To test:
- Apply the patches on master
- Run:
  $ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests return green
- Sign off

Note: A followup introduces the dependency for Try::Tiny. It needs to be
present for running the tests.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Jonathan Druart 2016-10-13 13:18:22 UTC
Created attachment 56394 [details] [review]
Bug 17425: Add Try::Tiny dependency

This patch just introduces Try::Tiny as a core Koha dependency.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Jonathan Druart 2016-10-13 13:18:26 UTC
Created attachment 56395 [details] [review]
Bug 17425: Add the Object::MethodNotCoveredByTests exception
Comment 14 Jonathan Druart 2016-10-13 13:18:46 UTC
Tomas, please have a look at the third patch.
Comment 15 Tomás Cohen Arazi 2016-10-14 07:57:15 UTC
Created attachment 56459 [details] [review]
Bug 17425: Make Koha::Object raise exceptions

This patch makes Koha::Object raise exceptions in the following
situations:
- When a non existent accessor is called
- When a non existent property is tried to be updated using ->set

On implementing this change, we introduce Koha::Exceptions::Object class
to contain all Koha::Object-specific exception definitions.

Unit tests for this change are introduced in
t/db_dependent/Koha/Objects.t

To test:
- Apply the patches on master
- Run:
  $ prove t/db_dependent/Koha/Objects.t
=> SUCCESS: Tests return green
- Sign off

Note: A followup introduces the dependency for Try::Tiny. It needs to be
present for running the tests.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2016-10-14 07:57:23 UTC
Created attachment 56460 [details] [review]
Bug 17425: Add Try::Tiny dependency

This patch just introduces Try::Tiny as a core Koha dependency.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Tomás Cohen Arazi 2016-10-14 07:57:55 UTC
Created attachment 56461 [details] [review]
Bug 17425: Add the Object::MethodNotCoveredByTests exception

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Kyle M Hall 2016-10-21 17:38:49 UTC
Pushed to master for 16.11, thanks Tomas, Jonathan!
Comment 19 Mirko Tietgen 2016-11-16 10:01:21 UTC
Ubuntu 14.04 has libtry-tiny-perl 0.19-1. Can we lower the dependency min version?
Comment 20 Kyle M Hall 2016-11-16 14:20:39 UTC
(In reply to Mirko Tietgen from comment #19)
> Ubuntu 14.04 has libtry-tiny-perl 0.19-1. Can we lower the dependency min
> version?

Mirko, can you file new bug reports for these requests? If you can supply the patch as well that should help us move it along! If we want to do that for 16.11, we need to move fast!
Comment 21 Mirko Tietgen 2016-11-16 15:10:47 UTC
Kyle, I'd drop the idea. It does not make sense to support 14.04 when we drop Wheezy support.
Comment 22 Mirko Tietgen 2016-11-16 15:20:43 UTC
(the thinking was if it is possible to support 14.04 while 16.04 has still problems with mysql 5.7, but as mariadb is a working alternative, I don't care about 14.04)