Skip to content

Convert the conf/database.conf.dist file to a Perl module. #2702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

drgrice1
Copy link
Member

@drgrice1 drgrice1 commented Apr 14, 2025

The database layout is now defined in the Perl WeBWorK::DB::Layout package. An instance of the WeBWorK::DB package is no longer instantiated by passing the layout. Instead it is passed a course environment object. An instance of the WeBWorK::DB::Layout package is constructed from that.

In addition, the WeBWorK::DB::Driver, WeBWorK::DB::Driver::Null, and WeBWorK::DB::Driver::SQL.pm modules have been deleted. Instead there is the WeBWorK::DB::Database module that does everything those modules used to do. It is now the only "driver" for all tables. Although, that never should have been called a driver. The driver is still optional and is DBD::MariaDB or DBD::mysql. That is what is usually referred to as the driver, not the DBI connection handle which is what the WeBWorK::DB::Database module deals with, and what the previous "driver" modules did as well. Furthermore, only one instance of this module is constructed when a WeBWorK::DB object is constructed, and that is passed to each table. That is essentially the same as before in terms of DBI connection handles except that before there were multiple WeBWorK::DB::Driver::SQL instances that all shared the same DBI conneciton handle via the connect_cached call.

So the layout is simplified considerably. The "driver", "source", "engine", and "character_set" are no longer options for a table, and there are no "%sqlParams" passed to each table. The only thing that is variable in the layout is the "$courseName" used for the tableOverride. The "source", "engine", and "character_set" are parameters of the WeBWorK::DB::Database object and retrieved from there as needed.

Since there is no choice of database layout there is no point in having that be part of the authentication module selection in the configuration files. So the $authen{user_module} in the configuration files should be either a string naming a single authentication module or an array of strings. For example,

$authen{user_module} = [
    'WeBWorK::Authen::LTIAdvantage',
    'WeBWorK::Authen::LTIAdvanced',
    'WeBWorK::Authen::Basic_TheLastOption'
];

The old format of { '*' => 'WeBWorK::Authen::LTIAdvantage' } (where * meant use this authentication for all database layouts) is still allowed for now, but eventually support for that will be dropped.

Several related unused files were deleted. Those are

  • bin/check_database_charsets.pl
    (This should have never been in the repository.)
  • bin/wwdb
    (This has been broken for a long time.)
  • lib/WeBWorK/DB/Driver/Null.pm
    (This was mentioned above, but I don't think this ever was used or even made sense to be used.)
  • lib/WeBWorK/Utils/CourseManagement/sql_moodle.pm
    (This is not used, I doubt this has worked for a long time, and is no longer supported in any case.)
  • lib/WeBWorK/Utils/CourseManagement/sql_single.pm
    (Also not used and hasn't been working for a while now.)
  • lib/WeBWorK/Utils/DBImportExport.pm
    (This hasn't worked for a while now and code that called this in lib/WeBWorK/DB.pm was removed.)
  • lib/WeBWorK/DB/Schema/NewSQL/VersionedMerge.pm
    (This is unused. At some point the lib/WeBWorK/DB/Schema/NewSQL/Merge.pm module was updated to handle its functionality directly.)

This is essentially a rework of #2338 except it does not move the database settings from site.conf, and it does not mess with the external program settings.

@drgrice1 drgrice1 mentioned this pull request Apr 14, 2025
@drgrice1 drgrice1 force-pushed the db-layout-module branch 9 times, most recently from ac8f4d2 to 64f333b Compare April 16, 2025 01:16
@drgrice1 drgrice1 force-pushed the db-layout-module branch 2 times, most recently from 22d95eb to 0f5e636 Compare April 20, 2025 11:12
Copy link
Member

@pstaabp pstaabp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested by running through most of the pages. Looks good.

Seems like we should get this in soon so we can test by using it.

The database layout is now defined in the  Perl `WeBWorK::DB::Layout`
package.  An instance of the `WeBWorK::DB` package is no longer
instantiated by passing the layout.  Instead it is passed a course
environment object.  An instance of the `WeBWorK::DB::Layout` package is
constructed from that.

In addition, the `WeBWorK::DB::Driver`, `WeBWorK::DB::Driver::Null`, and
`WeBWorK::DB::Driver::SQL.pm` modules have been deleted.  Instead there
is the `WeBWorK::DB::Database` module that does everything those modules
used to do.  It is now the only "driver" for all tables.  Although, that
never should have been called a driver.  The driver is still optional
and is `DBD::MariaDB` or `DBD::mysql`.  That is what is usually referred
to as the driver, not the DBI connection handle which is what the
`WeBWorK::DB::Database` module deals with, and what the previous
"driver" modules did as well.  Furthermore, only one instance of this
module is constructed when a `WeBWorK::DB` object is constructed, and
that is passed to each table.  That is essentially the same as before in
terms of DBI connection handles except that before there were multiple
`WeBWorK::DB::Driver::SQL` instances that all shared the same DBI
conneciton handle via the `connect_cached` call.

So the layout is simplified considerably.  The "driver", "source",
"engine", and "character_set" are no longer options for a table, and
there are no "%sqlParams" passed to each table. The only thing that is
variable in the layout is the "$courseName" used for the tableOverride.
The "source", "engine", and "character_set" are parameters of the
`WeBWorK::DB::Database` object and retrieved from there as needed.

Since there is no choice of database layout there is no point in having
that be part of the authentication module selection in the configuration
files.  So the `$authen{user_module}` in the configuration files should
be either a string naming a single authentication module or an array of
strings.  For example,

```perl
$authen{user_module} = [
    'WeBWorK::Authen::LTIAdvantage',
    'WeBWorK::Authen::LTIAdvanced',
    'WeBWorK::Authen::Basic_TheLastOption'
];

```

The old format of `{ '*' => 'WeBWorK::Authen::LTIAdvantage' }` (where
`*` meant use this authentication for all database layouts) is still
allowed for now, but eventually support for that will be dropped.

Several related unused files were deleted.  Those are

* `bin/check_database_charsets.pl`
  (This should have never been in the repository.)
* `bin/wwdb`
  (This has been broken for a long time.)
* `lib/WeBWorK/DB/Driver/Null.pm`
  (This was mentioned above, but I don't think this ever was used or
  even made sense to be used.)
* `lib/WeBWorK/Utils/CourseManagement/sql_moodle.pm`
  (This is not used, I doubt this has worked for a long time, and is no
  longer supported in any case.)
* `lib/WeBWorK/Utils/CourseManagement/sql_single.pm`
  (Also not used and hasn't been working for a while now.)
* `lib/WeBWorK/Utils/DBImportExport.pm`
  (This hasn't worked for a while now and code that called this in
  `lib/WeBWorK/DB.pm` was removed.)
* `lib/WeBWorK/DB/Schema/NewSQL/VersionedMerge.pm`
  (This is unused.  At some point the `lib/WeBWorK/DB/Schema/NewSQL/Merge.pm`
  module was updated to handle its functionality directly.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants