Mark Discussion as Helpful Reply

Developing (v7+)

Optimising code 8.2.1 User Import

Permalink December 21, 2017 at 10:15 AM

I am trying to be as efficient as a I can using a CSV to import users. I have a list of 700+ users and i've created an upload screen to take in a CSV and process the creation of new users. Code below.

Now I have ran the code several times (and then removed the users) to test how long its going to take for users of the system to generate a large list of customers. Currently its taking around 16.34 seconds per user!! which is obviously a huge amount of time when you're importing 700+ (3Hrs 24Mins total it took).

Can anyone make any suggestions for optimisations to speed things up, any rookie mistakes I've made. If you can explain why that would really help my understanding.

public function handleSubmit()
{
    $l = new Logger(LOG_TYPE_EXCEPTIONS);
    $l->info("");
    $l->info("Start User Import");
    $recordProcess = [];
    $eachUserCount = 0;
    $RegionEntities = Express::getList('region');
    $FacilityEntities = Express::getList('facility');
    $this->connImport = $this->getDB();
    $this->entityManager = \Core::make('database / orm')->entityManager();
    $userRegistrationService = \Core::make('\Concrete\Core\User\RegistrationServiceInterface');
    $userinfo = array();
    $htmlReturn = "";
    $token = $this->app->make('token');

Viewing 15 lines of 149 lines. View entire code block.

 
public function handleSubmit()
{
 
    $l = new Logger(LOG_TYPE_EXCEPTIONS);
    $l->info("");
    $l->info("Start User Import");
    $recordProcess = [];
    $eachUserCount = 0;
 
    $RegionEntities = Express::getList('region');
 
    $FacilityEntities = Express::getList('facility');
 
    $this->connImport = $this->getDB();
    $this->entityManager = \Core::make('database / orm')->entityManager();
 
    $userRegistrationService = \Core::make('\Concrete\Core\User\RegistrationServiceInterface');
    $userinfo = array();
    $htmlReturn = "";
 
    $token = $this->app->make('token');
 
    if (!$token->validate()) {
        throw new UserMessageException($token->getErrorMessage());
    }
 
    $file = $this->request->files->get('file');
    if (!($file instanceof UploadedFile)) {
        throw new UserMessageException(t('File not received . '));
    }
 
    if (!$file->isValid()) {
        throw new UserMessageException($file->getErrorMessage());
    }
    if ($file->getPathname()) {
        $fh = fopen($file->getPathname(), 'r');
 
    } else {
        return "File path not found";
    }
    if ($fh) {
 
        $sql = "TRUNCATE TABLE Users";
 
        $result = $this->connImport->query($sql);
 
        $result = $this->loadDataReplace($this->connImport, $file, 'UserDatabase . Users', 2);
 
        if ($result) {
            $dataLoaded = "Data loaded successfully into Users table\n";
            $sql = "call process_Users()";
 
            $result2 = $this->connImport->query($sql);
            if ($result2) {
                $dataLoaded .= "Users processed successfully. ";
            } else {
                $dataLoaded .= "There was a problem processing the Users: " . mysqli_error($this->connImport);
            }
        } else {
            $dataLoaded = 'There was a problem loading the data into Users table . ' . mysqli_error($this->connImport);
        }
 
        $htmlReturn .= "<table class='table display'>
                    <thead>
                    <tr>
                    <td>User Name</td>
                    <td>User Email</td>
                    <td>User Password</td>
                    <td>User Role</td>
                    <td>Created Successfully?</td>
                    </tr>
                    </thead>
                    <tbody class='user_import_scroll'>";
 
 
        $group = Group::getByName("Overall User Group");
        $dentalRegion = "Regional";
        $dentalFacility = "Facility";
 
        $groupRegion = Group::getByName($Region);
 
        $groupFacility = Group::getByName($Facility);
 
        $sql = "SELECT id,email, concat(id, forename) as pwd, role,regionAccess, facilityAccess, regions, facilities, if(regions like 'HQ % ','HQ','') as HQ FROM UsersDatabase.Users_combined";
 
        $result = $this->connImport->query($sql);
 
        $RegionEntity = [];
 
        foreach ($RegionEntities as $entity) {
            $RegionEntity[$entity->getRegionName()] = $entity->getId();
        }
 
        $FacilitiesEntity = [];
 
        foreach ($FacilityEntities as $facilityEntity) {
            $FacilitiesEntity[$facilityEntity->getFacilityName()] = $facilityEntity->getId();
        }
 
 
        while ($data = mysqli_fetch_assoc($result)) {
            $reg = false;
            $htmlReturn .= "<tr>";
            $userinfo = ['uName' => $data['id'], 'uEmail' => $data['email'], 'uPassword' => $data['pwd']];
 
            $ui = \Core::make('Concrete\Core\User\UserInfoRepository')->getByName($data['id']);
            if (!$ui) {
 
                $reg = $userRegistrationService->create($userinfo);
 
                $roles = explode(",", $data['role']);
                $role_string = '';
 
                foreach ($roles as $role) {
                    if (strpos($role_string, $role) === false) {
                        $role_string .= $role . " , ";
                    }
                }
 
                $reg->SetAttribute('user_role', $role_string);
 
                $u = \Core::make('Concrete\Core\User\UserInfoRepository')->getByName($data['id']);
 
                $user = $u->getUserObject();
 
                if (!$user->inGroup($group)) {
                    $user->enterGroup($group);
                }
 
                if (!$user->inGroup($groupRegion)) {
 
                    if ($data['regionAccess'] == $Region) {
                        $user->enterGroup($groupRegion);
                    }
                }
                if (!$user->inGroup($groupFacility)) {
                    if ($data['facilityAccess'] == $Facility) {
                        $user->enterGroup($groupFacility);
                    }
                }
 
                $region_entries = [];
 
                $regions = explode("#", $data['regions']);
 
                foreach ($regions as $region_name) {
                    foreach ($RegionEntity as $key => $value) {
 
                        if ($region_name == $key) {
 
                            $region_entries[] = $this->entityManager->getRepository('Concrete\Core\Entity\Express\Entry')->findOneById($value);
                        }
                    }
                }
 
 
                $u->setAttribute('region', $region_entries);
 
 
                $facility_entries = [];
 
                $facilities = explode("#", $data['facilities']);
 
                foreach ($facilities as $facility_name) {
                    foreach ($FacilitiesEntity as $key => $value) {
 
                        if ($facility_name == $key) {
                            $facility_entries[] = $this->entityManager->getRepository('Concrete\Core\Entity\Express\Entry')->findOneById($value);
                        }
                    }
                }
 
                $u->setAttribute('facility', $facility_entries);
 
                if ($data['HQ'] == "HQ") {
 
                    $u->setAttribute('hq', 'Yes');
                }
 
                $u->setAttribute('agreement_check ', 0);
 
            }
 
            $htmlReturn .= "<td>" . $userinfo['uName'] . "</td>";
            $htmlReturn .= "<td>" . $userinfo['uEmail'] . "</td>";
            $htmlReturn .= "<td>" . $userinfo['uPassword'] . "</td>";
            $htmlReturn .= "<td>" . rtrim($data['role'], " , ") . "</td>";
 
            if ($reg) {
                $htmlReturn .= "<td><span class='fa fa - check'></span></td>";
            } else {
                $htmlReturn .= "<td><span class='fa fa - close'></span> User already exists </td>";
            }
            $htmlReturn .= "</tr>";
 
        }
        fclose($fh);
 
    } else {
        return "No filename found, refresh page and try again";
    }
    $htmlReturn .= "</tbody>";
    $htmlReturn .= "</table>";
    $htmlReturn .= "<div class='alert alert - info'><strong>Notice</strong> " . $dataLoaded . "</div>";
 
    $l->info("");
    $l->info("End User Import");
    return $this->app->make(ResponseFactoryInterface::class)->json($htmlReturn);
}

mnakalay replied on Dec 21, 2017 at 12:55 pm Permalink Reply

Here are some micro-optimization you can try. They won't half the time it takes but they'll help.

You have this

$reg = $userRegistrationService->create($userinfo);
 // some more code
// ....
$u = \Core::make('Concrete\Core\User\UserInfoRepository')->getByName($data['id']);

You don't need to do that last line. When you create that user, you get the UserInfo object back so $reg is already what you need, you don't need to do that $u stuff.

In the while loop you have

$ui = \Core::make('Concrete\Core\User\UserInfoRepository')->getByName($data['id']);

Then later, still in the while but also inside 2 for loops inside the while loop you have

$this->entityManager->getRepository('Concrete\Core\Entity\Express\Entry')->findOneById($value);

You should probably do

$entity = $this->entityManager->getRepository('Concrete\Core\Entity\Express\Entry');

Outside before the while loop and then just use that variable instead of creating a new instance of the class every time. Same for the UserInfoRepository class.

What I understand from your code is you first empty the core Users table. Then you load the data from your file into UserDatabase.Users table. Then you do some processing.

Then you select data from UsersDatabase.Users_combined. I am not sure how that table came into existence in the first place.

Using that data you start checking if those users exist and if not create them using the data from UserDatabase.Users_combined

So the things that are not clear to me:
1- Am I right in assuming you are using totally separate databases for your processing?
2- Why is it first UserDatabase and then UsersDatabase with an s?
3- After loading your file's data into UserDatabase.Users, what do you do with that data? Is it needed?
4- Since you truncate the core Users table, why do you need to check for each user if it exists before adding it? It won't be there since you emptied the table. Unless your data has redundancies?
5- If I am right and you are using separate databases, do you have to? That's bound to slow things down a lot.
6- Adding users to the Core table the way you do it is slow. Isn't there a way you can load your data directly in the core Users table instead of UserDatabase.Users and then deal with attributes and other things?

Again, my assessment might be wrong but it might help.

dkw15 replied on Jan 3, 2018 at 8:58 am Permalink Reply

Hi mnakalay

1. I am using a separate database, this is so I don't interfere with the core database tables.
2. Probably a typo from copying across the code (this isn't the case in the code I have).
3. After the CSV is loaded in to the database it doesn't necessarily need to be stored (although for continuity it might be a good idea to keep a log of imported users with datetime)
4. So I am truncating the external table (ready for a new CSV load) - Not the Core Users table in the CMS database. But I still need to check the CMS Users table for the user to see if they exist before adding them in.
5. In an ideal world no, but to keep things separated then it's necessary at the moment.
6. Can you explain what you mean by Adding users the way I do is slow? Is there a better faster way?

I'll see if I can implement your optimisations and thanks for the suggestions and assistance! It's much appreciated.

JohntheFish replied on Jan 3, 2018 at 11:29 am Permalink Reply

For a few dollars there are user csv importers in the marketplace.
There is also a free importer by @hissy on github

dkw15 replied on Jan 4, 2018 at 7:19 am Permalink Reply

True, but the project and user import tool are fairly bespoke so any free/paid addon I would use would need modifying anyway. Thanks though

mnakalay replied on Jan 4, 2018 at 5:09 pm Permalink Reply

concerning (6) it's just that I don't understand why you need to import your data into a separate table in a separate database and from there run a series of queries to add one user at a time.

Wouldn't it be more efficient to process the data imported in a way that allows you to get rid of that intermediary table?

also, as far as the creation of users is concerned, it might be more efficient to add them all to the table using an SQL query instead of the C5 API and once all the users are in, then you can use the C5 API for attributes and groups and stuff like that.

I think it would be faster. Harder too probably.

Forums

Developing (v7+)

Optimising code 8.2.1 User Import

Code

Post Reply

Delete Post

Mark Post as Spam

Destroy Spammer

Sign In?