Applied another round of static analysis updates

This commit is contained in:
Dan Brown 2021-11-22 23:33:55 +00:00
parent 1bf59f434b
commit 024924eef3
No known key found for this signature in database
GPG key ID: 46D9F943C24A2EF9
21 changed files with 96 additions and 57 deletions

View file

@ -105,10 +105,10 @@ class ActivityService
$queryIds = [$entity->getMorphClass() => [$entity->id]];
if ($entity instanceof Book) {
$queryIds[(new Chapter())->getMorphClass()] = $entity->chapters()->visible()->pluck('id');
$queryIds[(new Chapter())->getMorphClass()] = $entity->chapters()->scopes('visible')->pluck('id');
}
if ($entity instanceof Book || $entity instanceof Chapter) {
$queryIds[(new Page())->getMorphClass()] = $entity->pages()->visible()->pluck('id');
$queryIds[(new Page())->getMorphClass()] = $entity->pages()->scopes('visible')->pluck('id');
}
$query = $this->activity->newQuery();

View file

@ -165,7 +165,7 @@ class LdapService
* Bind the system user to the LDAP connection using the given credentials
* otherwise anonymous access is attempted.
*
* @param $connection
* @param resource $connection
*
* @throws LdapException
*/

View file

@ -41,16 +41,18 @@ class OidcJwtSigningKey
protected function loadFromPath(string $path)
{
try {
$this->key = PublicKeyLoader::load(
$key = PublicKeyLoader::load(
file_get_contents($path)
)->withPadding(RSA::SIGNATURE_PKCS1);
);
} catch (\Exception $exception) {
throw new OidcInvalidKeyException("Failed to load key from file path with error: {$exception->getMessage()}");
}
if (!($this->key instanceof RSA)) {
if (!$key instanceof RSA) {
throw new OidcInvalidKeyException('Key loaded from file path is not an RSA key as expected');
}
$this->key = $key->withPadding(RSA::SIGNATURE_PKCS1);
}
/**
@ -81,14 +83,19 @@ class OidcJwtSigningKey
$n = strtr($jwk['n'] ?? '', '-_', '+/');
try {
/** @var RSA $key */
$this->key = PublicKeyLoader::load([
$key = PublicKeyLoader::load([
'e' => new BigInteger(base64_decode($jwk['e']), 256),
'n' => new BigInteger(base64_decode($n), 256),
])->withPadding(RSA::SIGNATURE_PKCS1);
]);
} catch (\Exception $exception) {
throw new OidcInvalidKeyException("Failed to load key from JWK parameters with error: {$exception->getMessage()}");
}
if (!$key instanceof RSA) {
throw new OidcInvalidKeyException('Key loaded from file path is not an RSA key as expected');
}
$this->key = $key->withPadding(RSA::SIGNATURE_PKCS1);
}
/**

View file

@ -12,6 +12,7 @@ use Illuminate\Support\Str;
use Laravel\Socialite\Contracts\Factory as Socialite;
use Laravel\Socialite\Contracts\Provider;
use Laravel\Socialite\Contracts\User as SocialUser;
use Laravel\Socialite\Two\GoogleProvider;
use SocialiteProviders\Manager\SocialiteWasCalled;
use Symfony\Component\HttpFoundation\RedirectResponse;
@ -278,7 +279,7 @@ class SocialAuthService
{
$driver = $this->socialite->driver($driverName);
if ($driverName === 'google' && config('services.google.select_account')) {
if ($driver instanceof GoogleProvider && config('services.google.select_account')) {
$driver->with(['prompt' => 'select_account']);
}

View file

@ -79,53 +79,43 @@ class Book extends Entity implements HasCoverImage
/**
* Get all pages within this book.
*
* @return HasMany
*/
public function pages()
public function pages(): HasMany
{
return $this->hasMany(Page::class);
}
/**
* Get the direct child pages of this book.
*
* @return HasMany
*/
public function directPages()
public function directPages(): HasMany
{
return $this->pages()->where('chapter_id', '=', '0');
}
/**
* Get all chapters within this book.
*
* @return HasMany
*/
public function chapters()
public function chapters(): HasMany
{
return $this->hasMany(Chapter::class);
}
/**
* Get the shelves this book is contained within.
*
* @return BelongsToMany
*/
public function shelves()
public function shelves(): BelongsToMany
{
return $this->belongsToMany(Bookshelf::class, 'bookshelves_books', 'book_id', 'bookshelf_id');
}
/**
* Get the direct child items within this book.
*
* @return Collection
*/
public function getDirectChildren(): Collection
{
$pages = $this->directPages()->visible()->get();
$chapters = $this->chapters()->visible()->get();
$pages = $this->directPages()->scopes('visible')->get();
$chapters = $this->chapters()->scopes('visible')->get();
return $pages->concat($chapters)->sortBy('priority')->sortByDesc('draft');
}

