ImportUser only one transaction for all imports

[someplace53]

I have the question why the ImportUsers scheduled task does all the imports in one transaction? In my opinion it would be more convenient if every import would be its own transaction (than you should be able to remove the transaction statement).

1. The transaction is blocking the table for quiet some time (especially if you have a lot of groups), which can block logins
2. It is a bit annoying if one of the groups fails non of the groups will be synchronized
3. if an import fails, the log is only written to the log file, which at least I only check seldomly
4. why check all groups if already one failed?

It should not be hard to remove the transaction and I would be willing to do that, but maybe there are reason why I should not? The problem with the log I do not know how to fix and so I would not.

[xperseguers]

The rationale for doing a single transaction is that we need to first delete/disable existing accounts to ensure removed accounts from LDAP are reflected in TYPO3.

What could certainly be done however is to move the transaction within the loop on the configurations:

Instead of:

        $tableConnection->beginTransaction();

        // Loop on each configuration and context and import the related users
        $failures = 0;
        foreach ($ldapConfigurations as $configuration) {
            foreach ($executionContexts as $aContext) {
                // snip
            }
        }

        // If some failures were registered, rollback the whole transaction and report error
        if ($failures > 0) {
            $tableConnection->rollBack();
            $message = 'Some or all imports failed. Synchronisation was aborted. Check your settings or your network connection';
            $this->getLogger()->error($message);
            throw new ImportUsersException($message, 1410774015);

        } else {
            // Everything went fine, commit the changes
            $tableConnection->commit();
        }

have this instead:

        // Loop on each configuration and context and import the related users
        foreach ($ldapConfigurations as $configuration) {
            foreach ($executionContexts as $aContext) {
                $failures = 0;
                $tableConnection->beginTransaction();

                // snip

                // If some failures were registered, rollback the whole transaction and report error
                if ($failures > 0) { ... }
            }
        }

what do you think?

[someplace53]

This would be an improvement :-) and would work for me. I assume you are aware of the missing commit() and the continue statement, which might make this a bit more complicate.