owncloud / data_exporter

Export/Import for ownCloud user data
GNU General Public License v2.0
7 stars 5 forks source link

[QA] data_exporter fails with setIgnoredAttributes() call to undefined method. #232

Closed jnweiger closed 11 months ago

jnweiger commented 1 year ago

Seen with owncloud core 10.12.2 and data-exporter-0.3.0-rc.1

occ instance:export:user admin /tmp/xxx
An unhandled exception has been thrown:
Error: Call to undefined method Symfony\Component\Serializer\Normalizer\ObjectNormalizer::setIgnoredAttributes() in /var/www/owncloud/apps-external/data_exporter/lib/Serializer.php:74
Stack trace:
#0 /var/www/owncloud/apps-external/data_exporter/lib/Utilities/StreamHelper.php(92): OCA\DataExporter\Serializer->setIgnoredAttributes()
#1 /var/www/owncloud/apps-external/data_exporter/lib/Extractor/MetadataExtractor/FilesMetadataExtractor.php(79): OCA\DataExporter\Utilities\StreamHelper->writelnToStream()
#2 /var/www/owncloud/apps-external/data_exporter/lib/Extractor/MetadataExtractor.php(111): OCA\DataExporter\Extractor\MetadataExtractor\FilesMetadataExtractor->extract()
#3 /var/www/owncloud/apps-external/data_exporter/lib/Exporter.php(84): OCA\DataExporter\Extractor\MetadataExtractor->extract()
#4 /var/www/owncloud/apps-external/data_exporter/lib/Command/ExportUser.php(77): OCA\DataExporter\Exporter->export()
#5 /var/www/owncloud/lib/composer/symfony/console/Command/Command.php(255): OCA\DataExporter\Command\ExportUser->execute()
#6 /var/www/owncloud/lib/composer/symfony/console/Application.php(1021): Symfony\Component\Console\Command\Command->run()
#7 /var/www/owncloud/lib/composer/symfony/console/Application.php(275): Symfony\Component\Console\Application->doRunCommand()
#8 /var/www/owncloud/lib/composer/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun()
#9 /var/www/owncloud/lib/private/Console/Application.php(165): Symfony\Component\Console\Application->run()
#10 /var/www/owncloud/console.php(116): OC\Console\Application->run()
#11 /var/www/owncloud/occ(11): require_once('/var/www/ownclo...')

seems the setIgnoredAttributes() call is deprecated. Some google showed me code like this:

     $legacy ? $this->normalizer->setIgnoredAttributes($ignoredAttributes) : $this->createNormalizer([ObjectNormalizer::IGNORED_ATTRIBUTES => $ignoredAttributes]);

do we need to do something similar?

phil-davis commented 1 year ago

I have deja-vu about this. I am sure that I have done something like this in some other app or test - looking...

phil-davis commented 1 year ago

https://github.com/owncloud/files_classifier/pull/755/files

From memory, Symfony4 supports both setIgnoredAttributes() (1) (which is deprecated), and the [ObjectNormalizer::IGNORED_ATTRIBUTES => $ignoredAttributes] (2) way of doing this.

So we can change to using (2) and it will work for both Symfony releases.

I will code something, and also try to see why we do not have any failing tests in CI.

phil-davis commented 1 year ago

data_exporter is using Symfony3, and we left it at Symfony3 so that it will keep working with older oC core releases etc.

occ instance:export:user admin /tmp/xxx works for me in core master.

The tarball of https://github.com/owncloud/data_exporter/releases/tag/v0.3.0-rc.1 has vendor/symfony/serializer/Normalizer with ObjectNormalizer, which extends AbstractObjectNormalizer, which extends AbstractNormalizer.

AbstractNormalizer has:

    /**
     * Set ignored attributes for normalization and denormalization.
     *
     * @return self
     */
    public function setIgnoredAttributes(array $ignoredAttributes)
    {
        $this->ignoredAttributes = $ignoredAttributes;

        return $this;
    }

But vendor/composer/autoload_real.php has different code than the one that composer generates for me locally.

Thinking...

phil-davis commented 1 year ago

I was using: Composer version 2.3.10 2022-07-13 15:48:23

