Uploaded image for project: 'Blesta Core'
  1. Blesta Core
  2. CORE-2581

Unpaid pending services may be activated under certain circumstances

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 4.2.0
    • Fix Version/s: 4.2.2, 4.3.0-b1
    • Component/s: None
    • Labels:
      None

      Description

      The "Provision Paid Pending Services" automation tasks runs every 5 minutes by default to provision paid pending services. A paid pending service is considered paid if any linked invoices are paid. If there are no linked invoices, it's considered paid.

      During order, the service is created first, then the invoice. If there is a delay between service and invoice creation, and the automation task runs between this delay, a service may be provisioned though it has not been paid since the invoice was not yet created.

      I'm not sure what the right solution is here. Ideally, I suppose, it'd be great if there were some kind of "lock" put on a service that is released once the invoice has been generated, and then the automation task ignores services that are locked. Or, a delay in provisioning paid pending services to allow sufficient time for an invoice to be generated, though that is arbitrary.

        Activity

        admin Paul Phillips created issue -
        admin Paul Phillips made changes -
        Field Original Value New Value
        Rank Ranked higher
        admin Paul Phillips made changes -
        Security Private [ 10000 ]
        tyson Tyson Phillips (Inactive) made changes -
        Sprint 4.3.0 Sprint 3 [ 53 ]
        tyson Tyson Phillips (Inactive) made changes -
        Rank Ranked higher
        tyson Tyson Phillips (Inactive) made changes -
        Summary Unpaid pending services may be activate under certain circumstances Unpaid pending services may be activated under certain circumstances
        tyson Tyson Phillips (Inactive) made changes -
        Fix Version/s 4.2.2 [ 11021 ]
        Fix Version/s 4.3.0-b1 [ 11019 ]
        Fix Version/s Short Term [ 10800 ]
        tyson Tyson Phillips (Inactive) made changes -
        Assignee Tyson Phillips [ tyson ]
        Hide
        tyson Tyson Phillips (Inactive) added a comment -

        A good solution to this would be to have the entire process of the service + invoice creation be in a database transaction. The problem, however, is that there already exists transactions that occur in parts of the process when adding/updating services and when adding invoices, and since you can't have nested database transactions, we would have to refactor the process. I already had plans to refactor models in CORE-2430, which is where this could be resolved.

        How often does this occur? An order has to be placed at exactly the right time such that between when the service is created and the invoice is created the cron happens to run the provision paid pending services task to fetch those services. While possible, that race condition seems like a shot in the dark, and I'm sure there would be other race conditions with similar undesired effects.

        We could delay the provision paid pending services task as you suggested by some arbitrary amount of time, and that may reduce the frequency tremendously, although it will still be possible to encounter the issue. That would probably be alright in the interim.

        Show
        tyson Tyson Phillips (Inactive) added a comment - A good solution to this would be to have the entire process of the service + invoice creation be in a database transaction. The problem, however, is that there already exists transactions that occur in parts of the process when adding/updating services and when adding invoices, and since you can't have nested database transactions, we would have to refactor the process. I already had plans to refactor models in CORE-2430 , which is where this could be resolved. How often does this occur? An order has to be placed at exactly the right time such that between when the service is created and the invoice is created the cron happens to run the provision paid pending services task to fetch those services. While possible, that race condition seems like a shot in the dark, and I'm sure there would be other race conditions with similar undesired effects. We could delay the provision paid pending services task as you suggested by some arbitrary amount of time, and that may reduce the frequency tremendously, although it will still be possible to encounter the issue. That would probably be alright in the interim.
        tyson Tyson Phillips (Inactive) made changes -
        Remaining Estimate 0 minutes [ 0 ]
        Time Spent 38 minutes [ 2280 ]
        Worklog Id 10708 [ 10708 ]
        Automated transition triggered when Tyson Phillips (Inactive) created a branch in Stash -
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        tyson Tyson Phillips (Inactive) added a comment -

        We can delay newly-created services from being provisioned within a certain time span. We assume 10 seconds is sufficient enough for the vast majority of cases. While the race condition can still be encountered (e.g. plugins that perform slow actions using the Services.add event), this work-around is good enough until much of the business logic surrounding service and invoice creation can be refactored, at which time this race condition would no longer be possible.

        Open /app/controllers/cron.php and find:

                        if ($provision_services) {
                            // Fetch all paid pending services for this client group
                            $services = $this->Services->getAllPaidPending($client_group->id);
        
                            foreach ($services as $service) {
                                // Add service module fields
                                $module_fields = [];
                                foreach ($service->fields as $field) {
                                    $module_fields[$field->key] = $field->value;
                                }
        

        replace with:

                        if ($provision_services) {
                            // Fetch all paid pending services for this client group
                            $services = $this->Services->getAllPaidPending($client_group->id);
        
                            // Set a 10 second cutoff for provisioning services from the current time
                            $service_cutoff_time = $this->Date->toTime($this->Date->format('c')) - 10;
        
                            foreach ($services as $service) {
                                // Skip services that were added too recently. They may not have an invoice yet.
                                // This temporarily works around a race condition whereby this task provisions the service
                                // before it could be paid because it has no invoice yet
                                if ($this->Date->toTime($service->date_added . 'Z') >= $service_cutoff_time) {
                                    continue;
                                }
        
                                // Add service module fields
                                $module_fields = [];
                                foreach ($service->fields as $field) {
                                    $module_fields[$field->key] = $field->value;
                                }
        
        Show
        tyson Tyson Phillips (Inactive) added a comment - We can delay newly-created services from being provisioned within a certain time span. We assume 10 seconds is sufficient enough for the vast majority of cases. While the race condition can still be encountered (e.g. plugins that perform slow actions using the Services.add event), this work-around is good enough until much of the business logic surrounding service and invoice creation can be refactored, at which time this race condition would no longer be possible. Open /app/controllers/cron.php and find: if ($provision_services) { // Fetch all paid pending services for this client group $services = $ this ->Services->getAllPaidPending($client_group->id); foreach ($services as $service) { // Add service module fields $module_fields = []; foreach ($service->fields as $field) { $module_fields[$field->key] = $field->value; } replace with: if ($provision_services) { // Fetch all paid pending services for this client group $services = $ this ->Services->getAllPaidPending($client_group->id); // Set a 10 second cutoff for provisioning services from the current time $service_cutoff_time = $ this ->Date->toTime($ this ->Date->format('c')) - 10; foreach ($services as $service) { // Skip services that were added too recently. They may not have an invoice yet. // This temporarily works around a race condition whereby this task provisions the service // before it could be paid because it has no invoice yet if ($ this ->Date->toTime($service->date_added . 'Z') >= $service_cutoff_time) { continue ; } // Add service module fields $module_fields = []; foreach ($service->fields as $field) { $module_fields[$field->key] = $field->value; }
        Automated transition triggered when Tyson Phillips (Inactive) created pull request #395 in Stash -
        Status In Progress [ 3 ] In Review [ 5 ]
        Resolution Fixed [ 1 ]
        tyson Tyson Phillips (Inactive) made changes -
        Time Spent 38 minutes [ 2280 ] 1 hour, 42 minutes [ 6120 ]
        Worklog Id 10716 [ 10716 ]
        Automated transition triggered when Tyson Phillips (Inactive) merged pull request #395 in Stash -
        Status In Review [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            tyson Tyson Phillips (Inactive)
            Reporter:
            admin Paul Phillips
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Fix Release Date:
              22/Feb/18

              Time Tracking

              Estimated:
              Original Estimate - Not Specified
              Not Specified
              Remaining:
              Remaining Estimate - 0 minutes
              0m
              Logged:
              Time Spent - 1 hour, 42 minutes
              1h 42m

                Agile