Skip to content

Commit f98f01d

Browse files
committed
Strengthen Azure Blob adapter tests after review
- Narrow the upload seam (uploadBlob) so the real upload() body and getUploadPath() composition run under test; assert the computed blob path. - Assert the listBlobs() prefix and the deleted blob pathname so a wrong prefix / wrong-target regression is caught. - Replace brittle exact debug() call-count expectations with assertions on the actual log messages and resulting state. - Assert simulate() masks the credential-bearing connection string (connectionString: ********) and never logs the raw value. Signed-off-by: Sebastian Mendel <sebastian.mendel@netresearch.de>
1 parent 1341799 commit f98f01d

4 files changed

Lines changed: 73 additions & 21 deletions

File tree

src/Backup/Sync/AzureBlob.php

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,19 @@ protected function createContainer(BlobContainerClient $client)
211211
protected function upload(Target $target, BlobContainerClient $client)
212212
{
213213
$source = $this->getFileHandle($target->getPathname(), 'r');
214-
$client->getBlobClient($this->getUploadPath($target))->upload($source);
214+
$this->uploadBlob($client, $this->getUploadPath($target), $source);
215+
}
216+
217+
/**
218+
* Upload a single blob to the container.
219+
*
220+
* @param \AzureOss\Storage\Blob\BlobContainerClient $client
221+
* @param string $path
222+
* @param resource $source
223+
*/
224+
protected function uploadBlob(BlobContainerClient $client, string $path, $source)
225+
{
226+
$client->getBlobClient($path)->upload($source);
215227
}
216228

217229
/**

tests/phpbu/Backup/Collector/AzureBlobTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,12 @@ class TestableAzureBlobCollector extends AzureBlob
1919
/** @var \AzureOss\Storage\Blob\Models\Blob[] */
2020
public $blobs = [];
2121

22+
/** @var string|null */
23+
public $capturedPrefix = null;
24+
2225
protected function listBlobs(string $prefix): iterable
2326
{
27+
$this->capturedPrefix = $prefix;
2428
return $this->blobs;
2529
}
2630
}
@@ -59,6 +63,7 @@ public function testCollector()
5963

6064
$files = $collector->getBackupFiles();
6165

66+
$this->assertEquals('collector/static-dir/', $collector->capturedPrefix, 'blobs must be listed with the static path prefix');
6267
$this->assertCount(2, $files);
6368
$this->assertArrayHasKey('975672000-foo-2000-12-01-12_00.txt-0', $files);
6469
$this->assertEquals(

tests/phpbu/Backup/File/AzureBlobTest.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,16 @@ class TestableAzureBlobFile extends AzureBlob
2020
/** @var bool */
2121
public $deleteThrows = false;
2222

23+
/** @var string|null */
24+
public $deletedPathname = null;
25+
2326
protected function deleteBlob(): void
2427
{
2528
if ($this->deleteThrows) {
2629
throw new \Exception('delete failed');
2730
}
28-
$this->deleted = true;
31+
$this->deleted = true;
32+
$this->deletedPathname = $this->pathname;
2933
}
3034
}
3135

@@ -58,6 +62,7 @@ public function testCreateFileWithCorrectProperties()
5862

5963
$file->unlink();
6064
$this->assertTrue($file->deleted, 'blob should be deleted');
65+
$this->assertEquals('dump.tar.gz', $file->deletedPathname, 'the correct blob must be targeted for deletion');
6166
}
6267

6368
/**

tests/phpbu/Backup/Sync/AzureBlobTest.php

Lines changed: 49 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ class TestableAzureBlobSync extends AzureBlob
2424
/** @var bool */
2525
public $uploaded = false;
2626

27+
/** @var string|null */
28+
public $uploadedPath = null;
29+
2730
/** @var bool */
2831
public $uploadThrows = false;
2932

@@ -45,12 +48,17 @@ protected function createContainer(BlobContainerClient $client)
4548
$this->containerCreated = true;
4649
}
4750

48-
protected function upload(Target $target, BlobContainerClient $client)
51+
/**
52+
* Narrow seam: the real upload() body (file handle + getUploadPath) runs;
53+
* only the final SDK call is replaced, capturing the computed blob path.
54+
*/
55+
protected function uploadBlob(BlobContainerClient $client, string $path, $source)
4956
{
5057
if ($this->uploadThrows) {
5158
throw new \Exception('upload failed');
5259
}
53-
$this->uploaded = true;
60+
$this->uploaded = true;
61+
$this->uploadedPath = $path;
5462
}
5563

