From 9e2c1a457af34b93c110e236dbba204c4bccf97b Mon Sep 17 00:00:00 2001 From: irakli-bakhtadze Date: Fri, 17 Apr 2026 23:50:16 +0400 Subject: [PATCH 1/8] Update CODEOWNERS --- .github/CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index dede3952e..ef640d412 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -* @BrBrane +* @DPrice-FCBH From b12e5081d80109d2d536fa1e569f6ce764ff2fb4 Mon Sep 17 00:00:00 2001 From: Alexey Satsunkevich <89253689+alexeysatsunkevich@users.noreply.github.com> Date: Thu, 14 May 2026 18:56:30 +0300 Subject: [PATCH 2/8] Revert "Update CODEOWNERS for master branch" (#1082) --- .github/CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index ef640d412..dede3952e 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -* @DPrice-FCBH +* @BrBrane From a8a421b78437231ef588e89eba2575cc03df6fac Mon Sep 17 00:00:00 2001 From: Victor Gonzalez Date: Fri, 10 Apr 2026 13:08:28 -0500 Subject: [PATCH 3/8] Task-89223 API Key Whitelist for User Data Endpoints. --- .env.example | 1 + app/Http/Kernel.php | 2 + app/Http/Middleware/UserDataAccess.php | 55 +++++++++++++++++++++ app/Http/Middleware/UserDataAuditLog.php | 49 +++++++++++++++++++ config/auth.php | 1 + config/logging.php | 6 +++ routes/api.php | 61 +++++++++++------------- tests/Integration/UserRoutesTest.php | 60 +++++++++++++++++++++++ 8 files changed, 201 insertions(+), 34 deletions(-) create mode 100644 app/Http/Middleware/UserDataAccess.php create mode 100644 app/Http/Middleware/UserDataAuditLog.php diff --git a/.env.example b/.env.example index 3a496f853..e777869b4 100644 --- a/.env.example +++ b/.env.example @@ -159,6 +159,7 @@ BIBLE_SYNC_FILE_PATH= FORBIDDEN_ARCLIGHT_ISO= BIBLEIS_KEYS= GIDEONS_KEYS= +USER_DATA_ACCESS_KEYS= BIBLEIS_DEPRECATE_FROM_VERSION= GIDEONS_DEPRECATE_FROM_VERSION= diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index c1b81c3b5..507791311 100755 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -77,6 +77,8 @@ class Kernel extends HttpKernel 'verified' => \Illuminate\Auth\Middleware\EnsureEmailIsVerified::class, 'APIToken' => \App\Http\Middleware\APIToken::class, 'AccessControl' => \App\Http\Middleware\AccessControl::class, + 'UserDataAccess' => \App\Http\Middleware\UserDataAccess::class, + 'UserDataAuditLog' => \App\Http\Middleware\UserDataAuditLog::class, 'localize' => \Mcamara\LaravelLocalization\Middleware\LaravelLocalizationRoutes::class, 'localizationRedirect' => \Mcamara\LaravelLocalization\Middleware\LaravelLocalizationRedirectFilter::class, 'localeSessionRedirect' => \Mcamara\LaravelLocalization\Middleware\LocaleSessionRedirect::class, diff --git a/app/Http/Middleware/UserDataAccess.php b/app/Http/Middleware/UserDataAccess.php new file mode 100644 index 000000000..9598e18f4 --- /dev/null +++ b/app/Http/Middleware/UserDataAccess.php @@ -0,0 +1,55 @@ +isKeyWhitelisted($api_key)) { + return response()->json([ + 'error' => [ + 'message' => 'This API key is not authorized to access user data.', + 'status_code' => Response::HTTP_FORBIDDEN, + ] + ], Response::HTTP_FORBIDDEN); + } + + return $next($request); + } + + /** + * Check if the given API key is in the user data access whitelist. + */ + private function isKeyWhitelisted(?string $api_key): bool + { + if (empty($api_key)) { + return false; + } + + $keys = config('auth.compat_users.api_keys.user_data_access'); + + if ($keys === null || trim((string) $keys) === '') { + return false; + } + + + $allowed_keys = array_map('trim', explode(',', $keys)); + + return in_array($api_key, $allowed_keys, true); + } +} diff --git a/app/Http/Middleware/UserDataAuditLog.php b/app/Http/Middleware/UserDataAuditLog.php new file mode 100644 index 000000000..3adbec0a4 --- /dev/null +++ b/app/Http/Middleware/UserDataAuditLog.php @@ -0,0 +1,49 @@ +route('user_id') + ?? $request->route('playlist_id') + ?? $request->route('plan_id') + ?? $request->input('user_id'); + + Log::channel('user_data_access')->info('user_data_access', [ + 'api_key' => $masked_key, + 'endpoint' => $request->path(), + 'method' => $request->method(), + 'target_id' => $user_id, + 'status_code' => $response->getStatusCode(), + 'ip' => $request->ip(), + ]); + } +} diff --git a/config/auth.php b/config/auth.php index 10b1a3102..612697edd 100755 --- a/config/auth.php +++ b/config/auth.php @@ -82,6 +82,7 @@ 'api_keys' => [ 'bibleis' => env('BIBLEIS_KEYS'), 'gideons' => env('GIDEONS_KEYS'), + 'user_data_access' => env('USER_DATA_ACCESS_KEYS', ''), ], ], diff --git a/config/logging.php b/config/logging.php index 9401ddb5c..62707a2fb 100755 --- a/config/logging.php +++ b/config/logging.php @@ -65,6 +65,12 @@ 'path' => storage_path('logs/api/api.log') ], + 'user_data_access' => [ + 'driver' => 'daily', + 'path' => storage_path('logs/user_data_access_access.log'), + 'days' => 90, + ], + 'slack' => [ 'driver' => 'slack', 'url' => env('LOG_SLACK_WEBHOOK_URL'), diff --git a/routes/api.php b/routes/api.php index 9f6816de7..690dccc66 100755 --- a/routes/api.php +++ b/routes/api.php @@ -281,24 +281,21 @@ ->get('search/library', 'Bible\TextController@searchLibrary'); // VERSION 4 | Users (bible.is private) -Route::name('v4_internal_user.index')->get( - 'users', - 'User\UsersController@index' -); +Route::name('v4_internal_user.index') + ->middleware(['UserDataAccess', 'UserDataAuditLog']) + ->get('users', 'User\UsersController@index'); Route::name('v4_internal_user.store')->post( 'users', 'User\UsersController@store' ); -Route::name('v4_internal_user.show')->get( - 'users/{user_id}', - 'User\UsersController@show' -); -Route::name('v4_internal_user.update')->put( - 'users/{user_id}', - 'User\UsersController@update' -); +Route::name('v4_internal_user.show') + ->middleware(['UserDataAccess', 'UserDataAuditLog']) + ->get('users/{user_id}', 'User\UsersController@show'); +Route::name('v4_internal_user.update') + ->middleware(['UserDataAccess', 'UserDataAuditLog']) + ->put('users/{user_id}', 'User\UsersController@update'); Route::name('v4_internal_user.destroy') - ->middleware('APIToken:check') + ->middleware(['APIToken:check', 'UserDataAccess', 'UserDataAuditLog']) ->delete('users', 'User\UsersController@destroy'); Route::name('v4_internal_user.login')->post( '/login', @@ -331,16 +328,16 @@ // VERSION 4 | Playlists (bible.is private) Route::name('v4_internal_playlists.index') - ->middleware('APIToken') + ->middleware(['APIToken', 'UserDataAccess', 'UserDataAuditLog']) ->get('playlists', 'Playlist\PlaylistsController@index'); Route::name('v4_internal_playlists.store') ->middleware('APIToken:check') ->post('playlists', 'Playlist\PlaylistsController@store'); Route::name('v4_internal_playlists.show') - ->middleware('APIToken') + ->middleware(['APIToken', 'UserDataAccess', 'UserDataAuditLog']) ->get('playlists/{playlist_id}', 'Playlist\PlaylistsController@show'); Route::name('v4_internal_playlists.show_text') - ->middleware('APIToken') + ->middleware(['APIToken', 'UserDataAccess', 'UserDataAuditLog']) ->get( 'playlists/{playlist_id}/text', 'Playlist\PlaylistsController@showText' @@ -403,13 +400,13 @@ ->whereAlphaNumeric('book_id'); // VERSION 4 | Plans (bible.is private) Route::name('v4_internal_plans.index') - ->middleware('APIToken') + ->middleware(['APIToken', 'UserDataAccess', 'UserDataAuditLog']) ->get('plans', 'Plan\PlansController@index'); Route::name('v4_internal_plans.store') ->middleware('APIToken:check') ->post('plans', 'Plan\PlansController@store'); Route::name('v4_internal_plans.show') - ->middleware('APIToken') + ->middleware(['APIToken', 'UserDataAccess', 'UserDataAuditLog']) ->get('plans/{plan_id}', 'Plan\PlansController@show'); Route::name('v4_internal_plans.update') ->middleware('APIToken:check') @@ -443,22 +440,18 @@ ->delete('plans/{plan_id}/day', 'Plan\PlansController@deleteDays'); // VERSION 4 | Accounts (bible.is private) -Route::name('v4_internal_user_accounts.index')->get( - 'accounts', - 'User\AccountsController@index' -); -Route::name('v4_internal_user_accounts.store')->post( - 'accounts', - 'User\AccountsController@store' -); -Route::name('v4_internal_user_accounts.update')->put( - 'accounts', - 'User\AccountsController@update' -); -Route::name('v4_internal_user_accounts.destroy')->delete( - 'accounts', - 'User\AccountsController@destroy' -); +Route::name('v4_internal_user_accounts.index') + ->middleware(['UserDataAccess', 'UserDataAuditLog']) + ->get('accounts', 'User\AccountsController@index'); +Route::name('v4_internal_user_accounts.store') + ->middleware(['UserDataAccess', 'UserDataAuditLog']) + ->post('accounts', 'User\AccountsController@store'); +Route::name('v4_internal_user_accounts.update') + ->middleware(['UserDataAccess', 'UserDataAuditLog']) + ->put('accounts', 'User\AccountsController@update'); +Route::name('v4_internal_user_accounts.destroy') + ->middleware(['UserDataAccess', 'UserDataAuditLog']) + ->delete('accounts', 'User\AccountsController@destroy'); // VERSION 4 | Annotations with api_token (bible.is private) Route::middleware('APIToken')->group(function () { diff --git a/tests/Integration/UserRoutesTest.php b/tests/Integration/UserRoutesTest.php index 1d63308ad..a96794045 100644 --- a/tests/Integration/UserRoutesTest.php +++ b/tests/Integration/UserRoutesTest.php @@ -76,6 +76,14 @@ public function resources() $response->assertSuccessful(); } + /** + * Whitelist the test API key for user data access in the current test. + */ + private function whitelistTestKey() + { + config(['auth.compat_users.api_keys.user_data_access' => $this->key]); + } + /** * @category V4_API * @category Route Name: v4_user @@ -87,6 +95,8 @@ public function resources() */ public function users() { + $this->whitelistTestKey(); + $key = Key::with('user.projectMembers')->where('key', $this->key)->first(); $project_id = $key->user->projectMembers->whereIn('role_id', [2,4])->first()->project_id; @@ -315,4 +325,54 @@ public function bookmarks() $response = $this->withHeaders($this->params)->delete($path); $response->assertSuccessful(); } + + /** + * @group V4 + * @group travis + * @test + */ + public function nonWhitelistedKeyGetsForbiddenOnUserEndpoints() + { + // Ensure the test key is NOT whitelisted + config(['auth.compat_users.api_keys.user_data_access' => 'some-other-key']); + + $path = route('v4_internal_user.index', array_merge($this->params, ['project_id' => 1])); + $response = $this->withHeaders($this->params)->get($path); + $response->assertStatus(403); + + $path = route('v4_internal_user.show', array_merge($this->params, ['user_id' => 1])); + $response = $this->withHeaders($this->params)->get($path); + $response->assertStatus(403); + } + + /** + * @group V4 + * @group travis + * @test + */ + public function nonWhitelistedKeyGetsForbiddenOnAccountEndpoints() + { + config(['auth.compat_users.api_keys.user_data_access' => 'some-other-key']); + + $path = route('v4_internal_user_accounts.index', array_merge($this->params, ['project_id' => 1, 'user_id' => 1])); + $response = $this->withHeaders($this->params)->get($path); + $response->assertStatus(403); + } + + /** + * @group V4 + * @group travis + * @test + */ + public function whitelistedKeyCanAccessUserEndpoints() + { + $this->whitelistTestKey(); + + $key = Key::with('user.projectMembers')->where('key', $this->key)->first(); + $project_id = $key->user->projectMembers->whereIn('role_id', [2,4])->first()->project_id; + + $path = route('v4_internal_user.index', array_merge($this->params, ['project_id' => $project_id])); + $response = $this->withHeaders($this->params)->get($path); + $response->assertSuccessful(); + } } From 00744808aebc479d8a2ed2fe4b2d7c0c532c5590 Mon Sep 17 00:00:00 2001 From: Victor Gonzalez Date: Tue, 28 Apr 2026 17:44:21 -0500 Subject: [PATCH 4/8] Ticket-90116 Public API: Display Segmentation type. (cherry picked from commit 74d497d7121fed651b762ec08d99efac5095ccf5) --- .../Bible/BibleFileSetsController.php | 65 ++- .../Controllers/Bible/BiblesController.php | 45 +- app/Http/Controllers/Plan/PlansController.php | 2 +- app/Models/Bible/BibleFileset.php | 15 +- app/Transformers/BibleTransformer.php | 13 + app/Transformers/CopyrightTransformer.php | 12 + database/factories/Bible/BibleFactory.php | 3 + ...dd_segmentation_type_to_bible_filesets.php | 35 ++ public/openapi.json | 427 ++++++++++++++++-- tests/Integration/BiblesRoutesTest.php | 184 ++++++++ 10 files changed, 759 insertions(+), 42 deletions(-) create mode 100644 database/migrations/2026_04_28_180000_add_segmentation_type_to_bible_filesets.php diff --git a/app/Http/Controllers/Bible/BibleFileSetsController.php b/app/Http/Controllers/Bible/BibleFileSetsController.php index 503874b4f..26ef8491b 100644 --- a/app/Http/Controllers/Bible/BibleFileSetsController.php +++ b/app/Http/Controllers/Bible/BibleFileSetsController.php @@ -375,6 +375,12 @@ function () use ($fileset_id, $book_id, $limit, $type) { * @OA\Schema(ref="#/components/schemas/BibleFileset/properties/id"), * description="The fileset ID to retrieve the copyright information for" * ), + * @OA\Parameter( + * name="verify_segmentation", + * in="query", + * @OA\Schema(type="boolean", default=false), + * description="When true, the response includes a segmentation_type key (section, chapter, or null)." + * ), * @OA\Response( * response=200, * description="successful operation", @@ -391,7 +397,60 @@ function () use ($fileset_id, $book_id, $limit, $type) { * @OA\Property(property="id", ref="#/components/schemas/BibleFileset/properties/id"), * @OA\Property(property="type", ref="#/components/schemas/BibleFileset/properties/set_type_code"), * @OA\Property(property="size", ref="#/components/schemas/BibleFileset/properties/set_size_code"), - * @OA\Property(property="copyright", ref="#/components/schemas/LicenseGroup/properties/copyright"), + * @OA\Property(property="segmentation_type", ref="#/components/schemas/BibleFileset/properties/segmentation_type"), + * @OA\Property(property="asset_id", type="string", description="The S3 bucket / asset identifier the fileset is stored under"), + * @OA\Property(property="copyright", ref="#/components/schemas/v4_bible_filesets.copyright_details"), + * ) + * + * @OA\Schema ( + * type="object", + * schema="v4_bible_filesets.copyright_details", + * description="Structured copyright metadata returned by CopyrightTransformer", + * title="v4_bible_filesets.copyright_details", + * @OA\Property(property="copyright_date", type="string", format="date", nullable=true, description="Copyright date as recorded in the license group"), + * @OA\Property(property="copyright", type="string", nullable=true, description="Copyright statement text"), + * @OA\Property(property="created_at", type="string", format="date-time", nullable=true), + * @OA\Property(property="updated_at", type="string", format="date-time", nullable=true), + * @OA\Property(property="open_access", type="boolean", default=false, description="Whether the fileset is open access"), + * @OA\Property(property="is_combined", type="boolean", default=false, description="Whether the copyright is combined across multiple licensors"), + * @OA\Property( + * property="organizations", + * type="array", + * description="Licensor organizations associated with this copyright (only present when at least one is attached)", + * @OA\Items(ref="#/components/schemas/v4_bible_filesets.copyright_organization") + * ), + * ) + * + * @OA\Schema ( + * type="object", + * schema="v4_bible_filesets.copyright_organization", + * description="Organization shape used in the copyright details payload", + * title="v4_bible_filesets.copyright_organization", + * @OA\Property(property="id", type="integer", nullable=true), + * @OA\Property(property="slug", type="string", nullable=true), + * @OA\Property(property="abbreviation", type="string", nullable=true), + * @OA\Property(property="description", type="string", nullable=true), + * @OA\Property(property="description_short", type="string", nullable=true, description="Sourced from the organization's tagline"), + * @OA\Property(property="phone", type="string", nullable=true), + * @OA\Property(property="email", type="string", nullable=true), + * @OA\Property(property="email_director", type="string", nullable=true), + * @OA\Property(property="logos", type="array", @OA\Items(type="object")), + * @OA\Property(property="primaryColor", type="string", nullable=true), + * @OA\Property(property="secondaryColor", type="string", nullable=true), + * @OA\Property(property="inactive", type="boolean", default=false), + * @OA\Property(property="url_site", type="string", description="Sourced from the organization's url_website"), + * @OA\Property(property="url_donate", type="string"), + * @OA\Property(property="url_twitter", type="string"), + * @OA\Property(property="url_facebook", type="string"), + * @OA\Property(property="address", type="string", nullable=true), + * @OA\Property(property="address2", type="string", nullable=true), + * @OA\Property(property="city", type="string", nullable=true), + * @OA\Property(property="state", type="string", nullable=true), + * @OA\Property(property="country", type="string", nullable=true), + * @OA\Property(property="zip", type="string", nullable=true), + * @OA\Property(property="latitude", type="number", format="float", nullable=true), + * @OA\Property(property="longitude", type="number", format="float", nullable=true), + * @OA\Property(property="translations", type="array", @OA\Items(type="object")), * ) * * @param string $id @@ -400,6 +459,7 @@ function () use ($fileset_id, $book_id, $limit, $type) { public function copyright($id) { $iso = checkParam('iso') ?? 'eng'; + $verify_segmentation = checkBoolean('verify_segmentation'); $cache_params = [$id, $iso]; $fileset = cacheRemember( @@ -423,12 +483,13 @@ function () use ($iso, $id) { 'bible_filesets.mode_id as mode_id', 'bible_filesets.set_type_code as type', 'bible_filesets.set_size_code as size', + 'bible_filesets.segmentation_type', 'bible_filesets.license_group_id' ])->first(); } ); - return $this->reply(fractal($fileset, CopyrightTransformer::class, new ArraySerializer())); + return $this->reply(fractal($fileset, new CopyrightTransformer($verify_segmentation), new ArraySerializer())); } /** diff --git a/app/Http/Controllers/Bible/BiblesController.php b/app/Http/Controllers/Bible/BiblesController.php index de0abde8b..fe32294ec 100644 --- a/app/Http/Controllers/Bible/BiblesController.php +++ b/app/Http/Controllers/Bible/BiblesController.php @@ -93,6 +93,12 @@ class BiblesController extends APIController * description="Include country_id field in response. When true, adds country_id field containing the country ID from languages.country_id.", * example="true" * ), + * @OA\Parameter( + * name="verify_segmentation", + * in="query", + * @OA\Schema(type="boolean", default=false), + * description="When true, each fileset object includes a segmentation_type key (section, chapter, or null)." + * ), * @OA\Parameter(ref="#/components/parameters/page"), * @OA\Parameter(ref="#/components/parameters/limit"), * @OA\Response( @@ -116,6 +122,7 @@ public function index() $media_exclude = checkParam('media_exclude'); $audio_timing = checkParam('audio_timing') ?? false; $show_country = checkBoolean('show_country', false); + $verify_segmentation = checkBoolean('verify_segmentation'); $size = checkParam('size'); #removed from API for initial release $size_exclude = checkParam('size_exclude'); #removed from API for initial release $limit = (int) (checkParam('limit') ?? 50); @@ -165,7 +172,8 @@ public function index() $order_cache_key, $access_group_ids->toString(), $audio_timing, - $show_country + $show_country, + $verify_segmentation ]); $bibles = cacheRemember( @@ -185,7 +193,8 @@ function () use ( $limit, $order_by, $audio_timing, - $show_country + $show_country, + $verify_segmentation ) { $bibles = Bible::filterByLanguage($language_code) ->withRequiredFilesets([ @@ -255,7 +264,7 @@ function () use ( $bibles = $bibles->paginate($limit); $bibles_return = fractal( $bibles->getCollection(), - BibleTransformer::class, + new BibleTransformer($verify_segmentation), new DataArraySerializer() ); return $bibles_return->paginateWith(new IlluminatePaginatorAdapter($bibles)); @@ -402,6 +411,12 @@ function () use ($access_group_ids, $limit, $version_query) { * @OA\Schema(type="boolean", default=false), * description="When true, each entry in books includes a filesets array of { id, type } for all filesets that have content for that book; same id can appear with different types." * ), + * @OA\Parameter( + * name="verify_segmentation", + * in="query", + * @OA\Schema(type="boolean", default=false), + * description="When true, each fileset object includes a segmentation_type key (section, chapter, or null)." + * ), * @OA\Response( * response=200, * description="successful operation", @@ -419,6 +434,7 @@ public function show($id = null) $include_font = is_null(checkParam('include_font')) ? true : checkBoolean('include_font', false); $verify_content = is_null(checkParam('verify_content')) ? false : checkBoolean('verify_content', false); + $verify_segmentation = checkBoolean('verify_segmentation'); if ($this->v === 2 || $this->v === 3) { $id = substr($id, 0, 6); @@ -445,9 +461,10 @@ public function show($id = null) $access_group_ids, $include_font, $verify_content, - $id + $id, + $verify_segmentation )) - : $this->reply(fractal($bible, new BibleTransformer(), $this->serializer)); + : $this->reply(fractal($bible, new BibleTransformer($verify_segmentation), $this->serializer)); } private function loadBibleForShow(string $id, $access_group_ids, bool $include_font) @@ -479,17 +496,17 @@ function () use ($access_group_ids, $id, $include_font) { ); } - private function buildVerifyContentResponsePayload($bible, $access_group_ids, $include_font, $verify_content, $id) : array + private function buildVerifyContentResponsePayload($bible, $access_group_ids, $include_font, $verify_content, $id, bool $verify_segmentation = false) : array { return cacheRemember('bibles_show_verify_content_response', - [$id, $access_group_ids->toString(), $include_font, $verify_content], + [$id, $access_group_ids->toString(), $include_font, $verify_content, $verify_segmentation], now()->addDay(), - function () use ($bible, $access_group_ids, $verify_content, $id) { + function () use ($bible, $access_group_ids, $verify_content, $id, $verify_segmentation) { $book_fileset_map = cacheRemember( 'bibles_show_book_filesets', - [$id, $access_group_ids->toString(), $verify_content], + [$id, $access_group_ids->toString(), $verify_content, $verify_segmentation], now()->addDay(), - function () use ($bible) { + function () use ($bible, $verify_segmentation) { $batch_resolver = new FilesetBookIdBatchResolver(); $single_resolver = new FilesetBookIdResolver(); $fileset_book_ids = $batch_resolver->resolve($bible->filesets); @@ -499,7 +516,11 @@ function () use ($bible) { ?? $single_resolver->resolve($fileset); foreach ($book_ids as $book_id) { $map[$book_id] = $map[$book_id] ?? []; - $map[$book_id][] = ['id' => $fileset->id, 'type' => $fileset->set_type_code]; + $entry = ['id' => $fileset->id, 'type' => $fileset->set_type_code]; + if ($verify_segmentation) { + $entry['segmentation_type'] = $fileset->segmentation_type ?? null; + } + $map[$book_id][] = $entry; } } return $map; @@ -512,7 +533,7 @@ function () use ($bible) { $book->filesets = array_values($book_fileset_map[$book->book_id] ?? []); return $book; })); - return fractal($bible_response, new BibleTransformer(), $this->serializer)->toArray(); + return fractal($bible_response, new BibleTransformer($verify_segmentation), $this->serializer)->toArray(); } ); } diff --git a/app/Http/Controllers/Plan/PlansController.php b/app/Http/Controllers/Plan/PlansController.php index 6f943650a..86bb8a4e3 100644 --- a/app/Http/Controllers/Plan/PlansController.php +++ b/app/Http/Controllers/Plan/PlansController.php @@ -523,7 +523,7 @@ public function start(Request $request, $plan_id) * security={{"api_token":{}}}, * @OA\Parameter(name="plan_id", in="path", required=true, @OA\Schema(ref="#/components/schemas/Plan/properties/id")), * @OA\Parameter(name="days", in="query", required=true, @OA\Schema(type="integer"), description="Number of days to add to the plan"), - * @OA\Parameter(name="add_to_end", in="query", required=false, @OA\Schema(type="true"), description="If new days to add should be added to end of list of days") + * @OA\Parameter(name="add_to_end", in="query", required=false, @OA\Schema(type="boolean"), description="If new days to add should be added to end of list of days"), * @OA\Response( * response=200, * description="successful operation", diff --git a/app/Models/Bible/BibleFileset.php b/app/Models/Bible/BibleFileset.php index e666430e3..089bc2388 100644 --- a/app/Models/Bible/BibleFileset.php +++ b/app/Models/Bible/BibleFileset.php @@ -107,6 +107,19 @@ class BibleFileset extends Model */ protected $mode_id; + /** + * + * @OA\Property( + * property="segmentation_type", + * type="string", + * enum={"section","chapter"}, + * nullable=true, + * description="Segmentation strategy for this fileset (section, chapter, or null when unspecified)." + * ) + * + */ + protected $segmentation_type; + protected $created_at; @@ -117,7 +130,7 @@ class BibleFileset extends Model * @OA\Property( * title="license_group_id", * type="integer", - * description="The liceense group id", + * description="The license group id", * ) */ protected $license_group_id; diff --git a/app/Transformers/BibleTransformer.php b/app/Transformers/BibleTransformer.php index c107ce7c1..eac3f0aec 100644 --- a/app/Transformers/BibleTransformer.php +++ b/app/Transformers/BibleTransformer.php @@ -12,6 +12,15 @@ class BibleTransformer extends BaseTransformer { use OrganizationFilterTrait; + + private bool $verify_segmentation; + + public function __construct(bool $verify_segmentation = false) + { + parent::__construct(); + $this->verify_segmentation = $verify_segmentation; + } + /** * A Fractal transformer. * @@ -338,6 +347,10 @@ private function filesetWithMeta(BibleFileset $fileset) : array 'size' => $fileset->set_size_code, ]; + if ($this->verify_segmentation) { + $fileset_data['segmentation_type'] = $fileset->segmentation_type ?? null; + } + $meta_records_indexed = $fileset->getMetaTagsIndexedByName(); if (!empty($meta_records_indexed)) { diff --git a/app/Transformers/CopyrightTransformer.php b/app/Transformers/CopyrightTransformer.php index 39c861468..c44adcab0 100644 --- a/app/Transformers/CopyrightTransformer.php +++ b/app/Transformers/CopyrightTransformer.php @@ -9,6 +9,14 @@ class CopyrightTransformer extends BaseTransformer { use OrganizationFilterTrait; + private bool $verify_segmentation; + + public function __construct(bool $verify_segmentation = false) + { + parent::__construct(); + $this->verify_segmentation = $verify_segmentation; + } + /** * Transform copyright fileset data, filtering organizations as needed. * @@ -23,6 +31,10 @@ public function transform(BibleFileset $fileset) 'size' => $fileset->size, ]; + if ($this->verify_segmentation) { + $transformed['segmentation_type'] = $fileset->segmentation_type ?? null; + } + if (isset($fileset->asset_id)) { $transformed['asset_id'] = $fileset->asset_id; } diff --git a/database/factories/Bible/BibleFactory.php b/database/factories/Bible/BibleFactory.php index b368814a1..e9e739966 100644 --- a/database/factories/Bible/BibleFactory.php +++ b/database/factories/Bible/BibleFactory.php @@ -68,6 +68,9 @@ ]; }); +$factory->state(\App\Models\Bible\BibleFileset::class, 'segmentation_section', ['segmentation_type' => 'section']); +$factory->state(\App\Models\Bible\BibleFileset::class, 'segmentation_chapter', ['segmentation_type' => 'chapter']); + $factory->define(\App\Models\Bible\BibleFile::class, function (Faker $faker) { return [ 'id' => '', diff --git a/database/migrations/2026_04_28_180000_add_segmentation_type_to_bible_filesets.php b/database/migrations/2026_04_28_180000_add_segmentation_type_to_bible_filesets.php new file mode 100644 index 000000000..40c230328 --- /dev/null +++ b/database/migrations/2026_04_28_180000_add_segmentation_type_to_bible_filesets.php @@ -0,0 +1,35 @@ +table('bible_filesets', function (Blueprint $table) { + $table->enum('segmentation_type', ['section', 'chapter']) + ->nullable() + ->default(null) + ->after('archived'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::connection('dbp')->table('bible_filesets', function (Blueprint $table) { + $table->dropColumn('segmentation_type'); + }); + } +} diff --git a/public/openapi.json b/public/openapi.json index 6812c88af..a85a16f49 100644 --- a/public/openapi.json +++ b/public/openapi.json @@ -322,6 +322,115 @@ } } }, + "/download/package-create": { + "post": { + "tags": [ + "Bibles" + ], + "summary": "Create a download package from filesets", + "description": "Accepts a JSON payload containing fileset IDs and an encryption type, then proxies the request to BBHub package creation.", + "operationId": "v4_download_package_create", + "parameters": [ + { + "$ref": "#/components/parameters/version_number" + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "required": [ + "filesets", + "encryptionType" + ], + "properties": { + "filesets": { + "type": "array", + "items": { + "type": "string", + "minLength": 1 + }, + "minItems": 1, + "uniqueItems": true, + "example": [ + "ENGESVN2DA", + "ENGESVO1DA" + ] + }, + "encryptionType": { + "type": "integer", + "example": 1 + } + }, + "type": "object" + } + } + } + }, + "responses": { + "200": { + "description": "Response from the upstream BBHub service (status and body are proxied).", + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": true + } + } + } + }, + "400": { + "description": "Request body must be valid JSON.", + "content": { + "application/json": { + "schema": { + "properties": { + "error": { + "type": "string", + "example": "Request body must be valid JSON." + } + }, + "type": "object" + } + } + } + }, + "422": { + "description": "Validation failed for the request body.", + "content": { + "application/json": { + "schema": { + "properties": { + "error": { + "type": "string", + "example": "The filesets field is required." + } + }, + "type": "object" + } + } + } + }, + "503": { + "description": "BBHub is unavailable.", + "content": { + "application/json": { + "schema": { + "properties": { + "error": { + "type": "string", + "example": "BBHub is unavailable." + } + }, + "type": "object" + } + } + } + } + } + } + }, "/bibles/verses/{language_code}/{book_id}/{chapter_id}/{verse_number?}": { "get": { "tags": [ @@ -516,6 +625,25 @@ }, "example": "true" }, + { + "name": "show_country", + "in": "query", + "description": "Include country_id field in response. When true, adds country_id field containing the country ID from languages.country_id.", + "schema": { + "type": "boolean", + "default": false + }, + "example": "true" + }, + { + "name": "verify_segmentation", + "in": "query", + "description": "When true, each fileset object includes a segmentation_type key (section, chapter, or null).", + "schema": { + "type": "boolean", + "default": false + } + }, { "$ref": "#/components/parameters/page" }, @@ -631,6 +759,24 @@ "name": "include_font", "in": "query" }, + { + "name": "verify_content", + "in": "query", + "description": "When true, each entry in books includes a filesets array of { id, type } for all filesets that have content for that book; same id can appear with different types.", + "schema": { + "type": "boolean", + "default": false + } + }, + { + "name": "verify_segmentation", + "in": "query", + "description": "When true, each fileset object includes a segmentation_type key (section, chapter, or null).", + "schema": { + "type": "boolean", + "default": false + } + }, { "$ref": "#/components/parameters/version_number" } @@ -1566,8 +1712,15 @@ "size": { "$ref": "#/components/schemas/BibleFileset/properties/set_size_code" }, + "segmentation_type": { + "$ref": "#/components/schemas/BibleFileset/properties/segmentation_type" + }, + "asset_id": { + "description": "The S3 bucket / asset identifier the fileset is stored under", + "type": "string" + }, "copyright": { - "$ref": "#/components/schemas/BibleFilesetCopyright" + "$ref": "#/components/schemas/v4_bible_filesets.copyright_details" } }, "type": "object", @@ -1575,6 +1728,162 @@ "name": "v4_bible_filesets.copyright" } }, + "v4_bible_filesets.copyright_details": { + "title": "v4_bible_filesets.copyright_details", + "description": "Structured copyright metadata returned by CopyrightTransformer", + "properties": { + "copyright_date": { + "description": "Copyright date as recorded in the license group", + "type": "string", + "format": "date", + "nullable": true + }, + "copyright": { + "description": "Copyright statement text", + "type": "string", + "nullable": true + }, + "created_at": { + "type": "string", + "format": "date-time", + "nullable": true + }, + "updated_at": { + "type": "string", + "format": "date-time", + "nullable": true + }, + "open_access": { + "description": "Whether the fileset is open access", + "type": "boolean", + "default": false + }, + "is_combined": { + "description": "Whether the copyright is combined across multiple licensors", + "type": "boolean", + "default": false + }, + "organizations": { + "description": "Licensor organizations associated with this copyright (only present when at least one is attached)", + "type": "array", + "items": { + "$ref": "#/components/schemas/v4_bible_filesets.copyright_organization" + } + } + }, + "type": "object" + }, + "v4_bible_filesets.copyright_organization": { + "title": "v4_bible_filesets.copyright_organization", + "description": "Organization shape used in the copyright details payload", + "properties": { + "id": { + "type": "integer", + "nullable": true + }, + "slug": { + "type": "string", + "nullable": true + }, + "abbreviation": { + "type": "string", + "nullable": true + }, + "description": { + "type": "string", + "nullable": true + }, + "description_short": { + "description": "Sourced from the organization's tagline", + "type": "string", + "nullable": true + }, + "phone": { + "type": "string", + "nullable": true + }, + "email": { + "type": "string", + "nullable": true + }, + "email_director": { + "type": "string", + "nullable": true + }, + "logos": { + "type": "array", + "items": { + "type": "object" + } + }, + "primaryColor": { + "type": "string", + "nullable": true + }, + "secondaryColor": { + "type": "string", + "nullable": true + }, + "inactive": { + "type": "boolean", + "default": false + }, + "url_site": { + "description": "Sourced from the organization's url_website", + "type": "string" + }, + "url_donate": { + "type": "string" + }, + "url_twitter": { + "type": "string" + }, + "url_facebook": { + "type": "string" + }, + "address": { + "type": "string", + "nullable": true + }, + "address2": { + "type": "string", + "nullable": true + }, + "city": { + "type": "string", + "nullable": true + }, + "state": { + "type": "string", + "nullable": true + }, + "country": { + "type": "string", + "nullable": true + }, + "zip": { + "type": "string", + "nullable": true + }, + "latitude": { + "type": "number", + "format": "float", + "nullable": true + }, + "longitude": { + "type": "number", + "format": "float", + "nullable": true + }, + "translations": { + "type": "array", + "items": { + "type": "object" + } + } + }, + "type": "object" + }, "v4_bible_filesets_download.index": { "title": "v4_bible_filesets_download.index", "description": "v4_bible_filesets_download.index", @@ -2232,6 +2541,14 @@ "minimum": 0, "example": 683, "nullable": true + }, + "is_complete_chapter": { + "title": "is_complete_chapter", + "description": "If the file is a complete chapter, this field will be true", + "type": "boolean", + "default": false, + "example": true, + "nullable": false } }, "type": "object", @@ -2300,6 +2617,23 @@ }, "set_size_code": { "$ref": "#/components/schemas/BibleFilesetSize/properties/set_size_code" + }, + "mode_id": { + "$ref": "#/components/schemas/BibleFilesetMode/properties/id" + }, + "segmentation_type": { + "description": "Segmentation strategy for this fileset (section, chapter, or null when unspecified).", + "type": "string", + "enum": [ + "section", + "chapter" + ], + "nullable": true + }, + "license_group_id": { + "title": "license_group_id", + "description": "The license group id", + "type": "integer" } }, "type": "object", @@ -2307,38 +2641,30 @@ "name": "BibleFileset" } }, - "BibleFilesetCopyright": { - "title": "Bible Fileset Copyright", - "description": "BibleFilesetCopyright", + "BibleFilesetMode": { + "title": "BibleFilesetMode", + "description": "The Bible fileset mode model communicates information about generalized fileset modes", + "required": [ + "filename" + ], "properties": { - "copyright_date": { - "title": "copyright_date", - "description": "The copyright date created copyright", - "type": "string", - "example": "2014" - }, - "copyright": { - "title": "copyright", - "description": "The copyright", - "type": "string", - "example": "© Ethnos360" + "id": { + "title": "id", + "description": "The id", + "type": "integer", + "minimum": 0, + "example": 4 }, - "copyright_description": { - "title": "copyright_description", - "description": "The copyright description", + "name": { + "title": "name", + "description": "The name", "type": "string", - "example": "© Ethnos360" - }, - "open_access": { - "title": "open_access", - "description": "The open_access description", - "type": "integer", - "example": 1 + "example": "video" } }, "type": "object", "xml": { - "name": "BibleFilesetCopyright" + "name": "BibleFilesetMode" } }, "BibleFilesetSize": { @@ -3639,6 +3965,50 @@ "name": "NumeralSystem" } }, + "LicenseGroup": { + "title": "License Group", + "description": "LicenseGroup", + "properties": { + "id": { + "title": "id", + "description": "The license group id", + "type": "integer" + }, + "name": { + "title": "name", + "description": "The license group name", + "type": "string", + "maxLength": 64 + }, + "permission_pattern_id": { + "title": "permission_pattern_id", + "description": "The permission pattern id", + "type": "integer", + "nullable": true + }, + "description": { + "title": "description", + "description": "The license group description", + "type": "string" + }, + "copyright": { + "title": "copyright", + "description": "The copyright text", + "type": "string", + "nullable": true + }, + "is_copyright_combined": { + "title": "is_copyright_combined", + "description": "Is this a combined copyright from multiple sources", + "type": "boolean", + "nullable": true + } + }, + "type": "object", + "xml": { + "name": "LicenseGroup" + } + }, "Asset": { "title": "Asset", "description": "Asset", @@ -4871,6 +5241,11 @@ "date": { "$ref": "#/components/schemas/Bible/properties/date" }, + "country_id": { + "description": "Country ID from languages.country_id (only included when show_country=true)", + "type": "string", + "nullable": true + }, "filesets": { "properties": { "dbp-prod": { diff --git a/tests/Integration/BiblesRoutesTest.php b/tests/Integration/BiblesRoutesTest.php index 4bd6872f3..6b2b3d1b3 100644 --- a/tests/Integration/BiblesRoutesTest.php +++ b/tests/Integration/BiblesRoutesTest.php @@ -70,6 +70,60 @@ public function bibleFilesetsCopyright() $response = $this->withHeaders($this->params)->get($path); $response->assertSuccessful(); + + // CopyrightTransformer uses ArraySerializer — response is the payload directly, no 'data' wrapper. + $payload = json_decode($response->getContent(), true) ?? []; + $this->assertArrayNotHasKey( + 'segmentation_type', + $payload, + 'Default copyright response must not contain segmentation_type' + ); + } + + /** + * @category V4_API + * @category Route Name: v4_internal_bible_filesets.copyright + * @category Route Path: https://api.dbp.test/bibles/filesets/{fileset_id}/copyright?v=4&key={key}&verify_segmentation=true + * @see \App\Http\Controllers\Bible\BibleFileSetsController::copyright + * @group BibleRoutes + * @group V4 + * @group travis + * @test + */ + public function bibleFilesetsCopyrightWithVerifySegmentation() + { + $fileset = BibleFileset::whereNotNull('segmentation_type') + ->where('hidden', 0) + ->where('archived', 0) + ->inRandomOrder() + ->first(); + + if (!$fileset) { + $this->markTestSkipped('No fileset with non-null segmentation_type seeded in this environment.'); + } + + $params = array_merge( + ['fileset_id' => $fileset->id, 'verify_segmentation' => 'true'], + $this->params + ); + $path = route('v4_internal_bible_filesets.copyright', $params); + echo "\nTesting: $path"; + + $response = $this->withHeaders($this->params)->get($path); + $response->assertSuccessful(); + + // CopyrightTransformer uses ArraySerializer — response is the payload directly, no 'data' wrapper. + $payload = json_decode($response->getContent(), true) ?? []; + $this->assertArrayHasKey( + 'segmentation_type', + $payload, + 'verify_segmentation=true must include segmentation_type key in copyright response' + ); + $this->assertContains( + $payload['segmentation_type'], + ['section', 'chapter', null], + 'segmentation_type must be section, chapter, or null' + ); } /** @@ -255,6 +309,82 @@ public function bibleOne() echo "\nTesting: $path"; $response = $this->withHeaders($this->params)->get($path); $response->assertSuccessful(); + + $decoded = json_decode($response->getContent(), true); + $payload = is_array($decoded) ? ($decoded['data'] ?? []) : []; + foreach ($payload['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + $this->assertArrayNotHasKey( + 'segmentation_type', + $fileset, + "Default v4_bible.one response must not contain segmentation_type for fileset {$fileset['id']}" + ); + } + } + } + + /** + * @category V4_API + * @category Route Name: v4_bible.one + * @category Route Path: https://api.dbp.test/bibles/{bible_id}?v=4&key={key}&verify_segmentation=true + * @see \App\Http\Controllers\Bible\BiblesController::show + * @group BibleRoutes + * @group V4 + * @group travis + * @test + */ + public function bibleOneWithVerifySegmentation() + { + // Discover an accessible bible whose filesets carry segmentation_type by hitting v4_bible.all first. + $index_path = route('v4_bible.all', array_merge(['verify_segmentation' => 'true'], $this->params)); + $index_response = $this->withHeaders($this->params)->get($index_path); + $index_response->assertSuccessful(); + $index_decoded = json_decode($index_response->getContent(), true); + $index_payload = is_array($index_decoded) ? ($index_decoded['data'] ?? []) : []; + + $bible_id = null; + foreach ($index_payload as $bible) { + foreach ($bible['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + if (!is_null($fileset['segmentation_type'] ?? null)) { + $bible_id = $bible['abbr']; + break 3; + } + } + } + } + + if (!$bible_id) { + $this->markTestSkipped('No accessible bible with a fileset carrying non-null segmentation_type in this environment.'); + } + + $path = route('v4_bible.one', array_merge( + ['bible_id' => $bible_id, 'verify_segmentation' => 'true'], + $this->params + )); + echo "\nTesting: $path"; + $response = $this->withHeaders($this->params)->get($path); + $response->assertSuccessful(); + + $decoded = json_decode($response->getContent(), true); + $payload = is_array($decoded) ? ($decoded['data'] ?? []) : []; + $checked_fileset_count = 0; + foreach ($payload['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + $this->assertArrayHasKey( + 'segmentation_type', + $fileset, + "verify_segmentation=true must include segmentation_type for fileset {$fileset['id']}" + ); + $this->assertContains( + $fileset['segmentation_type'], + ['section', 'chapter', null], + "segmentation_type must be section, chapter, or null for fileset {$fileset['id']}" + ); + $checked_fileset_count++; + } + } + $this->assertGreaterThan(0, $checked_fileset_count, 'Expected at least one fileset to verify'); } /** @@ -273,5 +403,59 @@ public function bibleAll() echo "\nTesting: $path"; $response = $this->withHeaders($this->params)->get($path); $response->assertSuccessful(); + + $decoded = json_decode($response->getContent(), true); + $payload = is_array($decoded) ? ($decoded['data'] ?? []) : []; + foreach ($payload as $bible) { + foreach ($bible['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + $this->assertArrayNotHasKey( + 'segmentation_type', + $fileset, + "Default v4_bible.all response must not contain segmentation_type for fileset {$fileset['id']}" + ); + } + } + } + } + + /** + * @category V4_API + * @category Route Name: v4_bible.all + * @category Route Path: https://api.dbp.test/bibles?v=4&key={key}&verify_segmentation=true + * @see \App\Http\Controllers\Bible\BiblesController::index + * @group BibleRoutes + * @group V4 + * @group travis + * @test + */ + public function bibleAllWithVerifySegmentation() + { + $path = route('v4_bible.all', array_merge(['verify_segmentation' => 'true'], $this->params)); + echo "\nTesting: $path"; + $response = $this->withHeaders($this->params)->get($path); + $response->assertSuccessful(); + + $decoded = json_decode($response->getContent(), true); + $payload = is_array($decoded) ? ($decoded['data'] ?? []) : []; + $checked_fileset_count = 0; + foreach ($payload as $bible) { + foreach ($bible['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + $this->assertArrayHasKey( + 'segmentation_type', + $fileset, + "verify_segmentation=true must include segmentation_type for fileset {$fileset['id']}" + ); + $this->assertContains( + $fileset['segmentation_type'], + ['section', 'chapter', null], + "segmentation_type must be section, chapter, or null for fileset {$fileset['id']}" + ); + $checked_fileset_count++; + } + } + } + $this->assertGreaterThan(0, $checked_fileset_count, 'Expected at least one fileset to verify'); } } From 3b53ce8ce055c4a054dae027800996ce9086bb63 Mon Sep 17 00:00:00 2001 From: Victor Gonzalez Date: Wed, 6 May 2026 17:59:39 -0500 Subject: [PATCH 5/8] Task-90749 Add per-book verse_starts on /bibles/{id} when both verify_segmentation=true and verify_content=true. (cherry picked from commit f471d34977601267f0a506391278b7e40fc6c9fd) --- .../Controllers/Bible/BiblesController.php | 39 ++++- app/Models/Bible/BibleFileset.php | 33 +++- .../Bibles/FilesetVerseStartsResolver.php | 59 +++++++ public/openapi.json | 22 ++- tests/Integration/BiblesRoutesTest.php | 159 ++++++++++++++++++ 5 files changed, 300 insertions(+), 12 deletions(-) create mode 100644 app/Services/Bibles/FilesetVerseStartsResolver.php diff --git a/app/Http/Controllers/Bible/BiblesController.php b/app/Http/Controllers/Bible/BiblesController.php index fe32294ec..c4118dc86 100644 --- a/app/Http/Controllers/Bible/BiblesController.php +++ b/app/Http/Controllers/Bible/BiblesController.php @@ -30,6 +30,7 @@ use App\Services\Bibles\BibleFilesetService; use App\Services\Bibles\FilesetBookIdBatchResolver; use App\Services\Bibles\FilesetBookIdResolver; +use App\Services\Bibles\FilesetVerseStartsResolver; use Exception; use GuzzleHttp\Client; use Illuminate\Http\Request; @@ -415,7 +416,7 @@ function () use ($access_group_ids, $limit, $version_query) { * name="verify_segmentation", * in="query", * @OA\Schema(type="boolean", default=false), - * description="When true, each fileset object includes a segmentation_type key (section, chapter, or null)." + * description="When true, each fileset object includes a segmentation_type key (section, chapter, or null). When combined with verify_content=true, qualifying audio filesets (segmentation_type='section') under each books[].filesets[] entry will additionally include a verse_starts array of {chapter_start, verse_start, verse_start_alt} items describing that book's section boundaries." * ), * @OA\Response( * response=200, @@ -506,7 +507,10 @@ function () use ($bible, $access_group_ids, $verify_content, $id, $verify_segmen 'bibles_show_book_filesets', [$id, $access_group_ids->toString(), $verify_content, $verify_segmentation], now()->addDay(), - function () use ($bible, $verify_segmentation) { + function () use ($bible, $access_group_ids, $id, $verify_segmentation) { + $verse_starts_map = $verify_segmentation + ? $this->loadVerseStartsMap($bible, $id, $access_group_ids) + : []; $batch_resolver = new FilesetBookIdBatchResolver(); $single_resolver = new FilesetBookIdResolver(); $fileset_book_ids = $batch_resolver->resolve($bible->filesets); @@ -519,6 +523,9 @@ function () use ($bible, $verify_segmentation) { $entry = ['id' => $fileset->id, 'type' => $fileset->set_type_code]; if ($verify_segmentation) { $entry['segmentation_type'] = $fileset->segmentation_type ?? null; + if (isset($verse_starts_map[$fileset->hash_id][$book_id])) { + $entry['verse_starts'] = $verse_starts_map[$fileset->hash_id][$book_id]; + } } $map[$book_id][] = $entry; } @@ -538,6 +545,34 @@ function () use ($bible, $verify_segmentation) { ); } + /** + * Load the verse_starts map for any qualifying section-segmented audio filesets on the bible. + * The map is keyed by hash_id then book_id, so attaching per-book entries is constant-time. + * Returns an empty array when no fileset qualifies (no DB query is issued in that case). + * + * The cache key includes the access group fingerprint because $bible->filesets + * is already filtered by isContentAvailable(); a narrower-access request must + * not poison the cache for a broader-access request that can see additional + * qualifying filesets. + */ + private function loadVerseStartsMap(Bible $bible, string $bible_id, $access_group_ids) : array + { + $resolver = new FilesetVerseStartsResolver(); + $qualifying_filesets = $resolver->qualifyingFilesets($bible->filesets); + if ($qualifying_filesets->isEmpty()) { + return []; + } + + return cacheRemember( + 'bibles_show_verse_starts', + [$bible_id, $access_group_ids->toString()], + now()->addDay(), + function () use ($resolver, $qualifying_filesets) { + return $resolver->resolveForFilesets($qualifying_filesets); + } + ); + } + /** * * @OA\Get( diff --git a/app/Models/Bible/BibleFileset.php b/app/Models/Bible/BibleFileset.php index 089bc2388..c2208025e 100644 --- a/app/Models/Bible/BibleFileset.php +++ b/app/Models/Bible/BibleFileset.php @@ -53,6 +53,13 @@ class BibleFileset extends Model public const TYPE_TEXT_PLAIN = 'text_plain'; public const TYPE_TEXT_USX = 'text_usx'; + public const AUDIO_TYPES = [ + self::TYPE_AUDIO, + self::TYPE_AUDIO_DRAMA, + self::TYPE_AUDIO_STREAM, + self::TYPE_AUDIO_DRAMA_STREAM, + ]; + public const NEW_TEXT_PLAIN_FILESET_LENGTH = 10; public const OLD_TEXT_PLAIN_FILESET_LENGTH = 6; public const V1_AUDIO_16_KBPS_FILESET_LENGTH = 12; @@ -120,6 +127,22 @@ class BibleFileset extends Model */ protected $segmentation_type; + /** + * + * @OA\Property( + * property="verse_starts", + * type="array", + * description="Section boundaries for the fileset, scoped to the parent book. Returned only on per-book fileset entries (under books[].filesets[]) when both verify_segmentation=true and verify_content=true are present, and only for audio filesets with segmentation_type='section'. verse_start is the numeric verse_sequence for client-side arithmetic; verse_start_alt preserves the original verse marker, which may be alphanumeric (e.g. '001', '2b').", + * @OA\Items( + * type="object", + * @OA\Property(property="chapter_start", type="integer", example=1, description="The starting chapter of the section."), + * @OA\Property(property="verse_start", ref="#/components/schemas/BibleFile/properties/verse_sequence"), + * @OA\Property(property="verse_start_alt", ref="#/components/schemas/BibleFile/properties/verse_start") + * ) + * ) + * + */ + protected $verse_starts; protected $created_at; @@ -460,15 +483,7 @@ public static function getConditionTagExcludeByIds(\Illuminate\Support\Collectio */ public function isAudio() : bool { - return in_array( - $this['set_type_code'], - [ - BibleFileset::TYPE_AUDIO_DRAMA, - BibleFileset::TYPE_AUDIO, - BibleFileset::TYPE_AUDIO_DRAMA, - BibleFileset::TYPE_AUDIO_DRAMA_STREAM - ] - ); + return in_array($this['set_type_code'], self::AUDIO_TYPES, true); } /** diff --git a/app/Services/Bibles/FilesetVerseStartsResolver.php b/app/Services/Bibles/FilesetVerseStartsResolver.php new file mode 100644 index 000000000..9a49f9e37 --- /dev/null +++ b/app/Services/Bibles/FilesetVerseStartsResolver.php @@ -0,0 +1,59 @@ +filter(function ($fileset) { + return ($fileset->segmentation_type ?? null) === self::SEGMENTATION_TYPE_SECTION + && $fileset->isAudio(); + })->values(); + } + + /** + * Fetch verse_starts data for the qualifying filesets in a single + * batched query and return a map keyed first by hash_id, then by + * book_id, so per-book attach lookups are constant-time. + * + * @return array>> + */ + public function resolveForFilesets(Collection $qualifying_filesets) : array + { + $hash_ids = $qualifying_filesets->pluck('hash_id')->unique()->values(); + if ($hash_ids->isEmpty()) { + return []; + } + + $rows = BibleFile::select(['hash_id', 'book_id', 'chapter_start', 'verse_start', 'verse_sequence']) + ->whereIn('hash_id', $hash_ids) + ->orderBy('hash_id') + ->orderBy('book_id') + ->orderBy('chapter_start') + ->orderBy('verse_sequence') + ->get(); + + $map = []; + foreach ($rows as $row) { + $map[$row->hash_id][$row->book_id][] = [ + 'chapter_start' => $row->chapter_start, + 'verse_start' => $row->verse_sequence, + 'verse_start_alt' => $row->verse_start, + ]; + } + return $map; + } +} diff --git a/public/openapi.json b/public/openapi.json index a85a16f49..cd9401291 100644 --- a/public/openapi.json +++ b/public/openapi.json @@ -771,7 +771,7 @@ { "name": "verify_segmentation", "in": "query", - "description": "When true, each fileset object includes a segmentation_type key (section, chapter, or null).", + "description": "When true, each fileset object includes a segmentation_type key (section, chapter, or null). When combined with verify_content=true, qualifying audio filesets (segmentation_type='section') under each books[].filesets[] entry will additionally include a verse_starts array of {chapter_start, verse_start, verse_start_alt} items describing that book's section boundaries.", "schema": { "type": "boolean", "default": false @@ -2630,6 +2630,26 @@ ], "nullable": true }, + "verse_starts": { + "description": "Section boundaries for the fileset, scoped to the parent book. Returned only on per-book fileset entries (under books[].filesets[]) when both verify_segmentation=true and verify_content=true are present, and only for audio filesets with segmentation_type='section'. verse_start is the numeric verse_sequence for client-side arithmetic; verse_start_alt preserves the original verse marker, which may be alphanumeric (e.g. '001', '2b').", + "type": "array", + "items": { + "properties": { + "chapter_start": { + "description": "The starting chapter of the section.", + "type": "integer", + "example": 1 + }, + "verse_start": { + "$ref": "#/components/schemas/BibleFile/properties/verse_sequence" + }, + "verse_start_alt": { + "$ref": "#/components/schemas/BibleFile/properties/verse_start" + } + }, + "type": "object" + } + }, "license_group_id": { "title": "license_group_id", "description": "The license group id", diff --git a/tests/Integration/BiblesRoutesTest.php b/tests/Integration/BiblesRoutesTest.php index 6b2b3d1b3..d188b1326 100644 --- a/tests/Integration/BiblesRoutesTest.php +++ b/tests/Integration/BiblesRoutesTest.php @@ -387,6 +387,165 @@ public function bibleOneWithVerifySegmentation() $this->assertGreaterThan(0, $checked_fileset_count, 'Expected at least one fileset to verify'); } + /** + * @category V4_API + * @category Route Name: v4_bible.one + * @category Route Path: https://api.dbp.test/bibles/{bible_id}?v=4&key={key}&verify_segmentation=true&verify_content=true + * @see \App\Http\Controllers\Bible\BiblesController::show + * @group BibleRoutes + * @group V4 + * @group travis + * @test + */ + public function bibleOneWithVerseStarts() + { + $audio_types = BibleFileset::AUDIO_TYPES; + + // Discover an accessible bible whose filesets carry a section-segmented audio fileset. + $index_path = route('v4_bible.all', array_merge(['verify_segmentation' => 'true'], $this->params)); + $index_response = $this->withHeaders($this->params)->get($index_path); + $index_response->assertSuccessful(); + $index_decoded = json_decode($index_response->getContent(), true); + $index_payload = is_array($index_decoded) ? ($index_decoded['data'] ?? []) : []; + + $bible_id = null; + foreach ($index_payload as $bible) { + foreach ($bible['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + if ( + ($fileset['segmentation_type'] ?? null) === 'section' + && in_array($fileset['type'] ?? null, $audio_types, true) + ) { + $bible_id = $bible['abbr']; + break 3; + } + } + } + } + + if (!$bible_id) { + $this->markTestSkipped('No accessible bible with a section-segmented audio fileset in this environment.'); + } + + // Both flags: per-book verse_starts must appear on qualifying filesets only. + $path = route('v4_bible.one', array_merge( + ['bible_id' => $bible_id, 'verify_segmentation' => 'true', 'verify_content' => 'true'], + $this->params + )); + echo "\nTesting: $path"; + $response = $this->withHeaders($this->params)->get($path); + $response->assertSuccessful(); + + $decoded = json_decode($response->getContent(), true); + $payload = is_array($decoded) ? ($decoded['data'] ?? []) : []; + + // Top-level filesets map must NOT include verse_starts. + foreach ($payload['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + $this->assertArrayNotHasKey( + 'verse_starts', + $fileset, + "Top-level fileset {$fileset['id']} must not include verse_starts" + ); + } + } + + $qualifying_book_filesets = 0; + foreach ($payload['books'] ?? [] as $book) { + foreach ($book['filesets'] ?? [] as $book_fileset) { + $is_qualifying = ($book_fileset['segmentation_type'] ?? null) === 'section' + && in_array($book_fileset['type'] ?? null, $audio_types, true); + if ($is_qualifying) { + $this->assertArrayHasKey( + 'verse_starts', + $book_fileset, + "Qualifying per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must include verse_starts" + ); + $this->assertIsArray($book_fileset['verse_starts']); + $this->assertNotEmpty( + $book_fileset['verse_starts'], + "verse_starts must contain at least one entry for fileset {$book_fileset['id']} (book {$book['book_id']})" + ); + foreach ($book_fileset['verse_starts'] as $entry) { + $this->assertArrayHasKey('chapter_start', $entry); + $this->assertArrayHasKey('verse_start', $entry); + $this->assertArrayHasKey('verse_start_alt', $entry); + $this->assertArrayNotHasKey( + 'book_id', + $entry, + "verse_starts entry must not carry book_id (implied by the parent book)" + ); + } + $qualifying_book_filesets++; + } else { + $this->assertArrayNotHasKey( + 'verse_starts', + $book_fileset, + "Non-qualifying per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must not include verse_starts" + ); + } + } + } + $this->assertGreaterThan(0, $qualifying_book_filesets, 'Expected at least one qualifying per-book fileset entry'); + + // verify_segmentation alone: verse_starts must be absent everywhere. + // Cache is flushed between scenarios so each request gets a fresh Bible model + // (mimics production Memcached, where each request deserializes a fresh copy). + \Illuminate\Support\Facades\Cache::store('array')->flush(); + $seg_only_path = route('v4_bible.one', array_merge( + ['bible_id' => $bible_id, 'verify_segmentation' => 'true'], + $this->params + )); + $this->assertNoVerseStartsAnywhere( + $this->withHeaders($this->params)->get($seg_only_path), + 'verify_segmentation=true alone' + ); + + // verify_content alone: verse_starts must be absent everywhere. + \Illuminate\Support\Facades\Cache::store('array')->flush(); + $content_only_path = route('v4_bible.one', array_merge( + ['bible_id' => $bible_id, 'verify_content' => 'true'], + $this->params + )); + $this->assertNoVerseStartsAnywhere( + $this->withHeaders($this->params)->get($content_only_path), + 'verify_content=true alone' + ); + + // Default request: verse_starts must be absent everywhere. + \Illuminate\Support\Facades\Cache::store('array')->flush(); + $default_path = route('v4_bible.one', array_merge(['bible_id' => $bible_id], $this->params)); + $this->assertNoVerseStartsAnywhere( + $this->withHeaders($this->params)->get($default_path), + 'default request' + ); + } + + private function assertNoVerseStartsAnywhere($response, string $context) : void + { + $response->assertSuccessful(); + $decoded = json_decode($response->getContent(), true); + $payload = is_array($decoded) ? ($decoded['data'] ?? []) : []; + foreach ($payload['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + $this->assertArrayNotHasKey( + 'verse_starts', + $fileset, + "[$context] top-level fileset {$fileset['id']} must not include verse_starts" + ); + } + } + foreach ($payload['books'] ?? [] as $book) { + foreach ($book['filesets'] ?? [] as $book_fileset) { + $this->assertArrayNotHasKey( + 'verse_starts', + $book_fileset, + "[$context] per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must not include verse_starts" + ); + } + } + } + /** * @category V4_API * @category Route Name: v4_bible.all From 776226233fb65d74cbfb13014348b0cec56d65ff Mon Sep 17 00:00:00 2001 From: Victor Gonzalez Date: Fri, 15 May 2026 16:24:30 -0500 Subject: [PATCH 6/8] Task-90749 The per-book payload is now gated behind a new query parameter and `segmentation_type` is no longer duplicated under `books[].filesets[]`. (cherry picked from commit d553c03e345924d2f20ce4c64ff85c0ce32c6e00) --- .../Controllers/Bible/BiblesController.php | 35 ++-- app/Models/Bible/BibleFileset.php | 2 +- public/openapi.json | 13 +- tests/Integration/BiblesRoutesTest.php | 182 +++++++++++++----- 4 files changed, 173 insertions(+), 59 deletions(-) diff --git a/app/Http/Controllers/Bible/BiblesController.php b/app/Http/Controllers/Bible/BiblesController.php index c4118dc86..aa9208cb9 100644 --- a/app/Http/Controllers/Bible/BiblesController.php +++ b/app/Http/Controllers/Bible/BiblesController.php @@ -416,7 +416,13 @@ function () use ($access_group_ids, $limit, $version_query) { * name="verify_segmentation", * in="query", * @OA\Schema(type="boolean", default=false), - * description="When true, each fileset object includes a segmentation_type key (section, chapter, or null). When combined with verify_content=true, qualifying audio filesets (segmentation_type='section') under each books[].filesets[] entry will additionally include a verse_starts array of {chapter_start, verse_start, verse_start_alt} items describing that book's section boundaries." + * description="When true, the top-level filesets map exposes a segmentation_type key on each fileset entry (section, chapter, or null). This metadata is intentionally not duplicated under books[].filesets[]; clients should consult the top-level map for fileset-level metadata." + * ), + * @OA\Parameter( + * name="verse_starts", + * in="query", + * @OA\Schema(type="boolean", default=false), + * description="When true, qualifying audio filesets (segmentation_type='section') under each books[].filesets[] entry include a verse_starts array of {chapter_start, verse_start, verse_start_alt} items describing that book's section boundaries. Setting this parameter implicitly enables verify_segmentation=true and verify_content=true; sending verify_segmentation=true and verify_content=true alone does NOT return verse_starts." * ), * @OA\Response( * response=200, @@ -436,6 +442,11 @@ public function show($id = null) $include_font = is_null(checkParam('include_font')) ? true : checkBoolean('include_font', false); $verify_content = is_null(checkParam('verify_content')) ? false : checkBoolean('verify_content', false); $verify_segmentation = checkBoolean('verify_segmentation'); + $include_verse_starts = checkBoolean('verse_starts'); + if ($include_verse_starts) { + $verify_segmentation = true; + $verify_content = true; + } if ($this->v === 2 || $this->v === 3) { $id = substr($id, 0, 6); @@ -463,7 +474,8 @@ public function show($id = null) $include_font, $verify_content, $id, - $verify_segmentation + $verify_segmentation, + $include_verse_starts )) : $this->reply(fractal($bible, new BibleTransformer($verify_segmentation), $this->serializer)); } @@ -497,18 +509,18 @@ function () use ($access_group_ids, $id, $include_font) { ); } - private function buildVerifyContentResponsePayload($bible, $access_group_ids, $include_font, $verify_content, $id, bool $verify_segmentation = false) : array + private function buildVerifyContentResponsePayload($bible, $access_group_ids, $include_font, $verify_content, $id, bool $verify_segmentation = false, bool $include_verse_starts = false) : array { return cacheRemember('bibles_show_verify_content_response', - [$id, $access_group_ids->toString(), $include_font, $verify_content, $verify_segmentation], + [$id, $access_group_ids->toString(), $include_font, $verify_content, $verify_segmentation, $include_verse_starts], now()->addDay(), - function () use ($bible, $access_group_ids, $verify_content, $id, $verify_segmentation) { + function () use ($bible, $access_group_ids, $verify_content, $id, $verify_segmentation, $include_verse_starts) { $book_fileset_map = cacheRemember( 'bibles_show_book_filesets', - [$id, $access_group_ids->toString(), $verify_content, $verify_segmentation], + [$id, $access_group_ids->toString(), $verify_content, $include_verse_starts], now()->addDay(), - function () use ($bible, $access_group_ids, $id, $verify_segmentation) { - $verse_starts_map = $verify_segmentation + function () use ($bible, $access_group_ids, $id, $include_verse_starts) { + $verse_starts_map = $include_verse_starts ? $this->loadVerseStartsMap($bible, $id, $access_group_ids) : []; $batch_resolver = new FilesetBookIdBatchResolver(); @@ -521,11 +533,8 @@ function () use ($bible, $access_group_ids, $id, $verify_segmentation) { foreach ($book_ids as $book_id) { $map[$book_id] = $map[$book_id] ?? []; $entry = ['id' => $fileset->id, 'type' => $fileset->set_type_code]; - if ($verify_segmentation) { - $entry['segmentation_type'] = $fileset->segmentation_type ?? null; - if (isset($verse_starts_map[$fileset->hash_id][$book_id])) { - $entry['verse_starts'] = $verse_starts_map[$fileset->hash_id][$book_id]; - } + if ($include_verse_starts && isset($verse_starts_map[$fileset->hash_id][$book_id])) { + $entry['verse_starts'] = $verse_starts_map[$fileset->hash_id][$book_id]; } $map[$book_id][] = $entry; } diff --git a/app/Models/Bible/BibleFileset.php b/app/Models/Bible/BibleFileset.php index c2208025e..76acb921d 100644 --- a/app/Models/Bible/BibleFileset.php +++ b/app/Models/Bible/BibleFileset.php @@ -132,7 +132,7 @@ class BibleFileset extends Model * @OA\Property( * property="verse_starts", * type="array", - * description="Section boundaries for the fileset, scoped to the parent book. Returned only on per-book fileset entries (under books[].filesets[]) when both verify_segmentation=true and verify_content=true are present, and only for audio filesets with segmentation_type='section'. verse_start is the numeric verse_sequence for client-side arithmetic; verse_start_alt preserves the original verse marker, which may be alphanumeric (e.g. '001', '2b').", + * description="Section boundaries for the fileset, scoped to the parent book. Returned only on per-book fileset entries (under books[].filesets[]) when verse_starts=true is sent (which implicitly enables verify_segmentation=true and verify_content=true), and only for audio filesets with segmentation_type='section'. verse_start is the numeric verse_sequence for client-side arithmetic; verse_start_alt preserves the original verse marker, which may be alphanumeric (e.g. '001', '2b').", * @OA\Items( * type="object", * @OA\Property(property="chapter_start", type="integer", example=1, description="The starting chapter of the section."), diff --git a/public/openapi.json b/public/openapi.json index cd9401291..b9700e0de 100644 --- a/public/openapi.json +++ b/public/openapi.json @@ -771,7 +771,16 @@ { "name": "verify_segmentation", "in": "query", - "description": "When true, each fileset object includes a segmentation_type key (section, chapter, or null). When combined with verify_content=true, qualifying audio filesets (segmentation_type='section') under each books[].filesets[] entry will additionally include a verse_starts array of {chapter_start, verse_start, verse_start_alt} items describing that book's section boundaries.", + "description": "When true, the top-level filesets map exposes a segmentation_type key on each fileset entry (section, chapter, or null). This metadata is intentionally not duplicated under books[].filesets[]; clients should consult the top-level map for fileset-level metadata.", + "schema": { + "type": "boolean", + "default": false + } + }, + { + "name": "verse_starts", + "in": "query", + "description": "When true, qualifying audio filesets (segmentation_type='section') under each books[].filesets[] entry include a verse_starts array of {chapter_start, verse_start, verse_start_alt} items describing that book's section boundaries. Setting this parameter implicitly enables verify_segmentation=true and verify_content=true; sending verify_segmentation=true and verify_content=true alone does NOT return verse_starts.", "schema": { "type": "boolean", "default": false @@ -2631,7 +2640,7 @@ "nullable": true }, "verse_starts": { - "description": "Section boundaries for the fileset, scoped to the parent book. Returned only on per-book fileset entries (under books[].filesets[]) when both verify_segmentation=true and verify_content=true are present, and only for audio filesets with segmentation_type='section'. verse_start is the numeric verse_sequence for client-side arithmetic; verse_start_alt preserves the original verse marker, which may be alphanumeric (e.g. '001', '2b').", + "description": "Section boundaries for the fileset, scoped to the parent book. Returned only on per-book fileset entries (under books[].filesets[]) when verse_starts=true is sent (which implicitly enables verify_segmentation=true and verify_content=true), and only for audio filesets with segmentation_type='section'. verse_start is the numeric verse_sequence for client-side arithmetic; verse_start_alt preserves the original verse marker, which may be alphanumeric (e.g. '001', '2b').", "type": "array", "items": { "properties": { diff --git a/tests/Integration/BiblesRoutesTest.php b/tests/Integration/BiblesRoutesTest.php index d188b1326..79391ccbf 100644 --- a/tests/Integration/BiblesRoutesTest.php +++ b/tests/Integration/BiblesRoutesTest.php @@ -390,7 +390,7 @@ public function bibleOneWithVerifySegmentation() /** * @category V4_API * @category Route Name: v4_bible.one - * @category Route Path: https://api.dbp.test/bibles/{bible_id}?v=4&key={key}&verify_segmentation=true&verify_content=true + * @category Route Path: https://api.dbp.test/bibles/{bible_id}?v=4&key={key}&verse_starts=true * @see \App\Http\Controllers\Bible\BiblesController::show * @group BibleRoutes * @group V4 @@ -427,44 +427,142 @@ public function bibleOneWithVerseStarts() $this->markTestSkipped('No accessible bible with a section-segmented audio fileset in this environment.'); } - // Both flags: per-book verse_starts must appear on qualifying filesets only. - $path = route('v4_bible.one', array_merge( + // Scenario 1: all three flags explicit — per-book verse_starts must appear on qualifying filesets. + $all_flags_path = route('v4_bible.one', array_merge( + ['bible_id' => $bible_id, 'verify_segmentation' => 'true', 'verify_content' => 'true', 'verse_starts' => 'true'], + $this->params + )); + echo "\nTesting: $all_flags_path"; + $qualifying = $this->assertVerseStartsPresentOnQualifyingFilesets( + $this->withHeaders($this->params)->get($all_flags_path), + $audio_types, + 'verify_segmentation=true & verify_content=true & verse_starts=true' + ); + $this->assertGreaterThan(0, $qualifying, 'Expected at least one qualifying per-book fileset entry'); + + // Scenario 2 (behavior change): verify_segmentation=true & verify_content=true WITHOUT verse_starts + // must produce no verse_starts anywhere; the per-book filesets array is still emitted + // (verify_content is on), but per-book entries must NOT carry segmentation_type — that key + // is exposed only on the top-level filesets map. Top-level entries DO carry segmentation_type + // because verify_segmentation is explicitly on. + // Cache is flushed between scenarios so each request gets a fresh Bible model + // (mimics production Memcached, where each request deserializes a fresh copy). + \Illuminate\Support\Facades\Cache::store('array')->flush(); + $combined_no_verse_starts_path = route('v4_bible.one', array_merge( ['bible_id' => $bible_id, 'verify_segmentation' => 'true', 'verify_content' => 'true'], $this->params )); - echo "\nTesting: $path"; - $response = $this->withHeaders($this->params)->get($path); - $response->assertSuccessful(); + $combined_response = $this->withHeaders($this->params)->get($combined_no_verse_starts_path); + $this->assertNoVerseStartsAnywhere( + $combined_response, + 'verify_segmentation=true & verify_content=true without verse_starts' + ); + $this->assertTopLevelSegmentationTypePresentAndNoPerBookDuplicate( + $combined_response, + 'verify_segmentation=true & verify_content=true without verse_starts' + ); + // Scenario 3: verse_starts=true alone — implicit-enable contract. The response shape must match + // Scenario 1: per-book filesets present (verify_content implicitly on), top-level filesets carry + // segmentation_type (verify_segmentation implicitly on), and verse_starts is attached to qualifying + // audio filesets under books[].filesets[]. Per-book entries themselves carry only {id, type, verse_starts?}. + \Illuminate\Support\Facades\Cache::store('array')->flush(); + $verse_starts_only_path = route('v4_bible.one', array_merge( + ['bible_id' => $bible_id, 'verse_starts' => 'true'], + $this->params + )); + $qualifying_implicit = $this->assertVerseStartsPresentOnQualifyingFilesets( + $this->withHeaders($this->params)->get($verse_starts_only_path), + $audio_types, + 'verse_starts=true alone (implicit-enable)' + ); + $this->assertGreaterThan( + 0, + $qualifying_implicit, + 'verse_starts=true alone must implicitly enable verify_content and produce qualifying per-book filesets' + ); + + // verify_segmentation alone: verse_starts must be absent everywhere. + \Illuminate\Support\Facades\Cache::store('array')->flush(); + $seg_only_path = route('v4_bible.one', array_merge( + ['bible_id' => $bible_id, 'verify_segmentation' => 'true'], + $this->params + )); + $this->assertNoVerseStartsAnywhere( + $this->withHeaders($this->params)->get($seg_only_path), + 'verify_segmentation=true alone' + ); + + // verify_content alone: verse_starts must be absent everywhere. + \Illuminate\Support\Facades\Cache::store('array')->flush(); + $content_only_path = route('v4_bible.one', array_merge( + ['bible_id' => $bible_id, 'verify_content' => 'true'], + $this->params + )); + $this->assertNoVerseStartsAnywhere( + $this->withHeaders($this->params)->get($content_only_path), + 'verify_content=true alone' + ); + + // Default request: verse_starts must be absent everywhere. + \Illuminate\Support\Facades\Cache::store('array')->flush(); + $default_path = route('v4_bible.one', array_merge(['bible_id' => $bible_id], $this->params)); + $this->assertNoVerseStartsAnywhere( + $this->withHeaders($this->params)->get($default_path), + 'default request' + ); + } + + private function assertVerseStartsPresentOnQualifyingFilesets($response, array $audio_types, string $context) : int + { + $response->assertSuccessful(); $decoded = json_decode($response->getContent(), true); $payload = is_array($decoded) ? ($decoded['data'] ?? []) : []; - // Top-level filesets map must NOT include verse_starts. + // The top-level filesets map is the single authoritative source for fileset-level metadata. + // Build a fileset_id => segmentation_type lookup from it; also assert each top-level entry + // exposes segmentation_type (verify_segmentation is on, explicit or implicit) and never + // carries verse_starts. + $segmentation_by_id = []; foreach ($payload['filesets'] ?? [] as $asset_group) { foreach ($asset_group as $fileset) { + $this->assertArrayHasKey( + 'segmentation_type', + $fileset, + "[$context] top-level fileset {$fileset['id']} must include segmentation_type (verify_segmentation is on, explicit or implicit)" + ); $this->assertArrayNotHasKey( 'verse_starts', $fileset, - "Top-level fileset {$fileset['id']} must not include verse_starts" + "[$context] top-level fileset {$fileset['id']} must not include verse_starts" ); + $segmentation_by_id[$fileset['id']] = $fileset['segmentation_type']; } } $qualifying_book_filesets = 0; foreach ($payload['books'] ?? [] as $book) { foreach ($book['filesets'] ?? [] as $book_fileset) { - $is_qualifying = ($book_fileset['segmentation_type'] ?? null) === 'section' + // Per-book entries must NOT duplicate fileset-level metadata. + $this->assertArrayNotHasKey( + 'segmentation_type', + $book_fileset, + "[$context] per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must not include segmentation_type — it is exposed only on the top-level filesets map" + ); + + $segmentation_type = $segmentation_by_id[$book_fileset['id']] ?? null; + $is_qualifying = $segmentation_type === 'section' && in_array($book_fileset['type'] ?? null, $audio_types, true); if ($is_qualifying) { $this->assertArrayHasKey( 'verse_starts', $book_fileset, - "Qualifying per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must include verse_starts" + "[$context] qualifying per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must include verse_starts" ); $this->assertIsArray($book_fileset['verse_starts']); $this->assertNotEmpty( $book_fileset['verse_starts'], - "verse_starts must contain at least one entry for fileset {$book_fileset['id']} (book {$book['book_id']})" + "[$context] verse_starts must contain at least one entry for fileset {$book_fileset['id']} (book {$book['book_id']})" ); foreach ($book_fileset['verse_starts'] as $entry) { $this->assertArrayHasKey('chapter_start', $entry); @@ -473,7 +571,7 @@ public function bibleOneWithVerseStarts() $this->assertArrayNotHasKey( 'book_id', $entry, - "verse_starts entry must not carry book_id (implied by the parent book)" + "[$context] verse_starts entry must not carry book_id (implied by the parent book)" ); } $qualifying_book_filesets++; @@ -481,44 +579,42 @@ public function bibleOneWithVerseStarts() $this->assertArrayNotHasKey( 'verse_starts', $book_fileset, - "Non-qualifying per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must not include verse_starts" + "[$context] non-qualifying per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must not include verse_starts" ); } } } - $this->assertGreaterThan(0, $qualifying_book_filesets, 'Expected at least one qualifying per-book fileset entry'); + return $qualifying_book_filesets; + } - // verify_segmentation alone: verse_starts must be absent everywhere. - // Cache is flushed between scenarios so each request gets a fresh Bible model - // (mimics production Memcached, where each request deserializes a fresh copy). - \Illuminate\Support\Facades\Cache::store('array')->flush(); - $seg_only_path = route('v4_bible.one', array_merge( - ['bible_id' => $bible_id, 'verify_segmentation' => 'true'], - $this->params - )); - $this->assertNoVerseStartsAnywhere( - $this->withHeaders($this->params)->get($seg_only_path), - 'verify_segmentation=true alone' - ); + private function assertTopLevelSegmentationTypePresentAndNoPerBookDuplicate($response, string $context) : void + { + $response->assertSuccessful(); + $decoded = json_decode($response->getContent(), true); + $payload = is_array($decoded) ? ($decoded['data'] ?? []) : []; - // verify_content alone: verse_starts must be absent everywhere. - \Illuminate\Support\Facades\Cache::store('array')->flush(); - $content_only_path = route('v4_bible.one', array_merge( - ['bible_id' => $bible_id, 'verify_content' => 'true'], - $this->params - )); - $this->assertNoVerseStartsAnywhere( - $this->withHeaders($this->params)->get($content_only_path), - 'verify_content=true alone' - ); + $top_level_checked = 0; + foreach ($payload['filesets'] ?? [] as $asset_group) { + foreach ($asset_group as $fileset) { + $this->assertArrayHasKey( + 'segmentation_type', + $fileset, + "[$context] top-level fileset {$fileset['id']} must include segmentation_type" + ); + $top_level_checked++; + } + } + $this->assertGreaterThan(0, $top_level_checked, "[$context] expected at least one top-level fileset to inspect"); - // Default request: verse_starts must be absent everywhere. - \Illuminate\Support\Facades\Cache::store('array')->flush(); - $default_path = route('v4_bible.one', array_merge(['bible_id' => $bible_id], $this->params)); - $this->assertNoVerseStartsAnywhere( - $this->withHeaders($this->params)->get($default_path), - 'default request' - ); + foreach ($payload['books'] ?? [] as $book) { + foreach ($book['filesets'] ?? [] as $book_fileset) { + $this->assertArrayNotHasKey( + 'segmentation_type', + $book_fileset, + "[$context] per-book fileset {$book_fileset['id']} (book {$book['book_id']}) must not include segmentation_type — it is exposed only on the top-level filesets map" + ); + } + } } private function assertNoVerseStartsAnywhere($response, string $context) : void From 6a123914c2538bb56e24b7f0570697467a9d1333 Mon Sep 17 00:00:00 2001 From: Victor Gonzalez Date: Wed, 15 Apr 2026 10:23:42 -0500 Subject: [PATCH 7/8] Task-89367 IP Whitelist to Bypass API Rate Limiting (cherry picked from commit 3ff41f9ac19a4bf1ef15071e7193eca8aa775218) --- .env.example | 4 + app/Http/Kernel.php | 2 +- .../ThrottleRequestsWithWhitelist.php | 56 +++++++++ config/app.php | 13 ++ tests/Feature/ThrottleWhitelistTest.php | 115 ++++++++++++++++++ 5 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 app/Http/Middleware/ThrottleRequestsWithWhitelist.php create mode 100644 tests/Feature/ThrottleWhitelistTest.php diff --git a/.env.example b/.env.example index e777869b4..16c818748 100644 --- a/.env.example +++ b/.env.example @@ -13,6 +13,10 @@ GET_STARTED_URL= NODE_PATH=/usr/local LOG_CHANNEL=stack +# Rate Limiting +# ----------------------------------- +IP_TRUSTED_NO_RATE_LIMIT= + # Database connections # ----------------------------------- diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 507791311..772bba05d 100755 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -73,7 +73,7 @@ class Kernel extends HttpKernel 'can' => \Illuminate\Auth\Middleware\Authorize::class, 'guest' => \App\Http\Middleware\RedirectIfAuthenticated::class, 'signed' => \Illuminate\Routing\Middleware\ValidateSignature::class, - 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, + 'throttle' => \App\Http\Middleware\ThrottleRequestsWithWhitelist::class, 'verified' => \Illuminate\Auth\Middleware\EnsureEmailIsVerified::class, 'APIToken' => \App\Http\Middleware\APIToken::class, 'AccessControl' => \App\Http\Middleware\AccessControl::class, diff --git a/app/Http/Middleware/ThrottleRequestsWithWhitelist.php b/app/Http/Middleware/ThrottleRequestsWithWhitelist.php new file mode 100644 index 000000000..b27c0ddc1 --- /dev/null +++ b/app/Http/Middleware/ThrottleRequestsWithWhitelist.php @@ -0,0 +1,56 @@ +ip() to prevent + // X-Forwarded-For spoofing when TrustProxies is set to '*'. + $peerIp = $request->server('REMOTE_ADDR'); + + if ($this->isIpWhitelisted($peerIp)) { + return $next($request); + } + + return parent::handle($request, $next, $maxAttempts, $decayMinutes, $prefix); + } + + /** + * Check if the given IP address is in the trusted no-rate-limit whitelist. + */ + private function isIpWhitelisted(?string $ip): bool + { + if (empty($ip)) { + return false; + } + + $trusted_ips = config('app.ip_trusted_no_rate_limit'); + + if ($trusted_ips === null || trim((string) $trusted_ips) === '') { + return false; + } + + $allowed_ips = array_map('trim', explode(',', $trusted_ips)); + + return in_array($ip, $allowed_ips, true); + } +} diff --git a/config/app.php b/config/app.php index c3ae22277..faa2877eb 100755 --- a/config/app.php +++ b/config/app.php @@ -112,6 +112,19 @@ 'cipher' => 'AES-256-CBC', + /* + |-------------------------------------------------------------------------- + | Rate Limiting IP Whitelist + |-------------------------------------------------------------------------- + | + | Comma-separated list of trusted IP addresses that bypass API rate + | limiting. Used for proxy servers (e.g. live.bible.is) that already + | have rate limiting enforced at the infrastructure level. + | + */ + + 'ip_trusted_no_rate_limit' => env('IP_TRUSTED_NO_RATE_LIMIT', ''), + /* |-------------------------------------------------------------------------- | Autoloaded Service Providers diff --git a/tests/Feature/ThrottleWhitelistTest.php b/tests/Feature/ThrottleWhitelistTest.php new file mode 100644 index 000000000..c0cbe1fb3 --- /dev/null +++ b/tests/Feature/ThrottleWhitelistTest.php @@ -0,0 +1,115 @@ +get(self::TEST_ROUTE, function () { + return response()->json(['status' => 'ok']); + }); + } + + /** + * @group throttle_whitelist + * @test + */ + public function whitelisted_ip_bypasses_rate_limit() + { + config(['app.ip_trusted_no_rate_limit' => self::TRUSTED_IP_1]); + + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => self::TRUSTED_IP_1, + ]); + + $response->assertStatus(200); + $this->assertFalse( + $response->headers->has('X-RateLimit-Limit'), + 'Whitelisted IP should not have rate limit headers' + ); + } + + /** + * @group throttle_whitelist + * @test + */ + public function non_whitelisted_ip_is_rate_limited() + { + config(['app.ip_trusted_no_rate_limit' => self::TRUSTED_IP_1]); + + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => self::UNTRUSTED_IP, + ]); + + $response->assertStatus(200); + $this->assertTrue( + $response->headers->has('X-RateLimit-Limit'), + 'Non-whitelisted IP should have rate limit headers' + ); + } + + /** + * @group throttle_whitelist + * @test + */ + public function empty_whitelist_applies_rate_limiting_to_all() + { + config(['app.ip_trusted_no_rate_limit' => '']); + + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => self::TRUSTED_IP_1, + ]); + + $response->assertStatus(200); + $this->assertTrue( + $response->headers->has('X-RateLimit-Limit'), + 'Empty whitelist should rate limit all IPs' + ); + } + + /** + * @group throttle_whitelist + * @test + */ + public function multiple_ips_in_whitelist() + { + $whitelist = implode(', ', [self::TRUSTED_IP_1, self::TRUSTED_IP_2, self::TRUSTED_IP_3]); + config(['app.ip_trusted_no_rate_limit' => $whitelist]); + + foreach ([self::TRUSTED_IP_1, self::TRUSTED_IP_2, self::TRUSTED_IP_3] as $ip) { + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => $ip, + ]); + + $response->assertStatus(200); + $this->assertFalse( + $response->headers->has('X-RateLimit-Limit'), + "Whitelisted IP {$ip} should not have rate limit headers" + ); + } + + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => self::UNTRUSTED_IP, + ]); + + $response->assertStatus(200); + $this->assertTrue( + $response->headers->has('X-RateLimit-Limit'), + 'IP not in whitelist should have rate limit headers' + ); + } +} From a48038548338c98cce45ee098587ba6469365146 Mon Sep 17 00:00:00 2001 From: Victor Gonzalez Date: Wed, 3 Jun 2026 21:32:43 -0500 Subject: [PATCH 8/8] Task-92918 Fix the rate-limit whitelist so live.bible.is proxies bypass throttling behind the AWS ALB. (cherry picked from commit 959bd59e6024b75e3ce2831fb465da082da44c64) --- .../ThrottleRequestsWithWhitelist.php | 40 +++++++- tests/Feature/ThrottleWhitelistTest.php | 99 +++++++++++++++++++ 2 files changed, 136 insertions(+), 3 deletions(-) diff --git a/app/Http/Middleware/ThrottleRequestsWithWhitelist.php b/app/Http/Middleware/ThrottleRequestsWithWhitelist.php index b27c0ddc1..60dd234a9 100644 --- a/app/Http/Middleware/ThrottleRequestsWithWhitelist.php +++ b/app/Http/Middleware/ThrottleRequestsWithWhitelist.php @@ -3,6 +3,7 @@ namespace App\Http\Middleware; use Closure; +use Illuminate\Http\Request; use Illuminate\Routing\Middleware\ThrottleRequests; class ThrottleRequestsWithWhitelist extends ThrottleRequests @@ -23,9 +24,7 @@ class ThrottleRequestsWithWhitelist extends ThrottleRequests */ public function handle($request, Closure $next, $maxAttempts = 60, $decayMinutes = 1, $prefix = '') { - // Use REMOTE_ADDR (actual peer IP) instead of $request->ip() to prevent - // X-Forwarded-For spoofing when TrustProxies is set to '*'. - $peerIp = $request->server('REMOTE_ADDR'); + $peerIp = $this->resolvePeerIp($request); if ($this->isIpWhitelisted($peerIp)) { return $next($request); @@ -34,8 +33,43 @@ public function handle($request, Closure $next, $maxAttempts = 60, $decayMinutes return parent::handle($request, $next, $maxAttempts, $decayMinutes, $prefix); } + /** + * Resolve the client IP used for whitelist matching. + * + * The API runs behind an AWS Application Load Balancer, so REMOTE_ADDR is the + * ALB's private IP, not the upstream (live.bible.is) proxy. The ALB appends the + * IP of the connection it received to the END of X-Forwarded-For, so the + * right-most entry is the address the ALB actually observed — a value a client + * cannot forge (anything a client puts in X-Forwarded-For is pushed left when the + * ALB appends the real source). We deliberately use this right-most entry instead + * of $request->ip(), which returns the spoofable left-most entry because + * TrustProxies is set to '*'. When X-Forwarded-For is absent (e.g. local/direct + * requests) we fall back to REMOTE_ADDR. + * + * @param \Illuminate\Http\Request $request The incoming HTTP request. + * @return string|null The resolved client IP address, or null if it cannot be determined. + */ + private function resolvePeerIp(Request $request): ?string + { + $forwarded = $request->server('HTTP_X_FORWARDED_FOR'); + + if (!empty($forwarded)) { + $entries = explode(',', $forwarded); + $edge_ip = trim(end($entries)); // right-most entry = appended by the AWS ALB + + if (filter_var($edge_ip, FILTER_VALIDATE_IP) !== false) { + return $edge_ip; + } + } + + return $request->server('REMOTE_ADDR'); + } + /** * Check if the given IP address is in the trusted no-rate-limit whitelist. + * + * @param string|null $ip The IP address to check. + * @return bool True if the IP is whitelisted, false otherwise. */ private function isIpWhitelisted(?string $ip): bool { diff --git a/tests/Feature/ThrottleWhitelistTest.php b/tests/Feature/ThrottleWhitelistTest.php index c0cbe1fb3..275176242 100644 --- a/tests/Feature/ThrottleWhitelistTest.php +++ b/tests/Feature/ThrottleWhitelistTest.php @@ -14,6 +14,11 @@ class ThrottleWhitelistTest extends TestCase private const TRUSTED_IP_2 = '192.0.2.2'; private const TRUSTED_IP_3 = '192.0.2.3'; private const UNTRUSTED_IP = '198.51.100.1'; + private const ATTACKER_IP = '203.0.113.99'; + + // Private ALB-like peer IP (REMOTE_ADDR) used to simulate requests arriving + // through the AWS load balancer. + private const ALB_REMOTE_ADDR = '10.0.1.50'; protected function setUp(): void { @@ -112,4 +117,98 @@ public function multiple_ips_in_whitelist() 'IP not in whitelist should have rate limit headers' ); } + + /** + * Behind the AWS ALB the upstream client may appear on the left and the ALB + * appends the real proxy IP (live.bible.is) on the right. TrustProxies='*' does + * NOT rewrite the raw HTTP_X_FORWARDED_FOR server var, so the middleware reads + * exactly what is injected here. + * + * @group throttle_whitelist + * @test + */ + public function whitelisted_proxy_in_rightmost_xff_bypasses_rate_limit() + { + config(['app.ip_trusted_no_rate_limit' => self::TRUSTED_IP_1]); + + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => self::ALB_REMOTE_ADDR, + 'HTTP_X_FORWARDED_FOR' => self::UNTRUSTED_IP . ', ' . self::TRUSTED_IP_1, + ]); + + $response->assertStatus(200); + $this->assertFalse( + $response->headers->has('X-RateLimit-Limit'), + 'Whitelisted proxy as right-most X-Forwarded-For entry should bypass rate limiting' + ); + } + + /** + * Common case: the proxy did not forward an upstream client, so the ALB appends + * only the proxy IP as the single X-Forwarded-For entry. + * + * @group throttle_whitelist + * @test + */ + public function whitelisted_proxy_as_single_xff_entry_bypasses_rate_limit() + { + config(['app.ip_trusted_no_rate_limit' => self::TRUSTED_IP_1]); + + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => self::ALB_REMOTE_ADDR, + 'HTTP_X_FORWARDED_FOR' => self::TRUSTED_IP_1, + ]); + + $response->assertStatus(200); + $this->assertFalse( + $response->headers->has('X-RateLimit-Limit'), + 'Whitelisted proxy as the only X-Forwarded-For entry should bypass rate limiting' + ); + } + + /** + * A client cannot bypass throttling by spoofing a whitelisted IP in + * X-Forwarded-For: the AWS ALB appends the real observed source on the right, + * so the left-most (client-controlled) value is never used for the match. + * + * @group throttle_whitelist + * @test + */ + public function spoofed_xff_does_not_bypass_rate_limit() + { + config(['app.ip_trusted_no_rate_limit' => self::TRUSTED_IP_1]); + + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => self::ALB_REMOTE_ADDR, + 'HTTP_X_FORWARDED_FOR' => self::TRUSTED_IP_1 . ', ' . self::ATTACKER_IP, + ]); + + $response->assertStatus(200); + $this->assertTrue( + $response->headers->has('X-RateLimit-Limit'), + 'Spoofed left-most X-Forwarded-For must NOT bypass rate limiting' + ); + } + + /** + * Without X-Forwarded-For (e.g. direct/local request) the whitelist matches + * against REMOTE_ADDR. + * + * @group throttle_whitelist + * @test + */ + public function falls_back_to_remote_addr_without_xff() + { + config(['app.ip_trusted_no_rate_limit' => self::TRUSTED_IP_1]); + + $response = $this->call('GET', self::TEST_ROUTE, [], [], [], [ + 'REMOTE_ADDR' => self::TRUSTED_IP_1, + ]); + + $response->assertStatus(200); + $this->assertFalse( + $response->headers->has('X-RateLimit-Limit'), + 'Without X-Forwarded-For the whitelist should match REMOTE_ADDR' + ); + } }