Skip to content

Conversation

@mtillberg
Copy link
Contributor

@mtillberg mtillberg commented May 20, 2023

Description

Fix to PreviewCleanup query

Related Issue

Motivation and Context

Keeps the faster response time, returns the correct data, prevents unnecessary thumbnail deletes/recreations.

How Has This Been Tested?

  • test environment: Docker owncloud/server:10.12 (current version 10.12.1.3)
    Updated lib/private/PreviewCleanup.php with new query
    ran occ previews:cleanup
    saw that only the correct thunbnails were deleted.
    Test run against MariaDB 10.11.3

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@CLAassistant
Copy link

CLAassistant commented May 20, 2023

CLA assistant check
All committers have signed the CLA.

Copy link

@pako81 pako81 left a comment

Choose a reason for hiding this comment

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

Manually tested against MariaDB and can indeed confirm only relevant thumbnails are removed. However, CI fails at https://drone.owncloud.com/owncloud/core/38529/30/8. Maybe special care has to be taken for the query when running it on Oracle DB.

@owncloud owncloud deleted a comment from update-docs bot May 28, 2023
@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/38533/20/8

There was 1 error:
1) Test\PreviewCleanupTest::test
Doctrine\DBAL\Exception\SyntaxErrorException: An exception occurred while executing 'select `thumb`.`fileid`, `thumb`.`name`, `user_id`
 from `oc_mounts`, `oc_filecache` `thumb` left join `oc_filecache` `file`  on `thumb`.`name` = cast(`file`.`fileid` as VARCHAR(24))
where `thumb`.`parent` in (select `fileid` from `oc_filecache` where `storage` in (select `numeric_id` from `oc_storages` where `id` like 'home::%' or `id` like 'object::user:%') and `path` = 'thumbnails')
  and `oc_mounts`.`storage_id` = `thumb`.`storage`
  and `file`.`fileid` is null
  and `thumb`.`fileid` > ?
  order by `user_id`, `thumb`.`fileid` limit 1000' with params [0]:

SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'VARCHAR(24))
where `thumb`.`parent` in (select `fileid` from `oc_filecache` wher' at line 2

/drone/src/lib/composer/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php:98
/drone/src/lib/composer/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:182
/drone/src/lib/composer/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:159
/drone/src/lib/composer/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:2226
/drone/src/lib/composer/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:1313
/drone/src/lib/private/DB/Connection.php:191
/drone/src/lib/private/PreviewCleanup.php:97
/drone/src/lib/private/PreviewCleanup.php:43
/drone/src/tests/lib/PreviewCleanupTest.php:106
phpvfscomposer:///drone/src/lib/composer/phpunit/phpunit/phpunit:106

And locally I get:

$ php occ previews:cleanup

In AbstractMySQLDriver.php line 98:
                                                                                                                                                                    
  An exception occurred while executing 'select `thumb`.`fileid`, `thumb`.`name`, `user_id`                                                                         
   from `oc_mounts`, `oc_filecache` `thumb` left join `oc_filecache` `file`  on `thumb`.`name` = cast(`file`.`fileid` as VARCHAR(24))                               
  where `thumb`.`parent` in (select `fileid` from `oc_filecache` where `storage` in (select `numeric_id` from `oc_storages` where `id` like 'home::%' or `id` like  
   'object::user:%') and `path` = 'thumbnails')                                                                                                                     
    and `oc_mounts`.`storage_id` = `thumb`.`storage`                                                                                                                
    and `file`.`fileid` is null                                                                                                                                     
    and `thumb`.`fileid` > ?                                                                                                                                        
    order by `user_id`, `thumb`.`fileid` limit 1000' with params [0]:                                                                                               
                                                                                                                                                                    
  SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for  
   the right syntax to use near 'VARCHAR(24))                                                                                                                       
  where `thumb`.`parent` in (select `fileid` from `oc_filecache` wher' at line 2                                                                                    
                                                                                                                                                                    
In Exception.php line 18:
                                                                                                                                                                    
  SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for  
   the right syntax to use near 'VARCHAR(24))                                                                                                                       
  where `thumb`.`parent` in (select `fileid` from `oc_filecache` wher' at line 2                                                                                    
                                                                                                                                                                    
In PDOStatement.php line 117:
                                                                                                                                                                    
  SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for  
   the right syntax to use near 'VARCHAR(24))                                                                                                                       
  where `thumb`.`parent` in (select `fileid` from `oc_filecache` wher' at line 2                                                                                    

previews:cleanup [--output [OUTPUT]] [--all] [--] [<chunk_size>]

I am using mySQL - the unit tests on mySQL and the oldest mariaDB 10.2 are failing. They do not seem to like VARCHAR

@phil-davis phil-davis requested review from jvillafanez and pako81 May 28, 2023 13:42
@phil-davis
Copy link
Contributor

phil-davis commented May 28, 2023

Looks good. I have some acceptance test scenarios that fail on master and pass with this fixed code - good. I have put those in a separate PR #40816 so as not to mess up your branch.

@mtillberg you could squash the commits here - the code is pretty tidy now, so I don't think that we need the "development commit history".

@phil-davis
Copy link
Contributor

phil-davis commented Jun 8, 2023

I squashed and rebased. Anyone please review - maybe @jvillafanez

This fixes/improves the cleanup of previews/thumbnails. PR #40816 has acceptance test scenarios that confirm that this works. That PR can be merged straight after this one.

@phil-davis phil-davis self-assigned this Jun 8, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 8, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

42.6% 42.6% Coverage
0.9% 0.9% Duplication

@phil-davis phil-davis merged commit 1d014d1 into owncloud:master Jun 8, 2023
@mtillberg mtillberg deleted the preview-cleanup branch June 8, 2023 17:02
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.

PreviewCleanup query is incorrect

5 participants