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

Implements a Context that talks to the store mock server #293

Merged
merged 11 commits into from
Sep 22, 2023
Merged

Implements a Context that talks to the store mock server #293

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
189e3ea
Implements the WinMockContext header
CarlosNihelton Sep 20, 2023
b8b92b0
Implements the call() HTTP function wrapper
CarlosNihelton Sep 21, 2023
893dc4d
Moves to_hstrings() to a new header
CarlosNihelton Sep 21, 2023
de69f7a
Adds a Product factory fromJson
CarlosNihelton Sep 21, 2023
752450e
Implements GetProducts
CarlosNihelton Sep 21, 2023
09605ce
Implements AllLocallyAuthenticatedUserHashes
CarlosNihelton Sep 21, 2023
13bd038
Implements GenerateUserJwt.
CarlosNihelton Sep 21, 2023
7a78f2d
InitDialogs is NOOP for now
CarlosNihelton Sep 21, 2023
3ccb3b4
Implements purchase
CarlosNihelton Sep 21, 2023
9d029e4
Support for testing the services with the mock
CarlosNihelton Sep 22, 2023
1615761
Turns fromJson into a Product constructor
CarlosNihelton Sep 22, 2023
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
17 changes: 17 additions & 0 deletions storeapi/base/impl/WinMockContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,23 @@ std::vector<std::string> WinMockContext::AllLocallyAuthenticatedUserHashes() {
return result;
}

std::string WinMockContext::GenerateUserJwt(std::string token,
std::string userId) const {
assert(!token.empty() && "Azure AD token is required");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this simply mirrors the store context, but I'm thinking that, in both contexts, this assertion should also be run on Release (or remove it and let the server fail, doesn't really matter).

If we leave it as-is, we have diferent failure modes in Release and Debug, so we won't be testing the right thing. Maybe we can create a card and deal with these in a future PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of an assertion is to educate the programmers to ensure those checks won't be needed in release.
If we are unsure about whether we are safe to trust that those checks aren't needed in production, then we should revisit the usage of assertions and maybe convert into wide contracts with exceptions being thrown for invalid arguments.
I personally like assertions because they clearly state: "you'll called me outside of our contract, that's a bug, take your crash". Once we crash enough in debug mode, those checks shouldn't be necessary anymore. Exceptions are more fuzzy. What is an exceptional situation? As always, it depends...

Copy link
Contributor

@EduardGomezEscandell EduardGomezEscandell Sep 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having assertions mostly where an error is critical: to avoid segfaults or undefined behaviour. In this case, since this would error out anyways (just server-side), I'm not sure it is worth having a release/debug split. Particularly because we test error paths quite intensely, so we'd be testing a ficticious path here.

Since this is not performance-critital, it seems that it would not hurt to always return error, or to always let the server fail. Either way, this is not critical to this PR.

JsonObject res{nullptr};

UrlParams parameters{
{L"serviceticket", winrt::to_hstring(token)},
};
if (!userId.empty()) {
parameters.insert({L"publisheruserid", winrt::to_hstring(userId)});
}

res = call(L"generateuserjwt", parameters).get();

return winrt::to_string(res.GetNamedString(L"jwt"));
}

namespace {
EduardGomezEscandell marked this conversation as resolved.
Show resolved Hide resolved
// Returns the mock server endpoint address and port by reading the environment
// variable UP4W_MS_STORE_MOCK_ENDPOINT or localhost:9 if the variable is unset.
Expand Down