Bug 17425 - Koha::Object should raise exceptions
Summary: Koha::Object should raise exceptions
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Whiteboard:
Keywords: dependency, nowheezy
Depends on: 17091
Blocks: 17084 17428 17495
  Show dependency treegraph
 
Reported: 2016-10-11 11:38 UTC by Tomás Cohen Arazi
Modified: 2017-06-14 22:15 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17425: Make Koha::Object raise exceptions (5.40 KB, patch)
2016-10-11 11:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17425: Make Koha::Object raise exceptions (5.40 KB, patch)
2016-10-11 11:58 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17425: Add Try::Tiny dependency (843 bytes, patch)
2016-10-11 11:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17425: Make Koha::Object raise exceptions (5.45 KB, patch)
2016-10-11 13:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17425: Add Try::Tiny dependency (898 bytes, patch)
2016-10-11 13:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17425: Make Koha::Object raise exceptions (5.45 KB, patch)
2016-10-12 07:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17425: Add Try::Tiny dependency (898 bytes, patch)
2016-10-12 07:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17425: Make Koha::Object raise exceptions (5.44 KB, patch)
2016-10-13 13:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17425: Add Try::Tiny dependency (898 bytes, patch)
2016-10-13 13:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17425: Add the Object::MethodNotCoveredByTests exception (2.18 KB, patch)
2016-10-13 13:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17425: Make Koha::Object raise exceptions (5.49 KB, patch)
2016-10-14 07:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17425: Add Try::Tiny dependency (954 bytes, patch)
2016-10-14 07:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 17425: Add the Object::MethodNotCoveredByTests exception (2.23 KB, patch)
2016-10-14 07:57 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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)