Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29516 DB layer improvements 2.3 META
  3. MDL-29894

Prevent objects to be passed to moodle_database as params

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: Database SQL/XMLDB
    • Labels:
      None
    • Testing Instructions:
      Hide

      1/ run functional DB tests for all 5 supported drivers
      2/ try install for one DB type (they all throw the same errors now)
      3/ browse around a bit - please report any regressions here or as separate issues

      Show
      1/ run functional DB tests for all 5 supported drivers 2/ try install for one DB type (they all throw the same errors now) 3/ browse around a bit - please report any regressions here or as separate issues
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      w12_MDL-29894_m23_objectparams

      Description

      Before 2.2, the behavior of passing objects to moodle_database methods was undefined and, up to some point, the __toString() magic method was doing its work in a few cases.

      But it was not fully supported nor cross-db (MDL-29339) so finally it has been decided to be stricter and prevent (coding exception) any use of objects being passed as params. Caller should perform always the cast instead.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              timhunt Tim Hunt added a comment -

              Just got another bug report like MDL-29339. So, I thought it was not unreasonable to ping you about this bug Eloy.

              Show
              timhunt Tim Hunt added a comment - Just got another bug report like MDL-29339 . So, I thought it was not unreasonable to ping you about this bug Eloy.
              Hide
              skodak Petr Skoda added a comment -

              Hopefully this will not affect contrib much...

              Show
              skodak Petr Skoda added a comment - Hopefully this will not affect contrib much...
              Hide
              poltawski Dan Poltawski added a comment -

              Hi Petr,

              Thanks, that has been integrated now.

              Just some notes from my review:

              • Was there a reason you didn't use excpectException?
              • I think we should we also have unit tests for named params for the case where the logic ever changes there.

              Dan

              Show
              poltawski Dan Poltawski added a comment - Hi Petr, Thanks, that has been integrated now. Just some notes from my review: Was there a reason you didn't use excpectException? I think we should we also have unit tests for named params for the case where the logic ever changes there. Dan
              Hide
              skodak Petr Skoda added a comment -

              Hi Dan, the expectException allows one exception per method, another problem is that our simpletest was designed for PHP4, I hacked in basic exception support, but it was not working properly, so it is better to avoid using it for now.

              Show
              skodak Petr Skoda added a comment - Hi Dan, the expectException allows one exception per method, another problem is that our simpletest was designed for PHP4, I hacked in basic exception support, but it was not working properly, so it is better to avoid using it for now.
              Hide
              nebgor Aparup Banerjee added a comment -

              i'm not sure if this is related but its strange that i'm getting 'too many connections' errors on separate computers and separate DBs..

              pg:
              "Debug info:
              Warning: pg_connect(): Unable to connect to PostgreSQL server: FATAL: sorry, too many clients already in /home/aparup/mcode/integration/lib/dml/pgsql_native_moodle_database.php on line 164
              Stack trace:
              line 175 of /lib/dml/pgsql_native_moodle_database.php: dml_connection_exception thrown
              line 659 of /admin/tool/unittest/simpletestlib.php: call to pgsql_native_moodle_database->connect()
              line 616 of /lib/simpletestlib/test_case.php: call to UnitTestCaseUsingDatabase->__construct()
              line 620 of /lib/simpletestlib/test_case.php: call to TestSuite->run()
              line 53 of /admin/tool/unittest/ex_simple_test.php: call to TestSuite->run()
              line 142 of /admin/tool/unittest/simpletestcoveragelib.php: call to AutoGroupTest->run()
              line 119 of /admin/tool/unittest/index.php: call to autogroup_test_coverage->run()
              "

              ora:
              "Debug info: ORA-12516: TNS:listener could not find available handler with matching protocol stack
              Stack trace:
              line 209 of \lib\dml\oci_native_moodle_database.php: dml_connection_exception thrown
              line 659 of \admin\tool\unittest\simpletestlib.php: call to oci_native_moodle_database->connect()
              line 616 of \lib\simpletestlib\test_case.php: call to UnitTestCaseUsingDatabase->__construct()
              line 620 of \lib\simpletestlib\test_case.php: call to TestSuite->run()
              line 53 of \admin\tool\unittest\ex_simple_test.php: call to TestSuite->run()
              line 142 of \admin\tool\unittest\simpletestcoveragelib.php: call to AutoGroupTest->run()
              line 119 of \admin\tool\unittest\index.php: call to autogroup_test_coverage->run()
              "

              it might be my setups haven't been restarted in awhile? restarting everything and trying again.

              Show
              nebgor Aparup Banerjee added a comment - i'm not sure if this is related but its strange that i'm getting 'too many connections' errors on separate computers and separate DBs.. pg: "Debug info: Warning: pg_connect(): Unable to connect to PostgreSQL server: FATAL: sorry, too many clients already in /home/aparup/mcode/integration/lib/dml/pgsql_native_moodle_database.php on line 164 Stack trace: line 175 of /lib/dml/pgsql_native_moodle_database.php: dml_connection_exception thrown line 659 of /admin/tool/unittest/simpletestlib.php: call to pgsql_native_moodle_database->connect() line 616 of /lib/simpletestlib/test_case.php: call to UnitTestCaseUsingDatabase->__construct() line 620 of /lib/simpletestlib/test_case.php: call to TestSuite->run() line 53 of /admin/tool/unittest/ex_simple_test.php: call to TestSuite->run() line 142 of /admin/tool/unittest/simpletestcoveragelib.php: call to AutoGroupTest->run() line 119 of /admin/tool/unittest/index.php: call to autogroup_test_coverage->run() " ora: "Debug info: ORA-12516: TNS:listener could not find available handler with matching protocol stack Stack trace: line 209 of \lib\dml\oci_native_moodle_database.php: dml_connection_exception thrown line 659 of \admin\tool\unittest\simpletestlib.php: call to oci_native_moodle_database->connect() line 616 of \lib\simpletestlib\test_case.php: call to UnitTestCaseUsingDatabase->__construct() line 620 of \lib\simpletestlib\test_case.php: call to TestSuite->run() line 53 of \admin\tool\unittest\ex_simple_test.php: call to TestSuite->run() line 142 of \admin\tool\unittest\simpletestcoveragelib.php: call to AutoGroupTest->run() line 119 of \admin\tool\unittest\index.php: call to autogroup_test_coverage->run() " it might be my setups haven't been restarted in awhile? restarting everything and trying again.
              Hide
              skodak Petr Skoda added a comment -

              Could you please try previous week master? I think I saw this kind of problem before, but I do not remember the details. I think it is not related.

              Show
              skodak Petr Skoda added a comment - Could you please try previous week master? I think I saw this kind of problem before, but I do not remember the details. I think it is not related.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Hi Apu, did you have any luck working this one out?
              I've just tested a fresh install, an upgrade, and run the function DB tests on PG and everything has worked fine.
              I don't have an Oracle installation at the moment so can't test that sorry.

              Show
              samhemelryk Sam Hemelryk added a comment - Hi Apu, did you have any luck working this one out? I've just tested a fresh install, an upgrade, and run the function DB tests on PG and everything has worked fine. I don't have an Oracle installation at the moment so can't test that sorry.
              Hide
              nebgor Aparup Banerjee added a comment -

              yes i'm getting this on moodle.git master too so its unrelated. i'll open another issue.

              Show
              nebgor Aparup Banerjee added a comment - yes i'm getting this on moodle.git master too so its unrelated. i'll open another issue.
              Hide
              nebgor Aparup Banerjee added a comment - - edited

              issue created for connection problems during unit test. MDL-32152

              Show
              nebgor Aparup Banerjee added a comment - - edited issue created for connection problems during unit test. MDL-32152
              Hide
              nebgor Aparup Banerjee added a comment -

              passing this based on no problems seen in Sam's setup of pg.
              also no problems seen here on mysql and mssql.

              Show
              nebgor Aparup Banerjee added a comment - passing this based on no problems seen in Sam's setup of pg. also no problems seen here on mysql and mssql.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Congratulations are in order, you've made it, or at least your code has!
              It's now part of Moodle and both the git and cvs repositories have been updated.

              This issue is being marked as fixed and closed.

              Thank you.

              Show
              samhemelryk Sam Hemelryk added a comment - Congratulations are in order, you've made it, or at least your code has! It's now part of Moodle and both the git and cvs repositories have been updated. This issue is being marked as fixed and closed. Thank you.

                People

                • Votes:
                  0 Vote for this issue
                  Watchers:
                  3 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    25/Jun/12