CodeIgniter4 icon indicating copy to clipboard operation
CodeIgniter4 copied to clipboard

Add $cached param to BaseConnection::tableExists()

Open sclubricants opened this issue 3 years ago • 4 comments

Follow-up #6249 This PR fixes https://github.com/codeigniter4/CodeIgniter4/issues/6351

In Forge::createTable()

        // If table exists lets stop here
        if ($ifNotExists === true && $this->db->tableExists($table, false)) {
            $this->reset();

            return true;
        }

This PR forces to check if table exists rather than relying on cache.

We add $tableName param to Connection::_listTables(). This allows us to lookup only one table rather than return a full list of tables.

Checklist:

  • [x] Securely signed commits
  • [x] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [x] Unit testing, with >80% coverage
  • [ ] User guide updated
  • [x] Conforms to style guide

sclubricants avatar Aug 09 '22 22:08 sclubricants

Any idea what is wrong with Mock?

1) CodeIgniter\Database\Forge\CreateTableTest::testCreateTableWithExists
Error: Class "Mock_MockQuery_c8b84d32" not found

/home/runner/work/CodeIgniter4/CodeIgniter4/system/Test/Mock/MockConnection.php:58
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Database/BaseConnection.php:1423
/home/runner/work/CodeIgniter4/CodeIgniter4/system/Database/Forge.php:501
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Database/Forge/CreateTableTest.php:39
    public function query(string $sql, $binds = null, bool $setEscapeFlags = true, string $queryClass = '')
    {
        $queryClass = str_replace('Connection', 'Query', static::class);

        $query = new $queryClass($this);

sclubricants avatar Aug 10 '22 01:08 sclubricants

  1. CodeIgniter\Database\Forge\CreateTableTest::testCreateTableWithExists Error: Class "Mock_MockQuery_c8b84d32" not found

Because it uses PHPUnit mock. The Mock_MockQuery_c8b84d32 does not exists.

Don't worry. The test is no longer of much use.

kenjis avatar Aug 10 '22 01:08 kenjis

I'll remove it and create a live test

sclubricants avatar Aug 10 '22 01:08 sclubricants

Well not sure what to do about it. Do I need to create a mock class?

No, you can't. The part c8b84d32 of the classname Mock_MockQuery_c8b84d32 will be changed randomly.

I'll remove it and create a live test

Yes, createTable() now depends on the SQL, so the test with mock does not make sense much.

kenjis avatar Aug 10 '22 01:08 kenjis

I added code that detects when cache is out of sync. If out of sync then we call resetDataCache().

However only if dataCache['table_names'] has been built already.

sclubricants avatar Aug 10 '22 22:08 sclubricants