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');
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);
}
You have this
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
Then later, still in the while but also inside 2 for loops inside the while loop you have
You should probably do
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.