Bug 36432 - Remove circular dependency from Koha::Object
Summary: Remove circular dependency from Koha::Object
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 36486
  Show dependency treegraph
 
Reported: 2024-03-26 15:36 UTC by Julian Maurice
Modified: 2024-04-17 00:06 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00


Attachments
Bug 36432: Remove circular dependency from Koha::Object (11.37 KB, patch)
2024-03-26 15:37 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36432: Remove circular dependency from Koha::Object (11.27 KB, patch)
2024-03-26 15:51 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36432: Remove circular dependency from Koha::Object (11.37 KB, patch)
2024-03-26 15:54 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36432: Remove circular dependency from Koha::Object (11.43 KB, patch)
2024-03-27 03:59 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36432: Remove circular dependency from Koha::Object (14.37 KB, patch)
2024-03-27 07:39 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 36432: Remove circular dependency from Koha::Object (14.43 KB, patch)
2024-03-28 13:53 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36432: Remove circular dependency from Koha::Object (14.50 KB, patch)
2024-04-01 16:19 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2024-03-26 15:36:36 UTC
Koha::Object depends on Koha::DateUtils, which depends on C4::Context, which depends on Koha::Config::SysPref, which depends on... Koha::Object
Comment 1 Julian Maurice 2024-03-26 15:37:31 UTC
Created attachment 163940 [details] [review]
Bug 36432: Remove circular dependency from Koha::Object

Koha::Object depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPref, which depends on... Koha::Object

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Object (YAML::XS and ZOOM for instance).
As Koha::Object is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch moves some date parsing code to specific modules:
- Koha::DateTime::Format::RFC3339
- Koha::DateTime::Format::SQL

and it uses them in Koha::Object and Koha::DateUtils where it is
possible.

Test plan:
1. Do not apply the patch yet and run the following command:
   `perl -cw Koha/Object.pm`
   It should print several warnings about redefined subroutines, meaning
   there is a circular dependency.
2. Apply the patch
3. Run `perl -cw Koha/Object.pm`. It should only say:
   "Koha/Object.pm syntax OK"
4. Run the following command:
    prove \
        t/DateUtils.t \
        t/Koha/DateTime/Format/RFC3339.t \
        t/db_dependent/Koha/Object.t
Comment 2 Julian Maurice 2024-03-26 15:51:16 UTC
Created attachment 163941 [details] [review]
Bug 36432: Remove circular dependency from Koha::Object

Koha::Object depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPref, which depends on... Koha::Object

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Object (YAML::XS and ZOOM for instance).
As Koha::Object is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch moves some date parsing code to specific modules:
- Koha::DateTime::Format::RFC3339
- Koha::DateTime::Format::SQL

and it uses them in Koha::Object and Koha::DateUtils where it is
possible.

Test plan:
1. Do not apply the patch yet and run the following command:
   `perl -cw Koha/Object.pm`
   It should print several warnings about redefined subroutines, meaning
   there is a circular dependency.
2. Apply the patch
3. Run `perl -cw Koha/Object.pm`. It should only say:
   "Koha/Object.pm syntax OK"
4. Run the following command:
    prove \
        t/DateUtils.t \
        t/Koha/DateTime/Format/RFC3339.t \
        t/db_dependent/Koha/Object.t
Comment 3 Julian Maurice 2024-03-26 15:54:13 UTC
Created attachment 163942 [details] [review]
Bug 36432: Remove circular dependency from Koha::Object

Koha::Object depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPref, which depends on... Koha::Object

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Object (YAML::XS and ZOOM for instance).
As Koha::Object is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch moves some date parsing code to specific modules:
- Koha::DateTime::Format::RFC3339
- Koha::DateTime::Format::SQL

and it uses them in Koha::Object and Koha::DateUtils where it is
possible.

Test plan:
1. Do not apply the patch yet and run the following command:
   `perl -cw Koha/Object.pm`
   It should print several warnings about redefined subroutines, meaning
   there is a circular dependency.
2. Apply the patch
3. Run `perl -cw Koha/Object.pm`. It should only say:
   "Koha/Object.pm syntax OK"
4. Run the following command:
    prove \
        t/DateUtils.t \
        t/Koha/DateTime/Format/RFC3339.t \
        t/db_dependent/Koha/Object.t
Comment 4 Victor Grousset/tuxayo 2024-03-27 03:59:13 UTC
Created attachment 163971 [details] [review]
Bug 36432: Remove circular dependency from Koha::Object

Koha::Object depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPref, which depends on... Koha::Object

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Object (YAML::XS and ZOOM for instance).
As Koha::Object is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch moves some date parsing code to specific modules:
- Koha::DateTime::Format::RFC3339
- Koha::DateTime::Format::SQL

and it uses them in Koha::Object and Koha::DateUtils where it is
possible.

Test plan:
1. Do not apply the patch yet and run the following command:
   `perl -cw Koha/Object.pm`
   It should print several warnings about redefined subroutines, meaning
   there is a circular dependency.
