4
\$\begingroup\$

I am working on a small internal app that works along an external API for provisioning purpose.

The code essentially is comprised of a series of forms where the user inputs data, this data is then sent to the API to register new customers.

This is somewhat quite a linear process I will try to explain:

  1. Contact Creation: Basic customer info (email, address...).
  2. Customer Creation: More advanced info, for a customer to be created there must be a contactId belonging to this customer.
  3. Subscriber Creation: Info linking customer to the service acquired. The previous step has to be completed and a customerId must exist.
  4. Service Creation: Lasts bits of advanced info about the service. Once again, a suscriberId needs to exist in order to link the service.

I've managed a more or less quick process with a few tweaks here and there, but the first step (Contact) has a method that I can't seem to improve, which in turn causes this process to take up to a full minute!

As the whole process described earlier, the creation of each one of these is very linear too.

The API documentation states that the results of any GET should be paginated to a max of 10 entries, but following this further increases the time over the minute. Manual experiments showed that the best ratio is about 500 entries per page or in some cases, even the whole number of entries proved to be the fastest way rather than going 10 by 10.

Since the contact email can't be duplicated, one of the first things to do is check for the email provided in the form and compare it to all of the already existing emails stored.

In order to provide the $page and $entries to the API call, I must first fetch the total number of contacts. This number appears when calling the API to get the contacts. So the first method I use is:

function fetchTotalContacts($uri, $auth){
    $ch = curl_init();

    $options = array(
        CURLOPT_RETURNTRANSFER => true,
        CURLOPT_SSL_VERIFYHOST => false,
        CURLOPT_SSL_VERIFYPEER => false,
        CURLOPT_URL => $uri.'?page=1&rows=1',
        CURLOPT_HTTPHEADER => array('Content-Type: application/json', 'Authorization: $auth')
    );

    curl_setopt_array($ch, $options);

    $response = curl_exec($ch);
    $response = json_decode($response, true);
    $totalContacts = $response['total_count'];

    $http_code = curl_getinfo($ch, CURLINFO_HTTP_CODE);

    if ($http_code != 200) {
        echo "Error en fetchTotalContacts() - Código: $http_code | ";
    }
    
    curl_close($ch);

    return $totalContacts;
}

Now having $totalContacts, I can proceed and search if the email has been already registered, and this is the step I suspect to be responsible for the high execution time. This method searches the contacts and their emails, if it finds no coincidence, proceeds to create the contact with the data provided.

function checkDuplicatedEmail($uri, $totalContacts, $contactEmailArray, $auth, $contactEmail, $dataContact){
    $ch = curl_init();

    $options = array(
        CURLOPT_RETURNTRANSFER => true,
        CURLOPT_SSL_VERIFYHOST => false,
        CURLOPT_SSL_VERIFYPEER => false,
        CURLOPT_URL => $uri.'?page=1&rows='.$totalContacts,
        CURLOPT_HTTPHEADER => array('Content-Type: application/json', 'Authorization: $auth')
    );
    curl_setopt_array($ch, $options);

    $customers = curl_exec($ch);
    $customers = json_decode($customers, true);

    $http_code = curl_getinfo($ch, CURLINFO_HTTP_CODE);
    
    if ($http_code != 200) {
        echo "Error en checkDuplicatedEmail() - Código: $http_code | ";
    }


    curl_close($ch);

    /*
    foreach ($customers['_embedded']['ngcp:customercontacts'] as $customer) {
    
        $email = $customer['email'];

        array_push($contactEmailArray, $email);
    }

    if (in_array($contactEmail, $contactEmailArray)) {
        echo('El email utilizado ya ha sido registrado en la base de datos');
        die();
    }else{
        $contactCreated = createContact($uri, $dataContact);
        return $contactCreated;
    }
    */

    $repeated = 0;
    for ($i=0; $i < $totalContacts ; $i++) {
        if ($contactEmail == $customer["_embedded"]["ngcp:customercontacts"][$i]["email"]) {
            $repeated += 1;
        }
    }

    if ($repeated > 0) {
        die(echo('El email utilizado ya ha sido registrado en la base de datos'));
    }else{
        $contactCreated = createContact($uri, $dataContact, $auth);
        return $contactCreated;
    }
}

As you can see, these are the quickiest options I've found, both making the whole process take 40s, which is too much still.

The response for createContact($uri, $dataContact, $auth); is a success code (400, 201..), so when I want to go to the next step I do need to, again, search all the contacts to find the one I just created and get the id. Fortunately, here I can simply skip to the last 20 contacts (not the last one directly so it can be used simultaneously without issues) and search there, this makes it real quick, but for the email there is no such skip, all the entries must be analyzed.

I don't know how to drop the time here, the rest of the code consists of fetching the contactId and creating the customer so not much to do there as it is now.

If any of you deem necessary to see the rest of the page I wil update the post.

As a final reminder, I have manually tried with different configs of pages and entries and for this page, the fastest was 1 page - All entries. I've also tried taking the for/each loop outside the method, to no avail.

\$\endgroup\$
2
\$\begingroup\$

The for loop is not going to be the bottleneck in your loop, but what seems obvious to me is that you don't benefit from counting higher than $repeated = 1. This means that you don't need a counter variable, you actually need a loop breaking event -- in this case die(). For your information, die() will print the text in its first parameter, so using echo is redundant.

for ($i=0; $i < $totalContacts; ++$i) {
    if ($contactEmail == $customer["_embedded"]["ngcp:customercontacts"][$i]["email"]) {
        die('El email utilizado ya ha sido registrado en la base de datos');
    }
}
return createContact($uri, $dataContact, $auth);

Or another way, which I assume will be slower (because array_column() will be collecting all emails) is this functional design:

if (in_array($contactEmail, array_column($customer["_embedded"]["ngcp:customercontacts"], "email"))) {
    die('El email utilizado ya ha sido registrado en la base de datos');
}
return createContact($uri, $dataContact, $auth);
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you! I was heading back home and didn't have time to change the title! I changed it and it did improve the exec. time, I still feel like it's too much time, (little over 30s) but I don't really see more way for improvement, could this be because of the external API? I haven't used that many, but of the few ones I've worked with this one sure qualifies as the worst for other various reasons. It's 3rd party software so sadly there is not much I can do about it too. It is an internal app meant for very little people to use, but even so I don't feel quite happy about the job done. \$\endgroup\$ – Berny Aug 20 at 15:36
  • 1
    \$\begingroup\$ @Berny Whenever I suspect an external API is at fault, I replace all calls to the externals with debug statements. If that reduces your execution time from 30s to 1s, you know enough. \$\endgroup\$ – Mast Aug 21 at 17:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.