ファイル名に使われていない数字までループ

Question

前回の質問で画像を保存できるようになったんですが、画像を上書きせずに、重複していたら、ユーザ名_連番.pngという風にやっていきたいとおもいまして、コードを書いてみました。

//  もし同じファイル名があったら
if(file_exists($_SERVER['DOCUMENT_ROOT'] . '/../users/icons/'.$app->me()->id.".png")){
	//  ループ回数の変数
	$nu;
	//  使われていない数字までループを回す
	for($i = 0; file_exists($_SERVER['DOCUMENT_ROOT'] . '/../users/icons/'.$app->me()->id."_".$i.".png") === true; $i++){
		//  ループ回数を代入（使われていない数）
		$nu = $i;
    }
	rename($_SERVER['DOCUMENT_ROOT'] . '/../users/icons/'.$app->me()->id.".png", $_SERVER['DOCUMENT_ROOT'] . '/../users/icons/'.$app->me()->id."_".$nu.".png");
}

// ファイルデータからSHA-1ハッシュを取ってファイル名を決定し、ファイルを保存する
$path = sprintf($_SERVER['DOCUMENT_ROOT'] . '/../users/icons/'.$app->me()->id.".png");
	if (!move_uploaded_file($_FILES['upfile']['tmp_name'], $path)) {
		throw new RuntimeException('ファイル保存時にエラーが発生しました');
	}
chmod($path, 0644);

もし、ファイルが投稿されて、ファイルを保存する祭に、同じファイル名のファイルがあったら、そのファイルに対してファイル名_連番.pngというふうに名前を変えて、保存したいです。そのあとに、送信された画像が、ファイル名.pngで保存できますよね

問題は、for文で使われていない数字まで回すところが上手くいきません。
###追記

//  もし同じファイル名があったら
        $fname = $_SERVER['DOCUMENT_ROOT'] . '/../users/icons/'.$app->me()->id;
        if(file_exists($fname.".png")){
          //  フラグ
					$flg = true;
					//  ループ回数の変数
				  $i = 1;
				  //  使われていない数字までループを回す
					while($flg){
						if(!file_exists($fname.'_'.$i.'.png')){
							//重複する画像のリネーム処理
							rename($fname.".png", $fname."_".$i.".png");
							//$fname.'_'.$i.'.png'でネーミング
							$flg =false;
						}else{
						  //  ループ回数増加
							$i++;
						}
					}
        }
        
        // ファイルデータからSHA-1ハッシュを取ってファイル名を決定し、ファイルを保存する
        $path = sprintf($_SERVER['DOCUMENT_ROOT'] . '/../users/icons/'.$app->me()->id.".png");
        if (!move_uploaded_file($_FILES['upfile']['tmp_name'], $path)) {
          throw new RuntimeException('ファイル保存時にエラーが発生しました');
        }
        chmod($path, 0644);

Comments

[m.ts10806]

ソースコードをかなり削られたようですが・・・？png以外の拡張子は考えない感じですか？

[ryo-dev]

はい。pngのみです。他のコードを削ったのは、うまくいかないのは、このループのみで、他に沢山書いてあるとややこしいので省きました。

[m.ts10806]

他に沢山書いてある中に必要な情報があるかもしれないのに削るのは低評価のもとです。これだけではどういう流れで来ているのかわかりません。ややこしいかどうかは回答者側が判断する内容であって、質問者は出来る限りの情報を提示すべきです。それに前回の質問にもコメントがありましたが同名だが実態が画像ではない場合も考慮すべきとは思います（セキュリティ問題）

[ryo-dev]

元の状態に戻します。ファイルの中のチェックはまだ追加できてません。この問題が解決できたら、後で付けます

Answer 1

forよりwhileを使った方がいいんじゃないですかね。

$fname = "test";
if(file_exists($fname.'.png')){

	$flg = true;

	$i = 1;
	while($flg){
		if(!file_exists($fname.'_'.$i.'.png')){
			//重複する画像のアップロード処理
			//$fname.'_'.$i.'.png'でネーミング
			$flg =false;
		}else{

			$i++;
		}
	}
}else{
	//重複しない画像のアップロード処理
}

