RegionLoader: Switch to using named constructors

this makes the code more self-descriptive, and also helps to detect potential bugs.

src/world/format/io/region/RegionLoader.php

@@ -83,30 +83,45 @@ class RegionLoader{
 	/**
 	 * @throws CorruptedRegionException
 	 */
-	public function __construct(string $filePath){
+	private function __construct(string $filePath){
 		$this->filePath = $filePath;
 		$this->garbageTable = new RegionGarbageMap([]);
-
-		clearstatcache(false, $this->filePath);
-		$exists = file_exists($this->filePath);
-		if(!$exists){
-			touch($this->filePath);
-		}elseif(filesize($this->filePath) % 4096 !== 0){
-			throw new CorruptedRegionException("Region file should be padded to a multiple of 4KiB");
-		}
+		$this->lastUsed = time();
 
 		$filePointer = fopen($this->filePath, "r+b");
 		if($filePointer === false) throw new AssumptionFailedError("fopen() should not fail here");
 		$this->filePointer = $filePointer;
 		stream_set_read_buffer($this->filePointer, 1024 * 16); //16KB
 		stream_set_write_buffer($this->filePointer, 1024 * 16); //16KB
-		if(!$exists){
-			$this->createBlank();
-		}else{
-			$this->loadLocationTable();
+	}
+
+	/**
+	 * @throws CorruptedRegionException
+	 */
+	public static function loadExisting(string $filePath) : self{
+		clearstatcache(false, $filePath);
+		if(!file_exists($filePath)){
+			throw new \RuntimeException("File $filePath does not exist");
+		}
+		if(filesize($filePath) % 4096 !== 0){
+			throw new CorruptedRegionException("Region file should be padded to a multiple of 4KiB");
 		}
 
-		$this->lastUsed = time();
+		$result = new self($filePath);
+		$result->loadLocationTable();
+		return $result;
+	}
+
+	public static function createNew(string $filePath) : self{
+		clearstatcache(false, $filePath);
+		if(file_exists($filePath)){
+			throw new \RuntimeException("Region file $filePath already exists");
+		}
+		touch($filePath);
+
+		$result = new self($filePath);
+		$result->createBlank();
+		return $result;
 	}
 
 	public function __destruct(){

src/world/format/io/region/RegionWorldProvider.php

@@ -114,7 +114,7 @@ abstract class RegionWorldProvider extends BaseWorldProvider{
 			$path = $this->pathToRegion($regionX, $regionZ);
 
 			try{
-				$this->regions[$index] = new RegionLoader($path);
+				$this->regions[$index] = RegionLoader::loadExisting($path);
 			}catch(CorruptedRegionException $e){
 				$logger = \GlobalLogger::get();
 				$logger->error("Corrupted region file detected: " . $e->getMessage());
@@ -123,7 +123,7 @@ abstract class RegionWorldProvider extends BaseWorldProvider{
 				rename($path, $backupPath);
 				$logger->error("Corrupted region file has been backed up to " . $backupPath);
 
-				$this->regions[$index] = new RegionLoader($path); //this will create a new empty region to replace the corrupted one
+				$this->regions[$index] = RegionLoader::createNew($path);
 			}
 		}
 		return $this->regions[$index];

tests/phpunit/world/format/io/region/RegionLoaderTest.php

@@ -25,11 +25,14 @@ namespace pocketmine\world\format\io\region;
 
 use PHPUnit\Framework\TestCase;
 use pocketmine\world\format\ChunkException;
+use function bin2hex;
+use function clearstatcache;
 use function file_exists;
 use function random_bytes;
 use function str_repeat;
 use function sys_get_temp_dir;
 use function unlink;
+use const DIRECTORY_SEPARATOR;
 
 class RegionLoaderTest extends TestCase{
 
@@ -43,7 +46,7 @@ class RegionLoaderTest extends TestCase{
 		if(file_exists($this->regionPath)){
 			unlink($this->regionPath);
 		}
-		$this->region = new RegionLoader($this->regionPath);
+		$this->region = RegionLoader::createNew($this->regionPath);
 	}
 
 	public function tearDown() : void{
@@ -63,7 +66,7 @@ class RegionLoaderTest extends TestCase{
 		$this->region->writeChunk(0, 0, $data);
 		$this->region->close();
 
-		$r = new RegionLoader($this->regionPath);
+		$r = RegionLoader::loadExisting($this->regionPath);
 		self::assertSame($data, $r->readChunk(0, 0));
 	}
 
@@ -119,11 +122,27 @@ class RegionLoaderTest extends TestCase{
 	 */
 	public function testRegionHeaderCachedFilesizeRegression() : void{
 		$this->region->close();
-		$region = new RegionLoader($this->regionPath); //now we have a region, so the header will be verified, triggering two filesize() calls
+		$region = RegionLoader::loadExisting($this->regionPath); //now we have a region, so the header will be verified, triggering two filesize() calls
 		$data = str_repeat("hello", 2000);
 		$region->writeChunk(0, 0, $data); //add some data to the end of the file, to make the cached filesize invalid
 		$region->close();
-		$region = new RegionLoader($this->regionPath);
+		$region = RegionLoader::loadExisting($this->regionPath);
 		self::assertSame($data, $region->readChunk(0, 0));
 	}
+
+	public function testCreateNewWithExistingRegion() : void{
+		$this->region->close();
+		$this->expectException(\RuntimeException::class);
+		RegionLoader::createNew($this->regionPath);
+	}
+
+	public function testLoadExistingWithMissingFile() : void{
+		clearstatcache();
+
+		do{
+			$randfile = sys_get_temp_dir() . DIRECTORY_SEPARATOR . bin2hex(random_bytes(6)) . ".mca";
+		}while(file_exists($randfile));
+		$this->expectException(\RuntimeException::class);
+		RegionLoader::loadExisting($randfile);
+	}
 }

tools/compact-regions.php

@@ -108,7 +108,7 @@ function main(array $argv) : int{
 	$totalCount = count($files);
 	foreach($files as $file => $size){
 		try{
-			$oldRegion = new RegionLoader($file);
+			$oldRegion = RegionLoader::loadExisting($file);
 		}catch(CorruptedRegionException $e){
 			$logger->error("Damaged region in file $file (" . $e->getMessage() . "), skipping");
 			$corruptedFiles[] = $file;
@@ -117,7 +117,7 @@ function main(array $argv) : int{
 		}
 
 		$newFile = $file . ".compacted";
-		$newRegion = new RegionLoader($newFile);
+		$newRegion = RegionLoader::createNew($newFile);
 
 		$emptyRegion = true;
 		$corruption = false;