View file

@ -37,7 +37,7 @@ class Bookshelf extends Entity implements HasCoverImage
*/
public function visibleBooks(): BelongsToMany
{
return $this->books()->visible();
return $this->books()->scopes('visible');
}
/**

View file

@ -23,6 +23,7 @@ class Chapter extends BookChild
/**
* Get the pages that this chapter contains.
* @return HasMany<Page>
*/
public function pages(string $dir = 'ASC'): HasMany
{
@ -50,7 +51,8 @@ class Chapter extends BookChild
*/
public function getVisiblePages(): Collection
{
return $this->pages()->visible()
return $this->pages()
->scopes('visible')
->orderBy('draft', 'desc')
->orderBy('priority', 'asc')
->get();

View file

@ -121,11 +121,11 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable
return true;
}
if (($entity->isA('chapter') || $entity->isA('page')) && $this->isA('book')) {
if (($entity instanceof BookChild) && $this instanceof Book) {
return $entity->book_id === $this->id;
}
if ($entity->isA('page') && $this->isA('chapter')) {
if ($entity instanceof Page && $this instanceof Chapter) {
return $entity->chapter_id === $this->id;
}

View file

@ -63,10 +63,8 @@ class PageRevision extends Model
/**
* Get the previous revision for the same page if existing.
*
* @return \BookStack\Entities\PageRevision|null
*/
public function getPrevious()
public function getPrevious(): ?PageRevision
{
$id = static::newQuery()->where('page_id', '=', $this->page_id)
->where('id', '<', $this->id)

View file

@ -69,9 +69,10 @@ class PageRepo
*/
public function getByOldSlug(string $bookSlug, string $pageSlug): ?Page
{
/** @var ?PageRevision $revision */
$revision = PageRevision::query()
->whereHas('page', function (Builder $query) {
$query->visible();
$query->scopes('visible');
})
->where('slug', '=', $pageSlug)
->where('type', '=', 'version')
@ -80,7 +81,7 @@ class PageRepo
->with('page')
->first();
return $revision ? $revision->page : null;
return $revision->page ?? null;
}
/**

View file

@ -12,6 +12,8 @@ use BookStack\Uploads\ImageRepo;
use BookStack\Uploads\ImageService;
use BookStack\Util\HtmlContentFilter;
use DOMDocument;
use DOMElement;
use DOMNode;
use DOMNodeList;
use DOMXPath;
use Illuminate\Support\Str;
@ -237,9 +239,9 @@ class PageContent
* A map for existing ID's should be passed in to check for current existence.
* Returns a pair of strings in the format [old_id, new_id].
*/
protected function setUniqueId(\DOMNode $element, array &$idMap): array
protected function setUniqueId(DOMNode $element, array &$idMap): array
{
if (!$element instanceof \DOMElement) {
if (!$element instanceof DOMElement) {
return ['', ''];
}
@ -321,7 +323,7 @@ class PageContent
*/
protected function headerNodesToLevelList(DOMNodeList $nodeList): array
{
$tree = collect($nodeList)->map(function ($header) {
$tree = collect($nodeList)->map(function (DOMElement $header) {
$text = trim(str_replace("\xc2\xa0", '', $header->nodeValue));
$text = mb_substr($text, 0, 100);

View file

@ -9,6 +9,7 @@ use BookStack\Entities\Models\Page;
use BookStack\Entities\Models\SearchTerm;
use DOMDocument;
use DOMNode;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Support\Collection;
class SearchIndex
@ -76,7 +77,9 @@ class SearchIndex
foreach ($this->entityProvider->all() as $entityModel) {
$indexContentField = $entityModel instanceof Page ? 'html' : 'description';
$selectFields = ['id', 'name', $indexContentField];
$total = $entityModel->newQuery()->withTrashed()->count();
/** @var Builder<Entity> $query */
$query = $entityModel->newQuery();
$total = $query->withTrashed()->count();
$chunkSize = 250;
$processed = 0;
@ -223,7 +226,7 @@ class SearchIndex
if ($entity instanceof Page) {
$bodyTermsMap = $this->generateTermScoreMapFromHtml($entity->html);
} else {
$bodyTermsMap = $this->generateTermScoreMapFromText($entity->description ?? '', $entity->searchFactor);
$bodyTermsMap = $this->generateTermScoreMapFromText($entity->getAttribute('description') ?? '', $entity->searchFactor);
}
$mergedScoreMap = $this->mergeTermScoreMaps($nameTermsMap, $bodyTermsMap, $tagTermsMap);

View file

@ -145,13 +145,13 @@ class SearchRunner
if ($entityModelInstance instanceof BookChild) {
$relations['book'] = function (BelongsTo $query) {
$query->visible();
$query->scopes('visible');
};
}
if ($entityModelInstance instanceof Page) {
$relations['chapter'] = function (BelongsTo $query) {
$query->visible();
$query->scopes('visible');
};
}

View file

@ -15,6 +15,7 @@ use BookStack\Facades\Activity;
use BookStack\Uploads\AttachmentService;
use BookStack\Uploads\ImageService;
use Exception;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Support\Carbon;
class TrashCan
@ -141,11 +142,9 @@ class TrashCan
{
$count = 0;
$pages = $chapter->pages()->withTrashed()->get();
if (count($pages)) {
foreach ($pages as $page) {
$this->destroyPage($page);
$count++;
}
foreach ($pages as $page) {
$this->destroyPage($page);
$count++;
}
$this->destroyCommonRelations($chapter);
@ -183,9 +182,10 @@ class TrashCan
{
$counts = [];
/** @var Entity $instance */
foreach ((new EntityProvider())->all() as $key => $instance) {
$counts[$key] = $instance->newQuery()->onlyTrashed()->count();
/** @var Builder<Entity> $query */
$query = $instance->newQuery();
$counts[$key] = $query->onlyTrashed()->count();
}
return $counts;
@ -344,9 +344,9 @@ class TrashCan
$entity->deletions()->delete();
$entity->favourites()->delete();
if ($entity instanceof HasCoverImage && $entity->cover) {
if ($entity instanceof HasCoverImage && $entity->cover()->exists()) {
$imageService = app()->make(ImageService::class);
$imageService->destroy($entity->cover);
$imageService->destroy($entity->cover()->first());
}
}
}

View file

@ -75,7 +75,7 @@ class BookshelfApiController extends ApiController
$shelf = Bookshelf::visible()->with([
'tags', 'cover', 'createdBy', 'updatedBy', 'ownedBy',
'books' => function (BelongsToMany $query) {
$query->visible()->get(['id', 'name', 'slug']);
$query->scopes('visible')->get(['id', 'name', 'slug']);
},
])->findOrFail($id);

View file

@ -70,7 +70,7 @@ class ChapterApiController extends ApiController
public function read(string $id)
{
$chapter = Chapter::visible()->with(['tags', 'createdBy', 'updatedBy', 'ownedBy', 'pages' => function (HasMany $query) {
$query->visible()->get(['id', 'name', 'slug']);
$query->scopes('visible')->get(['id', 'name', 'slug']);
}])->findOrFail($id);
return response()->json($chapter);

View file

@ -114,7 +114,7 @@ class BookController extends Controller
{
$book = $this->bookRepo->getBySlug($slug);
$bookChildren = (new BookContents($book))->getTree(true);
$bookParentShelves = $book->shelves()->visible()->get();
$bookParentShelves = $book->shelves()->scopes('visible')->get();
View::incrementFor($book);
if ($request->has('shelf')) {

View file

@ -52,7 +52,7 @@ class ThemeService
*/
public function registerCommand(Command $command)
{
Artisan::starting(function(Application $application) use ($command) {
Artisan::starting(function (Application $application) use ($command) {
$application->addCommands([$command]);
});
}

View file

@ -103,7 +103,10 @@ class ImageRepo
if ($filterType === 'page') {
$query->where('uploaded_to', '=', $contextPage->id);
} elseif ($filterType === 'book') {
$validPageIds = $contextPage->book->pages()->visible()->pluck('id')->toArray();
$validPageIds = $contextPage->book->pages()
->scopes('visible')
->pluck('id')
->toArray();
$query->whereIn('uploaded_to', $validPageIds);
}
};

View file

@ -0,0 +1,32 @@
<?php
namespace Tests\Unit;
use BadMethodCallException;
use BookStack\Entities\Models\Page;
use Tests\TestCase;
/**
* This class tests assumptions we're relying upon in the framework.
* This is primarily to keep track of certain bits of functionality that
* may be used in important areas such as to enforce permissions.
*/
class FrameworkAssumptionTest extends TestCase
{
public function test_scopes_error_if_not_existing()
{
$this->expectException(BadMethodCallException::class);
$this->expectExceptionMessage('Call to undefined method BookStack\Entities\Models\Page::scopeNotfoundscope()');
Page::query()->scopes('notfoundscope');
}
public function test_scopes_applies_upon_existing()
{
// Page has SoftDeletes trait by default, so we apply our custom scope and ensure
// it stacks on the global scope to filter out deleted items.
$query = Page::query()->scopes('visible')->toSql();
$this->assertStringContainsString('joint_permissions', $query);
$this->assertStringContainsString('`deleted_at` is null', $query);
}
}

View file

@ -1 +1 @@
v21.10-dev
v21.11-dev