I have updated to: Composer version 2.5.8 2023-06-09 17:13:21

@jnweiger what version of composer is used when you make the release tarball?

phil-davis commented 1 year ago

https://drone.owncloud.com/owncloud/data_exporter/2584/20/4 Nightly CI runs with core latest Fetched 10.12.2 - channel: stable - build at 2023-06-06T15:14:30+00:00

https://drone.owncloud.com/owncloud/data_exporter/2584/20/11 The acceptance tests all run and pass - various combinations of export and import.

I unzipped the 0.3.0 RC tarball into a local core master, and export commands work fine for me. I will need to locally install a 10.12.2 (rather than my development core master) and see if I can reproduce the bad behavior.

jnweiger commented 1 year ago

@jnweiger what version of composer is used when you make the release tarball?

+ docker run --rm -it -v ****:/root/.owncloud/certificates -v ****:/var/www/owncloud/repo/data_exporter registry.owncloud.com/internal/releasesigning:latest /bin/bash -c 'echo '\''{ "allow_root": true }'\'' > /root/.bowerrc && composer --version && cd /var/www/owncloud/repo/data_exporter && make -f Makefile dist'
Composer version 2.1.6 2021-08-19 17:11:08
/usr/bin/composer install --no-dev
Installing dependencies from lock file
Verifying lock file contents can be installed on current platform.
Package operations: 13 installs, 0 updates, 0 removals
  - Downloading paragonie/random_compat (v2.0.21)
  - Downloading symfony/polyfill-ctype (v1.19.0)
  - Downloading webmozart/assert (1.9.1)
  - Downloading phpdocumentor/reflection-common (1.0.1)
  - Downloading phpdocumentor/type-resolver (0.4.0)
  - Downloading phpdocumentor/reflection-docblock (3.3.2)
  - Downloading symfony/filesystem (v3.4.47)
  - Downloading symfony/options-resolver (v3.4.47)
  - Downloading symfony/polyfill-php70 (v1.19.0)
  - Downloading symfony/inflector (v3.4.47)
  - Downloading symfony/property-access (v3.4.47)
  - Downloading symfony/property-info (v3.4.47)
  - Downloading symfony/serializer (v3.4.47)
  - Installing paragonie/random_compat (v2.0.21): Extracting archive
  - Installing symfony/polyfill-ctype (v1.19.0): Extracting archive
  - Installing webmozart/assert (1.9.1): Extracting archive
  - Installing phpdocumentor/reflection-common (1.0.1): Extracting archive
  - Installing phpdocumentor/type-resolver (0.4.0): Extracting archive
  - Installing phpdocumentor/reflection-docblock (3.3.2): Extracting archive
  - Installing symfony/filesystem (v3.4.47): Extracting archive
  - Installing symfony/options-resolver (v3.4.47): Extracting archive
  - Installing symfony/polyfill-php70 (v1.19.0): Extracting archive
  - Installing symfony/inflector (v3.4.47): Extracting archive
  - Installing symfony/property-access (v3.4.47): Extracting archive
  - Installing symfony/property-info (v3.4.47): Extracting archive
  - Installing symfony/serializer (v3.4.47): Extracting archive
Package symfony/inflector is abandoned, you should avoid using it. Use EnglishInflector from the String component instead.
Generating autoload files
8 packages you are using are looking for funding.
Use the `composer fund` command to find out more!

:thinking: so, is this composer 2.1.6 too old?

phil-davis commented 1 year ago

I have a feeling that composer 2.2 had new features to support "new stuff" - I would have to investigate. I had 2.3 and then updated to 2.5 today.

I think that somehow the occ command is finding a symfony/serializer that is different and does not have setIgnoredAttributes - but it makes no sense to me. setIgnoredAttributes should exist in Symfony3 and Symfony4 - so when you run core 10.12 (Symfony4) and data_exporter (Symfony3) then setIgnoredAttributes is in either vendor... directory.

Anyway, I think that we want to generate the latest autoloader code, using the the latest composer version. So try making a tarball that has vendor created by the latest composer.

I could update my composer on Ubuntu with sudo composer --self-update

jnweiger commented 1 year ago

Second RC built with composer 2.5.8 after composer self-update

