Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

prevent assertion error reverting unpublished asset definition set to empty string #1115

Merged
merged 1 commit into from
Aug 27, 2024
Merged

prevent assertion error reverting unpublished asset definition set to empty string #1115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 70 additions & 10 deletions src/api/impl/ConnectedServer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,8 @@ ResolveHash::Response ConnectedServer::ResolveHash(const ResolveHash::Request& r
api_params.game_hash = request.Hash.c_str();

rc_api_request_t api_request;
if (rc_api_init_resolve_hash_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_resolve_hash_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, ResolveHash::Name(), httpResponse, response))
Expand All @@ -539,6 +540,11 @@ ResolveHash::Response ConnectedServer::ResolveHash(const ResolveHash::Request& r
rc_api_destroy_resolve_hash_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand All @@ -554,7 +560,8 @@ FetchCodeNotes::Response ConnectedServer::FetchCodeNotes(const FetchCodeNotes::R
api_params.game_id = request.GameId;

rc_api_request_t api_request;
if (rc_api_init_fetch_code_notes_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_fetch_code_notes_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, FetchCodeNotes::Name(), httpResponse, response))
Expand Down Expand Up @@ -585,6 +592,11 @@ FetchCodeNotes::Response ConnectedServer::FetchCodeNotes(const FetchCodeNotes::R
rc_api_destroy_fetch_code_notes_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand Down Expand Up @@ -625,7 +637,8 @@ static void SetCodeNote(ApiResponseBase& response, const char* sApiName,
api_params.note = sNote;

rc_api_request_t api_request;
if (rc_api_init_update_code_note_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_update_code_note_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, sApiName, httpResponse, response))
Expand All @@ -641,6 +654,11 @@ static void SetCodeNote(ApiResponseBase& response, const char* sApiName,
rc_api_destroy_update_code_note_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
}
Expand Down Expand Up @@ -689,7 +707,8 @@ UpdateAchievement::Response ConnectedServer::UpdateAchievement(const UpdateAchie
api_params.type = request.Type;

rc_api_request_t api_request;
if (rc_api_init_update_achievement_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_update_achievement_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, UpdateAchievement::Name(), httpResponse, response))
Expand All @@ -706,6 +725,11 @@ UpdateAchievement::Response ConnectedServer::UpdateAchievement(const UpdateAchie
rc_api_destroy_update_achievement_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand All @@ -730,7 +754,8 @@ FetchAchievementInfo::Response ConnectedServer::FetchAchievementInfo(const Fetch
api_params.friends_only = 1;

rc_api_request_t api_request;
if (rc_api_init_fetch_achievement_info_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_fetch_achievement_info_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, FetchAchievementInfo::Name(), httpResponse, response))
Expand Down Expand Up @@ -759,6 +784,11 @@ FetchAchievementInfo::Response ConnectedServer::FetchAchievementInfo(const Fetch
rc_api_destroy_fetch_achievement_info_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand Down Expand Up @@ -790,7 +820,8 @@ UpdateLeaderboard::Response ConnectedServer::UpdateLeaderboard(const UpdateLeade
api_params.format = ValueFormatToString(request.Format);

rc_api_request_t api_request;
if (rc_api_init_update_leaderboard_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_update_leaderboard_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, UpdateLeaderboard::Name(), httpResponse, response))
Expand All @@ -807,6 +838,11 @@ UpdateLeaderboard::Response ConnectedServer::UpdateLeaderboard(const UpdateLeade
rc_api_destroy_update_leaderboard_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand All @@ -829,7 +865,8 @@ FetchLeaderboardInfo::Response ConnectedServer::FetchLeaderboardInfo(const Fetch
api_params.count = request.NumEntries;

rc_api_request_t api_request;
if (rc_api_init_fetch_leaderboard_info_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_fetch_leaderboard_info_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, FetchLeaderboardInfo::Name(), httpResponse, response))
Expand Down Expand Up @@ -859,6 +896,11 @@ FetchLeaderboardInfo::Response ConnectedServer::FetchLeaderboardInfo(const Fetch
rc_api_destroy_fetch_leaderboard_info_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand Down Expand Up @@ -901,7 +943,8 @@ FetchGamesList::Response ConnectedServer::FetchGamesList(const FetchGamesList::R
api_params.console_id = request.ConsoleId;

rc_api_request_t api_request;
if (rc_api_init_fetch_games_list_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_fetch_games_list_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, FetchGamesList::Name(), httpResponse, response))
Expand All @@ -924,6 +967,11 @@ FetchGamesList::Response ConnectedServer::FetchGamesList(const FetchGamesList::R
rc_api_destroy_fetch_games_list_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand Down Expand Up @@ -952,7 +1000,8 @@ SubmitNewTitle::Response ConnectedServer::SubmitNewTitle(const SubmitNewTitle::R
api_params.hash_description = sDescription.c_str();

rc_api_request_t api_request;
if (rc_api_init_add_game_hash_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_add_game_hash_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, SubmitNewTitle::Name(), httpResponse, response))
Expand All @@ -969,6 +1018,11 @@ SubmitNewTitle::Response ConnectedServer::SubmitNewTitle(const SubmitNewTitle::R
rc_api_destroy_add_game_hash_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand All @@ -982,7 +1036,8 @@ FetchBadgeIds::Response ConnectedServer::FetchBadgeIds(const FetchBadgeIds::Requ
memset(&api_params, 0, sizeof(api_params));

rc_api_request_t api_request;
if (rc_api_init_fetch_badge_range_request(&api_request, &api_params) == RC_OK)
const int result = rc_api_init_fetch_badge_range_request(&api_request, &api_params);
if (result == RC_OK)
{
ra::services::Http::Response httpResponse;
if (DoRequest(api_request, FetchBadgeIds::Name(), httpResponse, response))
Expand All @@ -1000,6 +1055,11 @@ FetchBadgeIds::Response ConnectedServer::FetchBadgeIds(const FetchBadgeIds::Requ
rc_api_destroy_fetch_badge_range_response(&api_response);
}
}
else
{
response.Result = ApiResult::Failed;
response.ErrorMessage = rc_error_str(result);
}

rc_api_destroy_request(&api_request);
return response;
Expand Down
6 changes: 6 additions & 0 deletions src/data/models/AssetModelBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,12 @@ void AssetModelBase::CommitTransaction()
pAsset->m_bLocalModified = true;
UpdateAssetDefinitionVersion(*pAsset, AssetChanges::Unpublished);
}
else if (pAsset->m_sLocalDefinition.empty() && !pAsset->m_bLocalModified)
{
// local and current are both empty, but local isn't modified.
// just change the state to unpublished.
UpdateAssetDefinitionVersion(*pAsset, AssetChanges::Unpublished);
}
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions tests/data/models/AssetModelBase_Tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,29 @@ TEST_CLASS(AssetModelBase_Tests)
Assert::AreEqual(std::string("ServerDefinition"), asset.GetDefinition());
}

TEST_METHOD(TestRestoreDefinitionCheckpointEmpty)
{
AssetDefinitionViewModelHarness asset;
asset.SetDefinition("ServerDefinition");
asset.CreateServerCheckpoint();
asset.CreateLocalCheckpoint(); // no local value

Assert::AreEqual(AssetChanges::None, asset.GetChanges());
Assert::AreEqual(std::string("ServerDefinition"), asset.GetDefinition());

asset.SetDefinition("");
Assert::AreEqual(AssetChanges::Modified, asset.GetChanges());
Assert::AreEqual(std::string(""), asset.GetDefinition());

asset.UpdateLocalCheckpoint();
Assert::AreEqual(AssetChanges::Unpublished, asset.GetChanges());
Assert::AreEqual(std::string(""), asset.GetDefinition());

asset.RestoreLocalCheckpoint();
Assert::AreEqual(AssetChanges::Unpublished, asset.GetChanges());
Assert::AreEqual(std::string(""), asset.GetDefinition());
}

TEST_METHOD(TestResetLocalCheckpoint)
{
AssetModelHarness asset;
Expand Down
40 changes: 40 additions & 0 deletions tests/ui/viewmodels/AssetUploadViewModel_Tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,46 @@ TEST_CLASS(AssetUploadViewModel_Tests)
vmUpload.AssertFailed(0, 1, L"* Title1: You must be a developer to modify values in Core!");
}

TEST_METHOD(TestSingleCoreAchievementApiError)
{
AssetUploadViewModelHarness vmUpload;
auto& pAchievement =
vmUpload.AddAchievement(AssetCategory::Core, 5, L"Title1", L"Desc1", L"12345", "0xH1234=1");
pAchievement.UpdateServerCheckpoint();
Assert::AreEqual(AssetChanges::None, pAchievement.GetChanges());

pAchievement.SetTrigger("");
pAchievement.UpdateLocalCheckpoint();
Assert::AreEqual(AssetChanges::Unpublished, pAchievement.GetChanges());

vmUpload.QueueAsset(pAchievement);
Assert::AreEqual({1U}, vmUpload.TaskCount());

// mockServer doesn't call the API that would return Error/Invalid state, so update the mockServer
// to behave as if the failure happened at the API level.
bool bApiCalled = false;
vmUpload.mockServer.HandleRequest<ra::api::UpdateAchievement>(
[&bApiCalled](const ra::api::UpdateAchievement::Request&, ra::api::UpdateAchievement::Response& pResponse) {
bApiCalled = true;
pResponse.Result = ra::api::ApiResult::Error;
pResponse.ErrorMessage = "Invalid state";
return true;
});

vmUpload.DoUpload();

Assert::IsTrue(bApiCalled);
Assert::AreEqual(AssetChanges::Unpublished, pAchievement.GetChanges());

vmUpload.AssertFailed(0, 1, L"* Title1: Invalid state");

// This simulates the failure code in AssetListViewModel::Publish. The test can't be in
// AssetListViewModel_Tests, because AssetListViewModel_Tests overrides Publish to avoid
// using the AssetUploadViewModel.
pAchievement.RestoreLocalCheckpoint();
Assert::AreEqual(AssetChanges::Unpublished, pAchievement.GetChanges());
}

TEST_METHOD(TestMultipleCoreAchievements)
{
AssetUploadViewModelHarness vmUpload;
Expand Down
Loading