Return 404 from Contents API when items don't exist

[zeripath]

This may technically be a breaking change but I think the current API is wrong enough that we should be returning 404.

[zeripath]

If course I now need to fix the tests which expect 500

[6543]

@zeripath now only test-change is left

[sapk]

I hesitate to think this should maybe kept as StatusInternalServerError to limit repo discovery.

In case you want to keep StatusForbidden you need to also update the others requests for consistency.
Like https://github.com/go-gitea/gitea/pull/10323/files#diff-f94444002d94f8c2766292e69cefbee2R466

[lafriks]

If you have read only access it should be forbidden status, in case you don't have any access it should be not found status but not internal server error

[zeripath]

In order to get to DeleteFile you have to pass:

						m.Delete("", bind(api.DeleteFileOptions{}), repo.DeleteFile)
					}, reqRepoWriter(models.UnitTypeCode), reqToken())
				}, reqRepoReader(models.UnitTypeCode))
...
			}, repoAssignment())

CanWriteFiles does the following in addition:

// CanWriteFiles returns true if repository is editable and user has proper access level.
func CanWriteFiles(r *context.Repository) bool {
	return r.Permission.CanWrite(models.UnitTypeCode) && !r.Repository.IsMirror && !r.Repository.IsArchived
}

Therefore HttpStatusForbidden or HttpBadRequest are the correct statuses not 404 nor 500.

[6543]

@zeripath pleace adjust Tests:

• TestAPIDeleteFile (api_repo_file_delete_test.go:41) Status: 400
• TestAPIGetContentsList (api_repo_get_contents_list_test.go:51) Status: 404 (&dont check msg)
• TestAPIGetContents (api_repo_get_contents_test.go:52) Status: 404 (&dont check msg)

[6543]

@zeripath send fix for this pr: zeripath#3

[6543]

@zeripath sorry missed to remove unused imports: zeripath#4

[zeripath]

@6543 just fixed it.

[6543]

api_repo_file_delete_test.go:113:
--
196 | Error Trace:	integration_test.go:400
197 | integration_test.go:258
198 | api_repo_file_delete_test.go:113
199 | git_helper_for_declarative_test.go:108
200 | api_repo_file_delete_test.go:39
201 | Error:      	Not equal:
202 | expected: 404
203 | actual  : 400
204 | Test:       	TestAPIDeleteFile
205 | Messages:   	Request: DELETE /api/v1/repos/user2/repo1/contents/delete/file5.txt?token=d289564555043a69039c84e140f71712bfb234b2
206 | api_repo_file_delete_test.go:113: Response: {"message":"sha does not match [given: badsha, expected: 103ff9234cefeee5ec5361d22b49fbb04d385885]","url":"http://localhost:3001/api/swagger"}

It just takes no end :/

[zeripath]

It appears the whole test just needs rewriting unfortunately.

This is why I prefer declarative tests which say what they're doing - much easier to write, add to and change.

[6543]

@zeripath can you apply my latest suggestion and update branch afterwards?

[6543]

worked 🎉 - so its ready to merge 🚀

[guillep2k]

Ping LG-TM