Laravel as client for Strava 3rd party API

Question

I’m developing a Laravel app which acts as a client to consume a 3rd party Strava API. My app also functions as an API for a frontend SPA. The user (which is just me for the time being) will already be ‘logged in’ via SPA and Laravel’s Sanctum (cookie-based session auth) so when the user is prompted to log in to their Strava account, this is merely for the purpose of fetching the activities (run, swim or whatever). Since the activities returned from Strava are immediately saved to the database, a full OAuth flow is not required as I have no need to store the access token / deal with refresh tokens.

The Strava activities are fetched and saved by two services:

Strava: gets the auth code, exchanges it for the athlete (user) and gets the activities. This service is bound via a singleton method to a custom provider and called from the controller via a facade

StravaActivity: Maps the returned activities and saves them to the database

What are people’s suggestions for improving this code? Some things I’m unsure of:

• The store action method being called by the handleCallback method in the controller... but from a UX perspective, I want the user to be able to fetch and save the activities via one click from the SPA (albeit having to log in to Strava en route)

• Whether the mapping function should be a class in its own right... though feel this might be over-engineering a small project

• Exceptions: Wanted to throw and catch in the services and simply relay any error to the controller to keep the controller slim

Here’s the relevant code:

Routes:

use App\Http\Controllers\StravaController;
use Illuminate\Support\Facades\Route;

Route::get(
    '/strava/auth',
    [StravaController::class, 'redirectToStrava']
)->name('web.strava.redirectToStrava');

Route::get(
    '/strava/auth/handleCallback',
    [StravaController::class, 'handleCallback']
)->name('web.strava.handleCallback');

StravaController:

namespace App\Http\Controllers;

use App\Facades\Strava;
use App\Services\StravaActivity;
use Illuminate\Http\Request;
use Illuminate\Http\JsonResponse;
use Illuminate\Http\RedirectResponse;

class StravaController extends Controller
{
    private $stravaActivity;

    public function __construct(StravaActivity $stravaActivity)
    {
        $this->stravaActivity = $stravaActivity;
    }

    public function redirectToStrava(): RedirectResponse
    {
        return Strava::getAuthCode();
    }

    public function handleCallback(Request $request): JsonResponse | RedirectResponse
    {
        if (!$request->has('code')) {
            // todo: redirect back to relevant SPA page
            return redirect('/')->withErrors('Auth failed');
        }

        return $this->store($request->code);
    }

    public function store(string $authCode): JsonResponse
    {
        $latestActivities = Strava::getLatestActivities($authCode);
        $response = $this->stravaActivity->saveActivities($latestActivities);

        if ($response['success'] === false) {
            return response()->json(['error' => $response['message']], 422);
        }

        return response()->json($response['recentActivities'], 201);
    }
} 

Strava (service):

namespace App\Services;

use Exception;
use Illuminate\Http\Client\Response;
use Illuminate\Http\RedirectResponse;
use Illuminate\Support\Facades\File;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Log;

class Strava
{
    private $stravaOauthUri = 'https://www.strava.com/oauth';
    private $stravaUri = 'https://www.strava.com/api/v3';

    private $stravaClientId;
    private $stravaSecretId;
    private $stravaRedirectUri;

    public function __construct(
        string $stravaClientId,
        string $stravaSecretId,
        string $stravaRedirectUri,
    ) {
        $this->stravaClientId = $stravaClientId;
        $this->stravaSecretId = $stravaSecretId;
        $this->stravaRedirectUri = $stravaRedirectUri;
    }

    public function getAuthCode(
        string $scope = 'read_all,profile:read_all,activity:read_all'
    ): RedirectResponse {
        $query = http_build_query([
            'client_id' => $this->stravaClientId,
            'response_type' => 'code',
            'redirect_uri' => $this->stravaRedirectUri,
            'scope' => $scope,
            'state' => 'strava',
        ]);

        return redirect("{$this->stravaOauthUri}/authorize?{$query}");
    }

    public function getLatestActivities(string $authCode): array
    {
        try {
            $tokenData = $this->getAthleteWithTokens($authCode);
            return $this->getActivities($tokenData['access_token']);
        } catch (Exception $error) {
            return [
                'success' => false,
                'message' => $error->getMessage()
            ];
        }
    }

