Bug 24213 - Koha::Object->get_from_storage should return undef if the object has been deleted
Summary: Koha::Object->get_from_storage should return undef if the object has been del...
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 21901
  Show dependency treegraph
 
Reported: 2019-12-11 08:22 UTC by Jonathan Druart
Modified: 2020-02-18 04:28 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.03, 19.05.08, 18.11.14


Attachments
Bug 24213: Add tests (1.55 KB, patch)
2019-12-11 08:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted (1.04 KB, patch)
2019-12-11 08:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24213: Add tests (1.61 KB, patch)
2020-01-30 16:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted (1.11 KB, patch)
2020-01-30 16:05 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24213: Add tests (1.71 KB, patch)
2020-01-31 06:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted (1.20 KB, patch)
2020-01-31 06:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2019-12-11 08:22:41 UTC
It currently explodes with something like:
  DBIC result _type  isn't of the _type Subscription at t/db_dependent/Biblio.t line 627.
Comment 1 Jonathan Druart 2019-12-11 08:41:09 UTC
Created attachment 96179 [details] [review]
Bug 24213: Add tests
Comment 2 Jonathan Druart 2019-12-11 08:41:12 UTC
Created attachment 96180 [details] [review]
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted

Without this patch it fails with something like:
  DBIC result _type  isn't of the _type Subscription at t/db_dependent/Biblio.t line 627.

Test plan:
- Apply the first patch (Add tests)
- Confirm that they fail
- Apply this patch
- Confirm that the tests pass
Comment 3 Martin Renvoize 2020-01-30 16:05:50 UTC
Created attachment 98175 [details] [review]
Bug 24213: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2020-01-30 16:05:53 UTC
Created attachment 98176 [details] [review]
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted

Without this patch it fails with something like:
  DBIC result _type  isn't of the _type Subscription at t/db_dependent/Biblio.t line 627.

Test plan:
- Apply the first patch (Add tests)
- Confirm that they fail
- Apply this patch
- Confirm that the tests pass

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2020-01-30 16:06:44 UTC
Works as expected and makes sense.. Signing off.
Comment 6 Marcel de Rooy 2020-01-31 06:38:39 UTC
QAing
Comment 7 Marcel de Rooy 2020-01-31 06:40:52 UTC
    # Subtest: Bad value tests
        1..3
        # No tests run!
    not ok 16 - No tests run for subtest "Bad value tests"
    #   Failed test 'No tests run for subtest "Bad value tests"'
    #   at t/db_dependent/Koha/Object.t line 685.
    # Looks like you failed 1 test of 16.
not ok 16 - store() tests
#   Failed test 'store() tests'
#   at t/db_dependent/Koha/Object.t line 688.

Stumbling across this fail, but also without your patch.
Comment 8 Marcel de Rooy 2020-01-31 06:43:43 UTC
(In reply to Marcel de Rooy from comment #7)
>     # Subtest: Bad value tests
>         1..3
>         # No tests run!
>     not ok 16 - No tests run for subtest "Bad value tests"
>     #   Failed test 'No tests run for subtest "Bad value tests"'
>     #   at t/db_dependent/Koha/Object.t line 685.
>     # Looks like you failed 1 test of 16.
> not ok 16 - store() tests
> #   Failed test 'store() tests'
> #   at t/db_dependent/Koha/Object.t line 688.
> 
> Stumbling across this fail, but also without your patch.

Can be ignored. Resolved with strict_sql_mode. Maybe an idea to put this key in the default koha-conf.xml (commented or zero)?
Comment 9 Marcel de Rooy 2020-01-31 06:46:21 UTC
Created attachment 98203 [details] [review]
Bug 24213: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2020-01-31 06:46:26 UTC
Created attachment 98204 [details] [review]
Bug 24213: Fix Koha::Object->get_from_storage if the object has been deleted

Without this patch it fails with something like:
  DBIC result _type  isn't of the _type Subscription at t/db_dependent/Biblio.t line 627.

Test plan:
- Apply the first patch (Add tests)
- Confirm that they fail
- Apply this patch
- Confirm that the tests pass

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Jonathan Druart 2020-01-31 10:08:35 UTC
(In reply to Marcel de Rooy from comment #8)
> (In reply to Marcel de Rooy from comment #7)
> >     # Subtest: Bad value tests
> >         1..3
> >         # No tests run!
> >     not ok 16 - No tests run for subtest "Bad value tests"
> >     #   Failed test 'No tests run for subtest "Bad value tests"'
> >     #   at t/db_dependent/Koha/Object.t line 685.
> >     # Looks like you failed 1 test of 16.
> > not ok 16 - store() tests
> > #   Failed test 'store() tests'
> > #   at t/db_dependent/Koha/Object.t line 688.
> > 
> > Stumbling across this fail, but also without your patch.
> 
> Can be ignored. Resolved with strict_sql_mode. Maybe an idea to put this key
> in the default koha-conf.xml (commented or zero)?

I do not understand what you meant.

The test passes for me, with strict_sql_mode set.
When run with prove, the strict mode is enabled automatically.
Also the flag is set for devs (so from koha-testing-docker), that's is expected, we want devs to catch such problems.
Comment 12 Martin Renvoize 2020-01-31 13:56:17 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 13 Joy Nelson 2020-02-10 23:53:01 UTC
Pushed to 19.11.x branch for 19.11.03
Comment 14 Lucas Gass 2020-02-14 21:01:10 UTC
backported to 19.05.x for 19.05.08
Comment 15 Hayley Mapley 2020-02-18 04:28:18 UTC
Backported to 18.11.x for 18.11.14.