2. Apply the patch
3. Run `perl -cw Koha/Object.pm`. It should only say:
   "Koha/Object.pm syntax OK"
4. Run the following command:
    prove \
        t/DateUtils.t \
        t/Koha/DateTime/Format/RFC3339.t \
        t/db_dependent/Koha/Object.t

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 5 Victor Grousset/tuxayo 2024-03-27 04:00:37 UTC
It works :)

---

To spare a QA roundtrip here is the next step:

 FAIL	Koha/DateTime/Format/RFC3339.pm
   FAIL	  pod coverage
		POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 5)

 FAIL	Koha/DateTime/Format/SQL.pm
   FAIL	  pod coverage
		POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 2)

 WARN	t/Koha/DateTime/Format/RFC3339.t
   WARN	  tidiness
		The file is less tidy than before (bad/messy lines before: 0, now: 23)
Comment 6 Julian Maurice 2024-03-27 07:39:08 UTC
Created attachment 163975 [details] [review]
Bug 36432: Remove circular dependency from Koha::Object

Koha::Object depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPref, which depends on... Koha::Object

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Object (YAML::XS and ZOOM for instance).
As Koha::Object is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch moves some date parsing code to specific modules:
- Koha::DateTime::Format::RFC3339
- Koha::DateTime::Format::SQL

and it uses them in Koha::Object and Koha::DateUtils where it is
possible.

Test plan:
1. Do not apply the patch yet and run the following command:
   `perl -cw Koha/Object.pm`
   It should print several warnings about redefined subroutines, meaning
   there is a circular dependency.
2. Apply the patch
3. Run `perl -cw Koha/Object.pm`. It should only say:
   "Koha/Object.pm syntax OK"
4. Run the following command:
    prove \
        t/DateUtils.t \
        t/Koha/DateTime/Format/RFC3339.t \
        t/db_dependent/Koha/Object.t
Comment 7 Julian Maurice 2024-03-27 07:39:49 UTC
(In reply to Victor Grousset/tuxayo from comment #5)
> To spare a QA roundtrip here is the next step:

Thanks. I attached a new patch that should pass all QA tests
Comment 8 Victor Grousset/tuxayo 2024-03-28 13:53:25 UTC
Created attachment 164056 [details] [review]
Bug 36432: Remove circular dependency from Koha::Object

Koha::Object depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPref, which depends on... Koha::Object

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Object (YAML::XS and ZOOM for instance).
As Koha::Object is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch moves some date parsing code to specific modules:
- Koha::DateTime::Format::RFC3339
- Koha::DateTime::Format::SQL

and it uses them in Koha::Object and Koha::DateUtils where it is
possible.

Test plan:
1. Do not apply the patch yet and run the following command:
   `perl -cw Koha/Object.pm`
   It should print several warnings about redefined subroutines, meaning
   there is a circular dependency.
2. Apply the patch
3. Run `perl -cw Koha/Object.pm`. It should only say:
   "Koha/Object.pm syntax OK"
4. Run the following command:
    prove \
        t/DateUtils.t \
        t/Koha/DateTime/Format/RFC3339.t \
        t/db_dependent/Koha/Object.t

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 9 Victor Grousset/tuxayo 2024-03-28 13:53:44 UTC
thanks :)

----

Added back signoff lost in the process.
Comment 10 Nick Clemens 2024-04-01 16:19:24 UTC
Created attachment 164210 [details] [review]
Bug 36432: Remove circular dependency from Koha::Object

Koha::Object depends on Koha::DateUtils, which depends on C4::Context,
which depends on Koha::Config::SysPref, which depends on... Koha::Object

Apart from the circular dependency, the dependency on C4::Context alone
is problematic as it loads a bunch of modules that are not needed at all
in Koha::Object (YAML::XS and ZOOM for instance).
As Koha::Object is used as a base for a lot of modules, we should take
care to only load the minimum required.

This patch moves some date parsing code to specific modules:
- Koha::DateTime::Format::RFC3339
- Koha::DateTime::Format::SQL

and it uses them in Koha::Object and Koha::DateUtils where it is
possible.

Test plan:
1. Do not apply the patch yet and run the following command:
   `perl -cw Koha/Object.pm`
   It should print several warnings about redefined subroutines, meaning
   there is a circular dependency.
2. Apply the patch
3. Run `perl -cw Koha/Object.pm`. It should only say:
   "Koha/Object.pm syntax OK"
4. Run the following command:
    prove \
        t/DateUtils.t \
        t/Koha/DateTime/Format/RFC3339.t \
        t/db_dependent/Koha/Object.t

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Julian Maurice 2024-04-02 07:36:30 UTC
Thanks for testing!
I added some unit tests for Koha::DateTime::Format::SQL in bug 36486
Comment 12 Katrin Fischer 2024-04-04 13:25:51 UTC
Pushed for 24.05!

Well done everyone, thank you!