https://github.com/owncloud/data_exporter/releases/download/v0.3.0-rc.2/data_exporter-0.3.0-rc.2.tar.gz

@GeraldLeikam Is this any better?

GeraldLeikam commented 1 year ago

Unfortunately, it didn't help. The same problem with the "ObjectNormalizer" still persists.

root@jw-oc10122-activity-272rc1-20230711-cte:/var/www/owncloud# occ instance:export:user admin exports/
An unhandled exception has been thrown:
Error: Call to undefined method Symfony\Component\Serializer\Normalizer\ObjectNormalizer::setIgnoredAttributes() in /var/www/owncloud/apps-external/data_exporter/lib/Serializer.php:74
Stack trace:
#0 /var/www/owncloud/apps-external/data_exporter/lib/Utilities/StreamHelper.php(92): OCA\DataExporter\Serializer->setIgnoredAttributes()
#1 /var/www/owncloud/apps-external/data_exporter/lib/Extractor/MetadataExtractor/FilesMetadataExtractor.php(79): OCA\DataExporter\Utilities\StreamHelper->writelnToStream()
#2 /var/www/owncloud/apps-external/data_exporter/lib/Extractor/MetadataExtractor.php(111): OCA\DataExporter\Extractor\MetadataExtractor\FilesMetadataExtractor->extract()
#3 /var/www/owncloud/apps-external/data_exporter/lib/Exporter.php(84): OCA\DataExporter\Extractor\MetadataExtractor->extract()
#4 /var/www/owncloud/apps-external/data_exporter/lib/Command/ExportUser.php(77): OCA\DataExporter\Exporter->export()
#5 /var/www/owncloud/lib/composer/symfony/console/Command/Command.php(255): OCA\DataExporter\Command\ExportUser->execute()
#6 /var/www/owncloud/lib/composer/symfony/console/Application.php(1021): Symfony\Component\Console\Command\Command->run()
#7 /var/www/owncloud/lib/composer/symfony/console/Application.php(275): Symfony\Component\Console\Application->doRunCommand()
#8 /var/www/owncloud/lib/composer/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun()
#9 /var/www/owncloud/lib/private/Console/Application.php(165): Symfony\Component\Console\Application->run()
#10 /var/www/owncloud/console.php(116): OC\Console\Application->run()
#11 /var/www/owncloud/occ(11): require_once('/var/www/ownclo...')
#12 {main}root@jw-oc10122-activity-272rc1-20230711-cte:/var/www/owncloud#
jnweiger commented 1 year ago

@phil-davis I'd say, composer is innocent in this case. Could this be a code mix-up somewhere? old symphony code is loaded by some other app, ... maybe?

phil-davis commented 1 year ago

I will try again to reproduce later today.

phil-davis commented 1 year ago

I did:

It works fine.

$ ls -l /tmp/aaa1/admin/
total 12
drwxrwxr-x 5 phil phil 4096 Jul 14 14:44 files
-rw-rw-r-- 1 phil phil 1932 Jul 14 14:44 files.jsonl
-rw-rw-r-- 1 phil phil    0 Jul 14 14:44 shares.jsonl
-rw-rw-r-- 1 phil phil  501 Jul 14 14:44 user.json
$ ls -l /tmp/aaa1/admin/files
total 12
drwxrwxr-x 2 phil phil 4096 Jul 14 14:44  Documents
drwxrwxr-x 2 phil phil 4096 Jul 14 14:44 'Learn more about ownCloud'
drwxrwxr-x 2 phil phil 4096 Jul 14 14:44  Photos

I also tried putting it in the apps folder and it works from there also.

Maybe you have some other app(s) enabled? Or??? - it will be good to find out what is the cause.

GeraldLeikam commented 1 year ago

I think I found the mistake. Encryption is enabled on my and Jürgen's machine. Could that be causing the error?

pako81 commented 1 year ago

Tested on 10.12.2 + https://github.com/owncloud/data_exporter/releases/tag/v0.3.0-rc.2 with and without master-key encryption: issue is not reproducible.

jnweiger commented 1 year ago

No, it is not encryption. it is files_classifier. We disable this app, and get a clean export. we enable this app, and get Gerald's crash.