5664
protected function getFileHandle($path, $mode)
@@ -160,10 +168,9 @@ public function testCreateClientBuildsContainerClient()
160168
*/
161169
public function testSync()
162170
{
163-
$target = $this->createTargetMock('foo.txt', 'foo.txt.gz');
164-
165-
$result = $this->createMock(Result::class);
166-
$result->expects($this->exactly(2))->method('debug');
171+
$target = $this->createTargetMock('foo.txt', 'foo.txt.gz');
172+
$messages = [];
173+
$result = $this->createDebugCollectingResult($messages);
167174

168175
$azureBlob = new TestableAzureBlobSync();
169176
$azureBlob->setup([
@@ -176,17 +183,19 @@ public function testSync()
176183

177184
$this->assertTrue($azureBlob->containerCreated, 'missing container should be created');
178185
$this->assertTrue($azureBlob->uploaded, 'backup should be uploaded');
186+
$this->assertStringStartsWith('backup/', (string) $azureBlob->uploadedPath, 'upload path must include the configured remote path');
187+
$this->assertContains('create blob container', $messages);
188+
$this->assertContains('upload: done', $messages);
179189
}
180190

181191
/**
182192
* Tests AzureBlob::sync does not create an already existing container
183193
*/
184194
public function testSyncContainerAlreadyExists()
185195
{
186-
$target = $this->createTargetMock('foo.txt', 'foo.txt.gz');
187-
188-
$result = $this->createMock(Result::class);
189-
$result->expects($this->once())->method('debug');
196+
$target = $this->createTargetMock('foo.txt', 'foo.txt.gz');
197+
$messages = [];
198+
$result = $this->createDebugCollectingResult($messages);
190199

191200
$azureBlob = new TestableAzureBlobSync();
192201
$azureBlob->containerExists = true;
@@ -200,17 +209,18 @@ public function testSyncContainerAlreadyExists()
200209

201210
$this->assertFalse($azureBlob->containerCreated, 'existing container must not be re-created');
202211
$this->assertTrue($azureBlob->uploaded, 'backup should be uploaded');
212+
$this->assertNotContains('create blob container', $messages, 'must not log container creation when it exists');
213+
$this->assertContains('upload: done', $messages);
203214
}
204215

205216
/**
206217
* Tests AzureBlob::sync with remote cleanup
207218
*/
208219
public function testSyncWithRemoteCleanup()
209220
{
210-
$target = $this->createTargetMock('foo.txt', 'foo.txt.gz');
211-
212-
$result = $this->createMock(Result::class);
213-
$result->expects($this->exactly(3))->method('debug');
221+
$target = $this->createTargetMock('foo.txt', 'foo.txt.gz');
222+
$messages = [];
223+
$result = $this->createDebugCollectingResult($messages);
214224

215225
$collector = $this->createMock(AzureBlobCollector::class);
216226
$collector->method('getBackupFiles')->willReturn([]);
@@ -228,6 +238,7 @@ public function testSyncWithRemoteCleanup()
228238
$azureBlob->sync($target, $result);
229239

230240
$this->assertTrue($azureBlob->uploaded, 'backup should be uploaded');
241+
$this->assertContains('upload: done', $messages);
231242
}
232243

233244
/**
@@ -258,18 +269,37 @@ public function testSimulate()
258269
{
259270
$azureBlob = new AzureBlob();
260271
$azureBlob->setup([
261-
'connection_string' => 'dummy-connection-string',
272+
'connection_string' => 'super-secret-connection-string',
262273
'container_name' => 'dummy-container-name',
263274
'path' => '/'
264275
]);
265276

266-
$resultStub = $this->createMock(Result::class);
267-
$resultStub->expects($this->once())
268-
->method('debug');
269-
277+
$messages = [];
278+
$resultStub = $this->createDebugCollectingResult($messages);
270279
$targetStub = $this->createMock(Target::class);
271280

272281
$azureBlob->simulate($targetStub, $resultStub);
282+
283+
$this->assertCount(1, $messages);
284+
// the credential carrying connection string must never be logged
285+
$this->assertStringContainsString('connectionString: ********', $messages[0]);
286+
$this->assertStringNotContainsString('super-secret-connection-string', $messages[0]);
287+
$this->assertStringContainsString('dummy-container-name', $messages[0]);
288+
}
289+
290+
/**
291+
* Build a Result mock that records every debug() message into $messages.
292+
*
293+
* @param array $messages
294+
* @return \phpbu\App\Result
295+
*/
296+
private function createDebugCollectingResult(array &$messages): Result
297+
{
298+
$result = $this->createMock(Result::class);
299+
$result->method('debug')->willReturnCallback(function ($message) use (&$messages) {
300+
$messages[] = $message;
301+
});
302+
return $result;
273303
}
274304

275305
/**

0 commit comments

Comments
 (0)