From a81f77ff220269b03bf10fddf0f819c2a083f687 Mon Sep 17 00:00:00 2001 From: Glenn Rice Date: Sun, 22 Oct 2023 07:00:23 -0500 Subject: [PATCH] Make SQL::Abstract work. This removes the optional dependence on SQL::Abstract::Classic or a version of SQL::Abstract prior to 2.000000, and only uses version 2.000000 or newer of SQL::Abstract. For this to work the capability of a field override needs to be removed. There doesn't seem to be a reliable way to do that with the newer versions of SQL::Abstract, although I didn't try very hard. I also don't see the need for having a field override. The only field that has an override is the `key_not_a_keyfield` or `key` field. Why was that ever done? Did someone think that MySQL would make a field a database key field if it had the name "key"? Did someone think it would be confusing to have a field named "key" that was not a database key field? It was probably the latter, but in my opinion the field override was even more confusing. The `key` field in the `key` table does not contain any permanent data, so upgrading courses does not require any special handling. The data from the old `key_not_a_keyfield` field can be safely dropped, and the new `key` field used in its place. You can leave the `key_not_a_keyfield` field in the database if you want to switch back and forth between other pull requests that use the `key_not_a_keyfield` field. --- bin/check_modules.pl | 27 +- conf/database.conf.dist | 450 ++++++++---------- lib/WeBWorK/DB/Schema/NewSQL/Merge.pm | 8 +- lib/WeBWorK/DB/Schema/NewSQL/Std.pm | 28 +- lib/WeBWorK/DB/Utils/SQLAbstractIdentTrans.pm | 87 ++-- lib/WebworkSOAP/Classes/Key.pm | 10 +- 6 files changed, 235 insertions(+), 375 deletions(-) diff --git a/bin/check_modules.pl b/bin/check_modules.pl index d2c7797633..6d709aeda5 100755 --- a/bin/check_modules.pl +++ b/bin/check_modules.pl @@ -141,6 +141,7 @@ =head1 DESCRIPTION Scalar::Util SOAP::Lite Socket + SQL::Abstract Statistics::R::IO String::ShellQuote SVG @@ -166,7 +167,8 @@ =head1 DESCRIPTION 'LWP::Protocol::https' => 6.06, 'Mojolicious' => 9.22, 'Net::SSLeay' => 1.46, - 'Perl::Tidy' => 20220613 + 'Perl::Tidy' => 20220613, + 'SQL::Abstract' => 2.000000 ); my ($test_programs, $test_modules, $show_help); @@ -249,29 +251,6 @@ sub check_modules { print " $module found and loaded\n"; } } - checkSQLabstract(); -} - -## this is specialized code to check for either SQL::Abstract or SQL::Abstract::Classic - -sub checkSQLabstract { - print "\n checking for SQL::Abstract\n\n"; - eval "use SQL::Abstract"; - my $sql_abstract = not($@); - my $sql_abstract_version = $SQL::Abstract::VERSION if $sql_abstract; - - eval "use SQL::Abstract::Classic"; - my $sql_abstract_classic = not($@); - - if ($sql_abstract_classic) { - print qq/ You have SQL::Abstract::Classic installed. This package will be used if either - the installed version of SQL::Abstract is version > 1.87 or if that package is not installed.\n/; - } elsif ($sql_abstract && $sql_abstract_version <= 1.87) { - print "You have version $sql_abstract_version of SQL::Abstract installed. This will be used\n"; - } else { - print qq/You need either SQL::Abstract version <= 1.87 or need to install SQL::Abstract::Classic. - If you are using cpan or cpanm, it is recommended to install SQL::Abstract::Classic.\n/; - } } 1; diff --git a/conf/database.conf.dist b/conf/database.conf.dist index 139120e3b9..3b13b4e393 100644 --- a/conf/database.conf.dist +++ b/conf/database.conf.dist @@ -38,10 +38,6 @@ line like the one below to the F or F file. $dbLayoutName = "layoutName"; *dbLayout = $dbLayouts{$dbLayoutName}; -=cut - -%dbLayouts = (); # layouts are added to this hash below - =head2 THE SQL_SINGLE DATABASE LAYOUT The C layout is similar to the C layout, except that it uses a @@ -65,14 +61,10 @@ Other parameters that can be given are as follows: tableOverride an alternate name to use when referring to the table (used when a table name is a reserved word) - fieldOverride a hash mapping WeBWorK field names to alternate names to use - when referring to those fields (used when one or more field - names are reserved words) debug if true, SQL statements are printed before being executed =cut - # params common to all tables my %sqlParams = ( @@ -84,127 +76,109 @@ my %sqlParams = ( mysqldump_path => $externalPrograms{mysqldump}, ); -if ( $ce->{database_driver} =~ /^mysql$/i ) { +if ($ce->{database_driver} =~ /^mysql$/i) { # The extra UTF8 connection setting is ONLY needed for older DBD:mysql driver # and forbidden by the newer DBD::MariaDB driver - if ( $ENABLE_UTF8MB4 ) { - $sqlParams{mysql_enable_utf8mb4} = 1; # Full 4-bit UTF-8 + if ($ENABLE_UTF8MB4) { + $sqlParams{mysql_enable_utf8mb4} = 1; # Full 4-bit UTF-8 } else { - $sqlParams{mysql_enable_utf8} = 1; # Only the partial 3-bit mySQL UTF-8 + $sqlParams{mysql_enable_utf8} = 1; # Only the partial 3-bit mySQL UTF-8 } } +%dbLayouts = (); # layouts are added to this hash below + $dbLayouts{sql_single} = { - locations => { - record => "WeBWorK::DB::Record::Locations", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - non_native => 1, - }, - }, - location_addresses => { - record => "WeBWorK::DB::Record::LocationAddresses", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - non_native => 1, - }, - }, - depths => { - record => "WeBWorK::DB::Record::Depths", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - params => { %sqlParams, - non_native => 1, - }, + locations => { + record => "WeBWorK::DB::Record::Locations", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, non_native => 1 }, }, - password => { - record => "WeBWorK::DB::Record::Password", + location_addresses => { + record => "WeBWorK::DB::Record::LocationAddresses", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, non_native => 1 }, + }, + depths => { + record => "WeBWorK::DB::Record::Depths", schema => "WeBWorK::DB::Schema::NewSQL::Std", driver => "WeBWorK::DB::Driver::SQL", source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_password", - }, + engine => $database_storage_engine, + params => { %sqlParams, non_native => 1 }, + }, + password => { + record => "WeBWorK::DB::Record::Password", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_password" }, }, permission => { - record => "WeBWorK::DB::Record::PermissionLevel", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_permission", - }, + record => "WeBWorK::DB::Record::PermissionLevel", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_permission" }, }, key => { - record => "WeBWorK::DB::Record::Key", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_key", - fieldOverride => { key => "key_not_a_keyword" }, - }, + record => "WeBWorK::DB::Record::Key", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_key" }, }, user => { - record => "WeBWorK::DB::Record::User", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_user", - }, + record => "WeBWorK::DB::Record::User", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_user" }, }, set => { - record => "WeBWorK::DB::Record::Set", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_set", - #fieldOverride => { visible => "published" }, # for compatibility -- visible was originally called published - }, + record => "WeBWorK::DB::Record::Set", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_set" }, }, set_user => { - record => "WeBWorK::DB::Record::UserSet", - schema => "WeBWorK::DB::Schema::NewSQL::NonVersioned", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_set_user", - #fieldOverride => { visible => "published" }, # for compatibility -- visible was originally called published - }, + record => "WeBWorK::DB::Record::UserSet", + schema => "WeBWorK::DB::Schema::NewSQL::NonVersioned", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_set_user" }, }, set_merged => { - record => "WeBWorK::DB::Record::UserSet", - schema => "WeBWorK::DB::Schema::NewSQL::Merge", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - depend => [qw/set_user set/], - params => { %sqlParams, + record => "WeBWorK::DB::Record::UserSet", + schema => "WeBWorK::DB::Schema::NewSQL::Merge", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + depend => [qw/set_user set/], + params => { + %sqlParams, non_native => 1, merge => [qw/set_user set/], }, @@ -214,195 +188,153 @@ $dbLayouts{sql_single} = { schema => "WeBWorK::DB::Schema::NewSQL::Versioned", driver => "WeBWorK::DB::Driver::SQL", source => $database_dsn, - engine => $database_storage_engine, - params => { %sqlParams, - non_native => 1, + engine => $database_storage_engine, + params => { + %sqlParams, + non_native => 1, tableOverride => "${courseName}_set_user", - #fieldOverride => { visible => "published" }, # for compatibility -- visible was originally called published }, }, set_version_merged => { - record => "WeBWorK::DB::Record::SetVersion", - schema => "WeBWorK::DB::Schema::NewSQL::Merge", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - depend => [qw/set_version set_user set/], - params => { %sqlParams, + record => "WeBWorK::DB::Record::SetVersion", + schema => "WeBWorK::DB::Schema::NewSQL::Merge", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + depend => [qw/set_version set_user set/], + params => { + %sqlParams, non_native => 1, merge => [qw/set_version set_user set/], }, }, - set_locations => { - record => "WeBWorK::DB::Record::SetLocations", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_set_locations" - }, - }, - set_locations_user => { - record => "WeBWorK::DB::Record::UserSetLocations", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_set_locations_user" - }, - }, + set_locations => { + record => "WeBWorK::DB::Record::SetLocations", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_set_locations" }, + }, + set_locations_user => { + record => "WeBWorK::DB::Record::UserSetLocations", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_set_locations_user" }, + }, problem => { - record => "WeBWorK::DB::Record::Problem", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_problem" - }, + record => "WeBWorK::DB::Record::Problem", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_problem" }, }, problem_user => { - record => "WeBWorK::DB::Record::UserProblem", - schema => "WeBWorK::DB::Schema::NewSQL::NonVersioned", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_problem_user" - }, + record => "WeBWorK::DB::Record::UserProblem", + schema => "WeBWorK::DB::Schema::NewSQL::NonVersioned", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_problem_user" }, }, problem_merged => { - record => "WeBWorK::DB::Record::UserProblem", - schema => "WeBWorK::DB::Schema::NewSQL::Merge", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - depend => [qw/problem_user problem/], - params => { %sqlParams, + record => "WeBWorK::DB::Record::UserProblem", + schema => "WeBWorK::DB::Schema::NewSQL::Merge", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + depend => [qw/problem_user problem/], + params => { + %sqlParams, non_native => 1, merge => [qw/problem_user problem/], }, }, problem_version => { - record => "WeBWorK::DB::Record::ProblemVersion", - schema => "WeBWorK::DB::Schema::NewSQL::Versioned", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - non_native => 1, + record => "WeBWorK::DB::Record::ProblemVersion", + schema => "WeBWorK::DB::Schema::NewSQL::Versioned", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { + %sqlParams, + non_native => 1, tableOverride => "${courseName}_problem_user", }, }, problem_version_merged => { - record => "WeBWorK::DB::Record::ProblemVersion", - schema => "WeBWorK::DB::Schema::NewSQL::Merge", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - depend => [qw/problem_version problem_user problem/], - params => { %sqlParams, + record => "WeBWorK::DB::Record::ProblemVersion", + schema => "WeBWorK::DB::Schema::NewSQL::Merge", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + depend => [qw/problem_version problem_user problem/], + params => { + %sqlParams, non_native => 1, merge => [qw/problem_version problem_user problem/], }, }, setting => { - record => "WeBWorK::DB::Record::Setting", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_setting" - }, + record => "WeBWorK::DB::Record::Setting", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_setting" }, }, - achievement => { - record => "WeBWorK::DB::Record::Achievement", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_achievement" - }, + achievement => { + record => "WeBWorK::DB::Record::Achievement", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_achievement" }, }, past_answer => { - record => "WeBWorK::DB::Record::PastAnswer", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_past_answer" - }, - }, + record => "WeBWorK::DB::Record::PastAnswer", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_past_answer" }, + }, achievement_user => { - record => "WeBWorK::DB::Record::UserAchievement", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_achievement_user" - }, - }, + record => "WeBWorK::DB::Record::UserAchievement", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_achievement_user" }, + }, global_user_achievement => { - record => "WeBWorK::DB::Record::GlobalUserAchievement", - schema => "WeBWorK::DB::Schema::NewSQL::Std", - driver => "WeBWorK::DB::Driver::SQL", - source => $database_dsn, - engine => $database_storage_engine, - character_set => $database_character_set, - params => { %sqlParams, - tableOverride => "${courseName}_global_user_achievement" - }, + record => "WeBWorK::DB::Record::GlobalUserAchievement", + schema => "WeBWorK::DB::Schema::NewSQL::Std", + driver => "WeBWorK::DB::Driver::SQL", + source => $database_dsn, + engine => $database_storage_engine, + character_set => $database_character_set, + params => { %sqlParams, tableOverride => "${courseName}_global_user_achievement" }, }, }; # include ("conf/database.conf"); # uncomment to provide local overrides - -=head1 DATABASE LAYOUT METADATA - -=over - -=item @dbLayout_order - -Database layouts listed in this array will be displayed first, in the order -specified, wherever database layouts are listed. (For example, in the "Add -Course" tool.) Other layouts are listed after these. - -=cut - -@dbLayout_order = qw/sql_single sql_moodle/; - -=item %dbLayout_descr - -Hash mapping database layout names to textual descriptions. - -=cut - -%dbLayout_descr = ( - sql_single => "Uses a single SQL database to record WeBWorK data for all courses using this layout. This is the recommended layout for new courses.", -# sql_moodle => "Similar to sql_single, but uses a Moodle database for user, password, and permission information. This layout should be used for courses used with wwmoodle.", -); - -=back - -=cut +1; diff --git a/lib/WeBWorK/DB/Schema/NewSQL/Merge.pm b/lib/WeBWorK/DB/Schema/NewSQL/Merge.pm index 28ec574fa4..f5185ceacc 100644 --- a/lib/WeBWorK/DB/Schema/NewSQL/Merge.pm +++ b/lib/WeBWorK/DB/Schema/NewSQL/Merge.pm @@ -122,11 +122,8 @@ sub merge_init { sub sql_init { my $self = shift; - # transformation functions for table and field names: these allow us to pass - # the WeBWorK table/field names to SQL::Abstract::Classic, and have it translate them - # to the SQL table/field names from tableOverride and fieldOverride. - # (Without this, it would be hard to translate field names in WHERE - # structures, since they're so convoluted.) + # Transformation function for table names. This allows us to pass the WeBWorK table names to + # SQL::Abstract, and have it translate them to the SQL table names from tableOverride. my $transform_table = sub { my $label = shift; if (exists $self->{sql_table_aliases}{$label}) { @@ -152,7 +149,6 @@ sub sql_init { return "`$table`.`$field`"; }; - # add SQL statement generation object $self->{sql} = new WeBWorK::DB::Utils::SQLAbstractIdentTrans( quote_char => "`", name_sep => ".", diff --git a/lib/WeBWorK/DB/Schema/NewSQL/Std.pm b/lib/WeBWorK/DB/Schema/NewSQL/Std.pm index f71647df0b..765d65c08d 100644 --- a/lib/WeBWorK/DB/Schema/NewSQL/Std.pm +++ b/lib/WeBWorK/DB/Schema/NewSQL/Std.pm @@ -42,11 +42,6 @@ This schema pays attention to the following items in the C entry. Alternate name for this table, to satisfy SQL naming requirements. -=item fieldOverride - -A reference to a hash mapping field names to alternate names, to satisfy SQL -naming requirements. - =back =cut @@ -70,36 +65,24 @@ sub new { sub sql_init { my $self = shift; - # transformation functions for table and field names: these allow us to pass - # the WeBWorK table/field names to SQL::Abstract::Classic, and have it translate them - # to the SQL table/field names from tableOverride and fieldOverride. - # (Without this, it would be hard to translate field names in WHERE - # structures, since they're so convoluted.) - my ($transform_table, $transform_field); + # Transformation function for table names. This allows us to pass the WeBWorK table names to + # SQL::Abstract, and have it translate them to the SQL table names from tableOverride. + my $transform_table; if (defined $self->{params}{tableOverride}) { $transform_table = sub { my $label = shift; if ($label eq $self->{table}) { return $self->{params}{tableOverride}; } else { - #warn "can't transform unrecognized table name '$label'"; return $label; } }; } - if (defined $self->{params}{fieldOverride}) { - $transform_field = sub { - my $label = shift; - return defined $self->{params}{fieldOverride}{$label} ? $self->{params}{fieldOverride}{$label} : $label; - }; - } - # add SQL statement generation object $self->{sql} = new WeBWorK::DB::Utils::SQLAbstractIdentTrans( quote_char => "`", name_sep => ".", - transform_table => $transform_table, - transform_field => $transform_field, + transform_table => $transform_table ); } @@ -902,10 +885,11 @@ sub character_set { my $self = shift; return (defined $self->{character_set} and $self->{character_set}) ? $self->{character_set} : 'latin1'; } + # returns non-quoted SQL name of given field sub sql_field_name { my ($self, $field) = @_; - return defined $self->{params}{fieldOverride}{$field} ? $self->{params}{fieldOverride}{$field} : $field; + return $field; } # returns fully quoted expression refering to the specified field diff --git a/lib/WeBWorK/DB/Utils/SQLAbstractIdentTrans.pm b/lib/WeBWorK/DB/Utils/SQLAbstractIdentTrans.pm index f69bd9ab0c..b0e15982b6 100644 --- a/lib/WeBWorK/DB/Utils/SQLAbstractIdentTrans.pm +++ b/lib/WeBWorK/DB/Utils/SQLAbstractIdentTrans.pm @@ -14,26 +14,12 @@ ################################################################################ package WeBWorK::DB::Utils::SQLAbstractIdentTrans; -my $BASE; - -BEGIN { - my $sql_abstract = eval { - require SQL::Abstract; - if ($SQL::Abstract::VERSION > 1.87) { - 0; - } else { - 1; - } - }; - $BASE = qw(SQL::Abstract) if $sql_abstract; - $BASE = qw(SQL::Abstract::Classic) unless $sql_abstract; -} -use base $BASE; +use parent qw(SQL::Abstract); =head1 NAME WeBWorK::DB::Utils::SQLAbstractIdentTrans - subclass of SQL::Abstract::Classic that -allows custom hooks to transform identifiers. +allows custom hooks to transform table names. =cut @@ -41,68 +27,51 @@ use strict; use warnings; sub _table { - my $self = shift; - my $tab = shift; - if (ref $tab eq 'ARRAY') { - return join ', ', map { $self->_quote_table($_) } @$tab; - } else { - return $self->_quote_table($tab); + my ($self, $from) = @_; + if (ref($from) eq 'ARRAY') { + return $self->SUPER::_table([ map { $self->_transform_table($_) } @$from ]); + } elsif (!ref($from)) { + return $self->SUPER::_table($self->_transform_table($from)); } + return $self->SUPER::_table($from); } sub _quote { - my $self = shift; - my $label = shift; + my ($self, $label) = @_; return $label if $label eq '*'; - return $self->_quote_field($label) - if !defined $self->{name_sep}; + return join($self->{name_sep} || '', map { $self->_quote($_) } @$label) if ref($label) eq 'ARRAY'; + + return $self->SUPER::_quote($label) unless defined $self->{name_sep}; - if (defined $self->{transform_all}) { + if (ref($self->{transform_all}) eq 'CODE') { return $self->{transform_all}->($label); } elsif ($label =~ /(.+)\.(.+)/) { - return $self->_quote_table($1) . $self->{name_sep} . $self->_quote_field($2); + return $self->SUPER::_quote($self->_transform_table($1)) . $self->{name_sep} . $self->SUPER::_quote($2); } else { - return $self->_quote_field($label); + return $self->SUPER::_quote($label); } } -sub _quote_table { - my $self = shift; - my $label = shift; - - # if the table name is a scalar reference, leave it alone (but dereference it) - return $$label if ref $label eq "SCALAR"; - - # call custom transform function - $label = $self->{transform_table}->($label) - if defined $self->{transform_table}; - - return $self->{quote_char} . $label . $self->{quote_char}; +sub _transform_table { + my ($self, $table) = @_; + return ref($self->{transform_table}) eq 'CODE' ? $self->{transform_table}->($table) : $table; } -sub _quote_field { - my $self = shift; - my $label = shift; - - # call custom transform function - $label = $self->{transform_field}->($label) - if defined $self->{transform_field}; - - return $self->{quote_char} . $label . $self->{quote_char}; +sub insert { + my ($self, $table, $data, $options) = @_; + return $self->SUPER::insert($self->_transform_table($table), $data, $options); } -sub _order_by { - my $self = shift; - my $ref = ref $_[0]; - - my @vals = $ref eq 'ARRAY' ? @{ $_[0] } : $ref eq 'SCALAR' ? $_[0] : # modification: don't dereference scalar refs - $ref eq '' ? $_[0] : $self->SUPER::puke("Unsupported data struct $ref for ORDER BY"); +sub update { + my ($self, $table, $set, $where, $options) = @_; + return $self->SUPER::update($self->_transform_table($table), $set, $where, $options); +} - # modification: if an item is a scalar ref, don't quote it, only dereference it - my $val = join ', ', map { ref $_ eq "SCALAR" ? $$_ : $self->_quote($_) } @vals; - return $val ? $self->_sqlcase(' order by') . " $val" : ''; +sub delete { + my ($self, $table, $where, $options) = @_; + return $self->SUPER::delete($self->_transform_table($table), $where, $options); } 1; diff --git a/lib/WebworkSOAP/Classes/Key.pm b/lib/WebworkSOAP/Classes/Key.pm index 4f85e7633a..e9b47907be 100644 --- a/lib/WebworkSOAP/Classes/Key.pm +++ b/lib/WebworkSOAP/Classes/Key.pm @@ -4,7 +4,7 @@ package WebworkSOAP::Classes::Key; =begin WSDL _ATTR user_id $string user_id - _ATTR key_not_a_keyboard $string key_not_a_keyboard + _ATTR key $string key _ATTR timestamp $string timestamp =end WSDL @@ -13,10 +13,10 @@ package WebworkSOAP::Classes::Key; sub new { my $self = shift; my $data = shift; - $self = {}; - $self->{user_id} = SOAP::Data->type('string', $data->user_id); - $self->{key_not_a_keyboard} = SOAP::Data->type('string', $data->key_not_a_keyboard); - $self->{timestamp} = SOAP::Data->type('string', $data->timestamp); + $self = {}; + $self->{user_id} = SOAP::Data->type('string', $data->user_id); + $self->{key} = SOAP::Data->type('string', $data->key); + $self->{timestamp} = SOAP::Data->type('string', $data->timestamp); bless $self; return $self; }