    private function getAthleteWithTokens(string $authCode): array
    {
        $url = "{$this->stravaOauthUri}/token";
        $config = [
            'client_id' => $this->stravaClientId,
            'client_secret' => $this->stravaSecretId,
            'code' => $authCode,
            'grant_type' => 'authorization_code'
        ];

        $response = Http::post($url, $config);
        if ($response->ok()) {
            return json_decode($response->getBody(), true);
        }

        $this->throwError($response);

        return [];
    }

    private function getActivities(string $token): array
    {
        $url = "{$this->stravaUri}/athlete/activities";

        $response = Http::withToken($token)->get($url);
        if ($response->ok()) {
            return json_decode($response->getBody()->getContents(), true);
        }

        $this->throwError($response);

        return [];
    }

    private function throwError(Response $response): void
    {
        $statusCode = $response->getStatusCode();
        $jsonResponse = $response->json();
        $errorMessage = $jsonResponse['message'] ?? 'Strava Service not available';

        Log::error(
            'Error getting Strava activities',
            [
                'status' => $statusCode,
                'message' => $errorMessage
            ]
        );

        throw new Exception("Strava API error: {$statusCode}: {$errorMessage}");
    }
}

StravaActivity (service):

namespace App\Services;

use App\Models\StravaActivity as StravaActivityModel;
use Illuminate\Database\QueryException;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Schema;

class StravaActivity
{
    public function saveActivities(array $activities): array
    {
        $mappedActivities = $this->mapActivities($activities);

        try {
            $recentActivities = $this->storeActivities($mappedActivities);
        } catch (QueryException $error) {
            $returnErrorMessage = $error->getMessage();

            Log::error(
                'Error saving Strava activities',
                [
                    'message' => $error->getMessage()
                ]
            );

            $returnErrorMessage = 'SQL error: Strava activity(ies) not persisted';

            return [
                'success' => false,
                'message' => $returnErrorMessage
            ];
        }

        return [
            'success' => true,
            'recentActivities' => $recentActivities
        ];
    }

    private function mapActivities(array $activities): array
    {
        $dataStatisticsToBeMapped = Schema::getColumnListing('strava_activities');

        return array_map(function ($activity) use ($dataStatisticsToBeMapped) {
            $activity['strava_id'] = $activity['id'];
            $activity['map_polyline'] = $activity['map']['summary_polyline'];
            unset($activity['id']);
            unset($activity['map']);

            return array_filter($activity, function ($statItem) use ($dataStatisticsToBeMapped) {
                return (in_array($statItem, $dataStatisticsToBeMapped));
            }, ARRAY_FILTER_USE_KEY);
        }, $activities);
    }

    private function storeActivities(array $mappedActivities): array
    {
        $recentActivities = [];

        foreach ($mappedActivities as $mappedActivity) {
            $newActivity = StravaActivityModel::firstOrCreate(
                ['strava_id' => $mappedActivity['strava_id']],
                $mappedActivity
            );

            if ($newActivity->wasRecentlyCreated) {
                array_push($recentActivities, $mappedActivity);
            }
        }

        return $recentActivities;
    }
}

Answer 1

Some minor points from me, just after a cursory look at the code. My PHP is rusty, but I know unset can destroy more than one variable. Useful to know.

I would try to simplify function saveActivities, and make the control flow more straightforward. This should be equivalent (but untested):

public function saveActivities(array $activities): array
{
    $mappedActivities = $this->mapActivities($activities);

    try {
        $recentActivities = $this->storeActivities($mappedActivities);
        
        return [
            'success' => true,
            'recentActivities' => $recentActivities
        ];
    } catch (QueryException $error) {
        Log::error(
            'Error saving Strava activities',
            [
                'message' => $error->getMessage()
            ]
        );

        return [
            'success' => false,
            'message' => 'SQL error: Strava activity(ies) not persisted';
        ];
    }
}

Changes:

• Don't reassign $returnErrorMessage, just return $error->getMessage(); first, and then the static message. By removing this variable, we shorten the code by 2 lines.
• The return true statement can go up under $recentActivities = $this->storeActivities($mappedActivities);. I believe the two possible outcomes (success/failure) are now better separated, and the risk of a logic error is decreased.

Your exception handling routine does not cover the whole function: $mappedActivities = $this->mapActivities($activities); could still fail. Maybe you could simply move it inside your try block. This further reduces the number of lines of code.