root@jw-oc10122-activity-272rc1-20230711-cte:~# occ instance:export:user bob /tmp/aaa7
root@jw-oc10122-activity-272rc1-20230711-cte:~# occ app:ena files_classifier
files_classifier enabled
root@jw-oc10122-activity-272rc1-20230711-cte:~# occ instance:export:user bob /tmp/aaa8
An unhandled exception has been thrown:
Error: Call to undefined method Symfony\Component\Serializer\Normalizer\ObjectNormalizer::setIgnoredAttributes() in /var/www/owncloud/apps-external/data_exporter/lib/Serializer.php:74
Stack trace:
#0 /var/www/owncloud/apps-external/data_exporter/lib/Utilities/StreamHelper.php(92): OCA\DataExporter\Serializer->setIgnoredAttributes()
#1 /var/www/owncloud/apps-external/data_exporter/lib/Extractor/MetadataExtractor/FilesMetadataExtractor.php(79): OCA\DataExporter\Utilities\StreamHelper->writelnToStream()
#2 /var/www/owncloud/apps-external/data_exporter/lib/Extractor/MetadataExtractor.php(111): OCA\DataExporter\Extractor\MetadataExtractor\FilesMetadataExtractor->extract()
#3 /var/www/owncloud/apps-external/data_exporter/lib/Exporter.php(84): OCA\DataExporter\Extractor\MetadataExtractor->extract()
#4 /var/www/owncloud/apps-external/data_exporter/lib/Command/ExportUser.php(77): OCA\DataExporter\Exporter->export()
#5 /var/www/owncloud/lib/composer/symfony/console/Command/Command.php(255): OCA\DataExporter\Command\ExportUser->execute()
#6 /var/www/owncloud/lib/composer/symfony/console/Application.php(1021): Symfony\Component\Console\Command\Command->run()
#7 /var/www/owncloud/lib/composer/symfony/console/Application.php(275): Symfony\Component\Console\Application->doRunCommand()
#8 /var/www/owncloud/lib/composer/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun()
#9 /var/www/owncloud/lib/private/Console/Application.php(165): Symfony\Component\Console\Application->run()
#10 /var/www/owncloud/console.php(116): OC\Console\Application->run()
#11 /var/www/owncloud/occ(11): require_once('/var/www/ownclo...')
#12 {main}root@jw-oc10122-activity-272rc1-20230711-cte:~# occ app:dis files_classifier
files_classifier disabled
root@jw-oc10122-activity-272rc1-20230711-cte:~# occ instance:export:user bob /tmp/aaa9
root@jw-oc10122-activity-272rc1-20230711-cte:~# 
pako81 commented 1 year ago

cannot confirm:


root@Pasquale-Ubuntu-20-04:/var/www/html/ownclouddataexporter# sudo -u www-data php occ app:enable files_classifier
files_classifier enabled
root@Pasquale-Ubuntu-20-04:/var/www/html/ownclouddataexporter# sudo -u www-data php occ instance:export:user user3 /tmp/user3
root@Pasquale-Ubuntu-20-04:/var/www/html/ownclouddataexporter# ls /tmp/user
user1/ user2/ user3/
jnweiger commented 1 year ago

yep. files_classifier-1.3.2 is harmless. The new 1.4.0-rc.1 or 1.4.0-rc.2 cause the explosion.

pako81 commented 1 year ago

yep. files_classifier-1.3.2 is harmless. The new 1.4.0-rc.1 or 1.4.0-rc.2 cause the explosion.

confirmed 💥

phil-davis commented 1 year ago

Happens with current core master in my development environment:

phil@phil-Inspiron-5468:~/git/owncloud/core$ cd apps-external/files_classifier/
phil@phil-Inspiron-5468:~/git/owncloud/core/apps-external/files_classifier$ composer install
Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Package operations: 20 installs, 0 updates, 0 removals
  - Downloading symfony/serializer (v5.4.25)
  - Downloading symfony/validator (v5.4.25)
  - Installing bamarni/composer-bin-plugin (1.8.2): Extracting archive
  - Installing laminas/laminas-xml (1.4.0): Extracting archive
  - Installing symfony/polyfill-ctype (v1.27.0): Extracting archive
  - Installing webmozart/assert (1.11.0): Extracting archive
  - Installing phpdocumentor/reflection-common (2.2.0): Extracting archive
  - Installing phpdocumentor/type-resolver (1.6.1): Extracting archive
  - Installing phpdocumentor/reflection-docblock (5.3.0): Extracting archive
  - Installing symfony/polyfill-php80 (v1.27.0): Extracting archive
  - Installing symfony/polyfill-mbstring (v1.27.0): Extracting archive
  - Installing symfony/polyfill-intl-normalizer (v1.27.0): Extracting archive
  - Installing symfony/polyfill-intl-grapheme (v1.27.0): Extracting archive
  - Installing symfony/string (v5.4.22): Extracting archive
  - Installing symfony/deprecation-contracts (v2.5.2): Extracting archive
  - Installing symfony/property-info (v5.4.24): Extracting archive
  - Installing symfony/property-access (v5.4.22): Extracting archive
  - Installing symfony/serializer (v5.4.25): Extracting archive
  - Installing symfony/translation-contracts (v2.5.2): Extracting archive
  - Installing symfony/polyfill-php81 (v1.27.0): Extracting archive
  - Installing symfony/polyfill-php73 (v1.27.0): Extracting archive
  - Installing symfony/validator (v5.4.25): Extracting archive
Generating autoload files
[bamarni-bin] The setting "extra.bamarni-bin.forward-command" will be set to "true" from 2.x onwards. If you wish to keep it to "false", you need to set it explicitly.
15 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
phil@phil-Inspiron-5468:~/git/owncloud/core/apps-external/files_classifier$ cd ../data_exporter/
phil@phil-Inspiron-5468:~/git/owncloud/core/apps-external/data_exporter$ composer install
Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Package operations: 14 installs, 0 updates, 0 removals
  - Installing bamarni/composer-bin-plugin (v1.5.0): Extracting archive
  - Installing paragonie/random_compat (v2.0.21): Extracting archive
  - Installing symfony/polyfill-ctype (v1.19.0): Extracting archive
  - Installing webmozart/assert (1.9.1): Extracting archive
  - Installing phpdocumentor/reflection-common (1.0.1): Extracting archive
  - Installing phpdocumentor/type-resolver (0.4.0): Extracting archive
  - Installing phpdocumentor/reflection-docblock (3.3.2): Extracting archive
  - Installing symfony/filesystem (v3.4.47): Extracting archive
  - Installing symfony/options-resolver (v3.4.47): Extracting archive
  - Installing symfony/polyfill-php70 (v1.19.0): Extracting archive
  - Installing symfony/inflector (v3.4.47): Extracting archive
  - Installing symfony/property-access (v3.4.47): Extracting archive
  - Installing symfony/property-info (v3.4.47): Extracting archive
  - Installing symfony/serializer (v3.4.47): Extracting archive
