Skip to content

Commit

Permalink
prevent assertion error reverting unpublished asset definition set to…
Browse files Browse the repository at this point in the history 
… empty string (#1115)
  • Loading branch information
@Jamiras
Jamiras authored Aug 27, 2024
1 parent 99e6a22 commit 00c4295
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 10 deletions.
80 changes: 70 additions & 10 deletions src/api/impl/ConnectedServer.cpp
View file
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
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
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
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

0 comments on commit 00c4295

Please sign in to comment.