Comments

[ryo-dev]

//重複する画像のアップロード処理
//$fname.'_'.$i.'.png'でネーミング
とありますが、ここでアップロードすると、おかしくなりませんか？
たとえば、test.pngというファイルをアップした時にすでにtest.pngというファイルがあった時にtest_1.pngからtest_5.pngまで既にあったとします。そしたら、６を探すまでループさせるじゃないですか、その探すループ中にファイルをすべてリネームするので、ここのループでは、使っていない数字を取得するだけにして、見つかったら抜けて、抜けた先でリネームしないとダメじゃないですか？

[m.ts10806]

そもそもですが、「test.pngというファイルをアップした時にすでにtest.pngというファイルがあった時に」は既に上書きされていますよね。
なので、最初の「if(file_exists($fname.'.png')){」で所在を確認し、アップはしません（というか、アップしてはいけません）
同様に組んで動かしてみればわかると思いますが、
「該当名称のファイルがない」ことを確認し、rename()なりなんなりでtmpからリネームしたらすぐにループを抜けています。
つまり「//重複する画像のアップロード処理」部分には重複しない名前が来るまで入りません。
入ったらご提示のケースだと「test_6.png」だけをアップしてループを抜けるためこの処理で問題ありません。
もちろん、数字だけとるのでもいいですが、その場合はelseで$i++させないようにしないといけないので念のためサンプルコードを修正しました。あとは$flg =false;ではなくbreak;の方が直感的でわかりやすいかもしれませんね。

[ryo-dev]

やりたい事は下の通りです。これで伝わってますよね？
もし、test.phpをアップした時に既に同じ名前が合った場合で、test_1.pngからtest_5.pngを使われていたら、アップする前に元あったファイルをtest_6.pngにリネームして、test.pngでアップされた画像を保存したいんです。

[ryo-dev]

追記で書かさせていただいた、ものがうまく動くものになります。
whilにelseが合ったと思うんですが、あのelseの中に重複しない時のコードを書くと、前の所で使われていない数字にリネームしたあと、保存をうまくしてくれなかったので、elseを無くして、独立させました。
こてで、リネームした後も、ファイルが無かった場合も両方をこれで保存できるようになりました。
あとは、本当に画像ファイルであるかを確かめるチェックをできるようにしたいです。

[m.ts10806]

あれ？解決済みになってますね。。
// ファイルデータからSHA-1ハッシュを取ってファイル名を決定し、ファイルを保存する
の部分がwhile内のrenameのところに来るのかなと思ってたんですが。。
ひとまず解決したようなので良かったです。
チェックについてはQiitaに良い記事がたくさんあがっているので、そちらを参考にするといいです。
どこまでやるかとかどうするのがベストかというのは色々試してみてくださいね。

[ryo-dev]

ファイルデータからSHA-1ハッシュを取って　というのが、本当に画像データかのチェックですか？
またwhile内のrename？

[退会済みユーザー]

横から失礼。偶然この質問を見かけたんですが、なんか本来の質問内容からそれていませんかね？解決ずみになっていますし、質問者さんにも質問内容が解決した自覚はあるんでしょう。
解決したあとに出てきた問題までどうすればいいかきいていたら、特定の回答者への負担が高くなるから、別の質問として投稿したらいかが？

[m.ts10806]

> ファイルデータからSHA-1ハッシュを取って
自身で書かれてることじゃないですか・・・
画像データかどうかのチェックは別途調べてわからないことがあれば新規質問あげてください。

[ryo-dev]

理解できました。
// ファイルデータからSHA-1ハッシュを取ってファイル名を決定し、ファイルを保存する
はそのままの場所でいいです。
もし、重複していなければそのまま保存で、重複していたら、元からあるファイル（古いファイル）の名前をリネームして、その後に保存したいので、最後に書かないといけないです。
画像の確認の方法は調べます。