Package symfony/inflector is abandoned, you should avoid using it. Use EnglishInflector from the String component instead.
Generating autoload files
8 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
phil@phil-Inspiron-5468:~/git/owncloud/core/apps-external/data_exporter$ cd ..
phil@phil-Inspiron-5468:~/git/owncloud/core/apps-external$ cd ..
phil@phil-Inspiron-5468:~/git/owncloud/core$ php occ a:e data_exporter
data_exporter enabled
phil@phil-Inspiron-5468:~/git/owncloud/core$ php occ instance:export:user admin /tmp/aaa3
phil@phil-Inspiron-5468:~/git/owncloud/core$ php occ a:e files_classifier
files_classifier enabled
phil@phil-Inspiron-5468:~/git/owncloud/core$ php occ instance:export:user admin /tmp/aaa4
An unhandled exception has been thrown:
Error: Call to undefined method Symfony\Component\Serializer\Normalizer\ObjectNormalizer::setIgnoredAttributes() in /home/phil/git/owncloud/core/apps-external/data_exporter/lib/Serializer.php:74
Stack trace:
#0 /home/phil/git/owncloud/core/apps-external/data_exporter/lib/Utilities/StreamHelper.php(92): OCA\DataExporter\Serializer->setIgnoredAttributes()
#1 /home/phil/git/owncloud/core/apps-external/data_exporter/lib/Extractor/MetadataExtractor/FilesMetadataExtractor.php(79): OCA\DataExporter\Utilities\StreamHelper->writelnToStream()
#2 /home/phil/git/owncloud/core/apps-external/data_exporter/lib/Extractor/MetadataExtractor.php(111): OCA\DataExporter\Extractor\MetadataExtractor\FilesMetadataExtractor->extract()
#3 /home/phil/git/owncloud/core/apps-external/data_exporter/lib/Exporter.php(84): OCA\DataExporter\Extractor\MetadataExtractor->extract()
#4 /home/phil/git/owncloud/core/apps-external/data_exporter/lib/Command/ExportUser.php(77): OCA\DataExporter\Exporter->export()
#5 /home/phil/git/owncloud/core/lib/composer/symfony/console/Command/Command.php(298): OCA\DataExporter\Command\ExportUser->execute()
#6 /home/phil/git/owncloud/core/lib/composer/symfony/console/Application.php(1040): Symfony\Component\Console\Command\Command->run()
#7 /home/phil/git/owncloud/core/lib/composer/symfony/console/Application.php(301): Symfony\Component\Console\Application->doRunCommand()
#8 /home/phil/git/owncloud/core/lib/composer/symfony/console/Application.php(171): Symfony\Component\Console\Application->doRun()
#9 /home/phil/git/owncloud/core/lib/private/Console/Application.php(165): Symfony\Component\Console\Application->run()
#10 /home/phil/git/owncloud/core/console.php(116): OC\Console\Application->run()
#11 /home/phil/git/owncloud/core/occ(11): require_once('/home/phil/git/...')
#12 {main}phil@phil-Inspiron-5468:~/git/owncloud/core$ 

I suppose that there is some "magic" in how the composer autoload sequence tries to find the referenced class, and it must have loaded a class definition that comes from files_classifier, and that does not have the old setIgnoredAttributes method.

I will play around and see if there is some way to help composer autoload to be smarter.

pako81 commented 1 year ago

@phil-davis yes, it seems the symfony/serializer from files_classifier is loaded and there AbstractNormalizer::setIgnoredAttributes() has been removed?

See 5.0.0 changelog in https://github.com/owncloud/files_classifier/pull/583, which was superseded by https://github.com/owncloud/files_classifier/pull/763 (merged).

phil-davis commented 1 year ago

Yes, Symfony5 does not have setIgnoredAttributes() any more. We need to either: 1) discover how to make sure that when running an occ command from core, that the dependencies are only loaded from the app where the Symfony console command is found. (and not from some other app that might have different versions of those dependencies), or;

2) merge #222 to make data_exporter use Symfony5.

(1) might be quite tricky to ensure that we get it right.

(2) is easy and will work. If we decide to do (2), then the new data_exporter release should still work fine with oC10 core releases that have Symfony4 (lots of 10.* releases, including 10.11 10.12 and the upcoming 10.13).

Before doing that, we should check for any unreleased little bits of code in data_exporter master, and release that as 0.2.2. Then 0.2.2 can be used with lots of old core like 9. and 10. right up to 10.12. I can see #221 and #227 that in in master but not released yet - first releasing those as 0.2.2 might be good.

jnweiger commented 1 year ago

Are #221 and #227 the correct PRs for a 0.2.2 release? I'd assume #221 should be avoided, and maybe it is #227 and #229 instead?

phil-davis commented 1 year ago

PR #221 is fine - we should have always returned an int from the execute() method. That ensures that the command line exit status gets set properly. Symfony4 did not enforce that, but it was always the right thing to do.

phil-davis commented 1 year ago

IMO we can release the current master as 0.2.2 to get those things out of the way, and have those fixes available to anyone using data_exporter on existing/older oC10.

jnweiger commented 1 year ago

Understood. So this should do the trick: https://github.com/owncloud/data_exporter/releases/download/v0.2.2-rc.1/data_exporter-0.2.2-rc.1.tar.gz @GeraldLeikam Does it look good?

jnweiger commented 11 months ago

Compatibility:

jnweiger commented 11 months ago

Confirmed fixed with 0.3.0 and 10.13.0-beta.1