Skip to content

Commit

Permalink
Ensure getUrl is callable (#391)
Browse files Browse the repository at this point in the history
  • Loading branch information
frankdekker authored Aug 23, 2023
1 parent 0de2046 commit 9a4d665
Show file tree
Hide file tree
Showing 11 changed files with 43 additions and 72 deletions.
5 changes: 5 additions & 0 deletions src/Entity/Repository/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ public function setMainBranchName(string $mainBranchName): Repository
return $this;
}

public function hasUrl(): bool
{
return isset($this->url);
}

public function getUrl(): UriInterface
{
return $this->url;
Expand Down
2 changes: 1 addition & 1 deletion src/Form/Repository/RepositoryUrlType.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public function configureOptions(OptionsResolver $resolver): void
[
'setter' => static function (Repository $repository, Uri $uri): void {
// if no new password, transfer existing password to the new uri
[, $password] = UriUtil::credentials($repository->getUrl());
[, $password] = UriUtil::credentials($repository->hasUrl() ? $repository->getUrl() : null);
[$username, $newPassword] = UriUtil::credentials($uri);
$repository->setUrl($uri->withUserInfo($username, $newPassword ?? $password));
}
Expand Down
58 changes: 12 additions & 46 deletions tests/Unit/Entity/Repository/RepositoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,39 +8,12 @@
use DR\Review\Entity\Repository\Repository;
use DR\Review\Entity\Repository\RepositoryProperty;
use DR\Review\Tests\AbstractTestCase;
use League\Uri\Uri;
use PHPUnit\Framework\Attributes\CoversClass;

/**
* @coversDefaultClass \DR\Review\Entity\Repository\Repository
* @covers ::__construct
*/
#[CoversClass(Repository::class)]
class RepositoryTest extends AbstractTestCase
{
/**
* @covers ::setId
* @covers ::getId
* @covers ::isActive
* @covers ::setActive
* @covers ::getName
* @covers ::setName
* @covers ::getDisplayName
* @covers ::setDisplayName
* @covers ::getMainBranchName
* @covers ::setMainBranchName
* @covers ::getUrl
* @covers ::setUrl
* @covers ::isFavorite
* @covers ::setFavorite
* @covers ::getUpdateRevisionsInterval
* @covers ::setUpdateRevisionsInterval
* @covers ::getUpdateRevisionsTimestamp
* @covers ::setUpdateRevisionsTimestamp
* @covers ::getValidateRevisionsInterval
* @covers ::setValidateRevisionsInterval
* @covers ::getValidateRevisionsTimestamp
* @covers ::setValidateRevisionsTimestamp
* @covers ::getCreateTimestamp
* @covers ::setCreateTimestamp
*/
public function testAccessorPairs(): void
{
$config = new ConstraintConfig();
Expand All @@ -50,9 +23,15 @@ public function testAccessorPairs(): void
static::assertAccessorPairs(Repository::class, $config);
}

/**
* @covers ::getRepositoryProperty
*/
public function testUrlAccessor(): void
{
$repository = new Repository();
static::assertFalse($repository->hasUrl());

$repository->setUrl(Uri::new('https://example.com'));
static::assertSame('https://example.com', (string)$repository->getUrl());
}

public function testGetRepositoryProperty(): void
{
$repository = new Repository();
Expand All @@ -62,11 +41,6 @@ public function testGetRepositoryProperty(): void
static::assertSame('value', $repository->getRepositoryProperty('property'));
}

/**
* @covers ::setRepositoryProperty
* @covers ::getRepositoryProperties
* @covers ::removeRepositoryProperty
*/
public function testRepositoryPropertyAccessors(): void
{
$propertyA = new RepositoryProperty('property', '5');
Expand All @@ -84,10 +58,6 @@ public function testRepositoryPropertyAccessors(): void
static::assertCount(0, $repository->getRepositoryProperties());
}

/**
* @covers ::getRevisions
* @covers ::setRevisions
*/
public function testRevisions(): void
{
$collection = new ArrayCollection();
Expand All @@ -99,10 +69,6 @@ public function testRevisions(): void
static::assertSame($collection, $repository->getRevisions());
}

/**
* @covers ::getReviews
* @covers ::setReviews
*/
public function testReviews(): void
{
$collection = new ArrayCollection();
Expand Down
4 changes: 2 additions & 2 deletions tests/Unit/Form/Repository/RepositoryUrlTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ public function testConfigureOptions(): void

// test callback
$repository = new Repository();
$repository->setUrl(Uri::createFromString('https://sherlock:holmes@example.com'));
$uri = Uri::createFromString('https://watson@example.com');
$repository->setUrl(Uri::new('https://sherlock:holmes@example.com'));
$uri = Uri::new('https://watson@example.com');
$callback($repository, $uri);

static::assertSame('https://watson:holmes@example.com', (string)$repository->getUrl());
Expand Down
6 changes: 3 additions & 3 deletions tests/Unit/Service/Git/Add/GitAddServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
class GitAddServiceTest extends AbstractTestCase
{
private CacheableGitRepositoryService&MockObject $repositoryService;
private GitCommandBuilderFactory&MockObject $builderFactory;
private GitAddService $service;
private GitCommandBuilderFactory&MockObject $builderFactory;
private GitAddService $service;

public function setUp(): void
{
Expand All @@ -39,7 +39,7 @@ public function setUp(): void
public function testAdd(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('https://url/'));
$repository->setUrl(Uri::new('https://url/'));
$path = '/foo/bar/';

$builder = $this->createMock(GitAddCommandBuilder::class);
Expand Down
6 changes: 3 additions & 3 deletions tests/Unit/Service/Git/Branch/GitBranchServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public function testGetRemoteBranchesMergedOnly(): void
public function testDeleteBranch(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('https://url/'));
$repository->setUrl(Uri::new('https://url/'));
$path = '/foo/bar/';

$builder = $this->createMock(GitBranchCommandBuilder::class);
Expand All @@ -106,7 +106,7 @@ public function testDeleteBranch(): void
public function testTryDeleteBranch(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('https://url/'));
$repository->setUrl(Uri::new('https://url/'));
$path = '/foo/bar/';

$builder = $this->createMock(GitBranchCommandBuilder::class);
Expand All @@ -126,7 +126,7 @@ public function testTryDeleteBranch(): void
public function testTryDeleteBranchCaptureException(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('https://url/'));
$repository->setUrl(Uri::new('https://url/'));
$path = '/foo/bar/';

$this->builderFactory->expects(self::once())->method('createBranch')->willThrowException(new RepositoryException());
Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Service/Git/Commit/GitCommitServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function testCommit(): void
$message = 'foobar';

$repository = new Repository();
$repository->setUrl(Uri::createFromString('https://url/'));
$repository->setUrl(Uri::new('https://url/'));

$builder = $this->createMock(GitCommitCommandBuilder::class);
$builder->expects(self::once())->method('message')->with($message)->willReturnSelf();
Expand Down
12 changes: 6 additions & 6 deletions tests/Unit/Service/Git/Diff/GitDiffServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public function testGetBundledDiffShouldSkipOnSingleHash(): void
public function testGetDiffFromRevision(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('http://foobar.com'));
$repository->setUrl(Uri::new('http://foobar.com'));
$revision = new Revision();
$revision->setRepository($repository);
$revision->setCommitHash('commit-hash');
Expand Down Expand Up @@ -128,7 +128,7 @@ public function testGetDiffFromRevision(): void
public function testGetDiffFromRevisionWithIgnoreSpace(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('http://foobar.com'));
$repository->setUrl(Uri::new('http://foobar.com'));
$revision = new Revision();
$revision->setRepository($repository);
$revision->setCommitHash('commit-hash');
Expand Down Expand Up @@ -156,7 +156,7 @@ public function testGetDiffFromRevisionWithIgnoreSpace(): void
public function testGetBundledDiffFromRevisions(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('http://foobar.com'));
$repository->setUrl(Uri::new('http://foobar.com'));

$builder = $this->createMock(GitDiffCommandBuilder::class);
$builder->expects(self::once())->method('hash')->with('HEAD')->willReturnSelf();
Expand All @@ -182,7 +182,7 @@ public function testGetBundledDiffFromRevisions(): void
public function testGetBundledDiffFromRevisionsIgnoreAllSpaceChange(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('http://foobar.com'));
$repository->setUrl(Uri::new('http://foobar.com'));

$builder = $this->createMock(GitDiffCommandBuilder::class);
$builder->expects(self::once())->method('hash')->with('HEAD')->willReturnSelf();
Expand All @@ -208,7 +208,7 @@ public function testGetBundledDiffFromRevisionsIgnoreAllSpaceChange(): void
public function testGetBundledDiffFromBranch(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('http://foobar.com'));
$repository->setUrl(Uri::new('http://foobar.com'));

$builder = $this->createMock(GitDiffCommandBuilder::class);
$builder->expects(self::once())->method('hash')->with('target...source')->willReturnSelf();
Expand All @@ -234,7 +234,7 @@ public function testGetBundledDiffFromBranch(): void
public function testGetBundledDiffFromBranchIgnoreAllSpaceChange(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('http://foobar.com'));
$repository->setUrl(Uri::new('http://foobar.com'));

$builder = $this->createMock(GitDiffCommandBuilder::class);
$builder->expects(self::once())->method('hash')->with('target...source')->willReturnSelf();
Expand Down
8 changes: 4 additions & 4 deletions tests/Unit/Service/Git/GitRepositoryServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public function testGetRepositoryWithoutCache(): void
{
$repository = new Repository();
$repository->setId(123);
$repository->setUrl(Uri::createFromString('http://my.repository.com'));
$repository->setUrl(Uri::new('http://my.repository.com'));
$gitRepository = $this->createMock(GitRepository::class);

// setup mocks
Expand All @@ -72,7 +72,7 @@ public function testGetRepositoryWithCache(): void
{
$repository = new Repository();
$repository->setId(123);
$repository->setUrl(Uri::createFromString('http://my.repository.com'));
$repository->setUrl(Uri::new('http://my.repository.com'));

// setup mocks
$this->filesystem->expects(static::once())->method('mkdir')->with(self::CACHE_DIRECTORY);
Expand All @@ -91,7 +91,7 @@ public function testGetRepositoryWithGitException(): void
{
$repository = new Repository();
$repository->setId(123);
$repository->setUrl(Uri::createFromString('http://my.repository.com'));
$repository->setUrl(Uri::new('http://my.repository.com'));
$runnerResult = new RunnerResult('git', 1, ['output'], ['failure']);

// setup mocks
Expand All @@ -113,7 +113,7 @@ public function testGetRepositoryWithException(): void
{
$repository = new Repository();
$repository->setId(123);
$repository->setUrl(Uri::createFromString('http://my.repository.com'));
$repository->setUrl(Uri::new('http://my.repository.com'));

// setup mocks
$this->filesystem->expects(static::exactly(5))->method('mkdir')->with(self::CACHE_DIRECTORY);
Expand Down
4 changes: 2 additions & 2 deletions tests/Unit/Service/Git/Reset/GitResetServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function setUp(): void
public function testResetHard(): void
{
$repository = new Repository();
$repository->setUrl(Uri::createFromString('https://example.com'));
$repository->setUrl(Uri::new('https://example.com'));

$builder = $this->createMock(GitResetCommandBuilder::class);
$builder->expects(self::once())->method('hard')->willReturnSelf();
Expand All @@ -57,7 +57,7 @@ public function testResetSoft(): void
$commitHash = '123abc';

$repository = new Repository();
$repository->setUrl(Uri::createFromString('https://example.com'));
$repository->setUrl(Uri::new('https://example.com'));

$builder = $this->createMock(GitResetCommandBuilder::class);
$builder->expects(self::once())->method('soft')->willReturnSelf();
Expand Down
8 changes: 4 additions & 4 deletions tests/Unit/Utility/UriUtilTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,25 @@ public function testCredentials(): void
*/
public function testCredentialsUriWithoutCredentials(): void
{
static::assertSame([null, null], UriUtil::credentials(Uri::createFromString('https://example.com')));
static::assertSame([null, null], UriUtil::credentials(Uri::new('https://example.com')));
}

/**
* @covers ::credentials
*/
public function testCredentialsUriWithUsername(): void
{
static::assertSame(['shërlock', null], UriUtil::credentials(Uri::createFromString('https://sh%C3%ABrlock@example.com')));
static::assertSame(['shërlock', null], UriUtil::credentials(Uri::new('https://sh%C3%ABrlock@example.com')));
}

/**
* @covers ::credentials
*/
public function testCredentialsUriWithCredentials(): void
{
static::assertSame(['sherlock', 'passw*rd'], UriUtil::credentials(Uri::createFromString('https://sherlock:passw%2Ard@example.com')));
static::assertSame(['sherlock', 'passw*rd'], UriUtil::credentials(Uri::new('https://sherlock:passw%2Ard@example.com')));

// with special character
static::assertSame(['sher:lock', 'passw*rd'], UriUtil::credentials(Uri::createFromString('https://sher%3Alock:passw%2Ard@example.com')));
static::assertSame(['sher:lock', 'passw*rd'], UriUtil::credentials(Uri::new('https://sher%3Alock:passw%2Ard@example.com')));
}
}

0 comments on commit 9a4d665

Please sign in to comment.