MS Store API backend mock #268
Conversation
CarlosNihelton
force-pushed
the
store-mock-udeng-1257
branch
2 times, most recently
from
970e72d to
6577c7f
Compare
There was a problem hiding this comment.
Good overall structure. The refactor in #267 makes this easy to review :).
| } | ||
|
|
||
| if id == "servererror" { | ||
| slog.Info("server error triggered", id) |
There was a problem hiding this comment.
I'd prefix this with the function/endpoint name so that it's easier to read the logs
There was a problem hiding this comment.
Maybe instead of prefixing, having the "endpoint" key-value pair in the log record?
There was a problem hiding this comment.
I'm not very familiar with what is idiomatic to separate into the key-value pairs. I know last time I put stuff into the KV pairs, I was told not to 😅.
@didrocks can maybe help.
There was a problem hiding this comment.
@didrocks PING?
There was a problem hiding this comment.
So, the pair/value is basically for metadata on filtering logs (really used in services in particular). What we need to think about is: for the average user, is this readable?
So, anything that is important and printed client side should read as a sentence (ideally without stuttering, which is the hard part when wrappers errors on errors and reporting them after traversing multiple stacks).
For server sides logs too: it should be readable: what did fail? Which requests was dropped? Then, if we need analyzes and reporting this in another place with filtering, this is where the key-value pair helps.
One instance of a valuable key/value pair: session-id: this is autogenerated, brings no value for the user/admin, but helps to group any calls for a session.
I hope this sheds some lights on that topic. :)
| } | ||
|
|
||
| if p.IsInUserCollection { | ||
| slog.Info("product already in user collection", id) |
There was a problem hiding this comment.
slog's calls have a first message and then key-value pairs, so this call is ill-formed (we should get a linter for this).
The right call would be:
slog.Info("product already in user collection", "id", id)or probably better--if uglier:
slog.Info(fmt.Sprintf("product %q already in user collection", id))Other than this, same comment about adding some context to the log indicating the endpoint it originates from.
There was a problem hiding this comment.
I prefer the first recommendation. Seems more in line with the whole purpose of structured logging.
There was a problem hiding this comment.
Great!
Feel free to rebase main (so that #278 allows the debian build to pass) and the merge without further re-approval.
Import the building blocks from package restserver
Failing fast:
If the product is named "nonexistent" we fail fast = 400
If the product is named "servererror" we return a server error
If the product is named "cannotpurchase" we return a not purchased error
The product properties are updated if the purchase succeeds.
If the requested product ID exists in the database the purchase suceeds,
unless it is in the user collection already ("AlreadyPurchased")
// https://learn.microsoft.com/en-us/uwp/api/windows.services.store.storepurchasestatus?view=winrt-22621#fields
Products are filtered by IDs and Kinds as specified in MS Docs Return null is not an error. the output is solely governed by the store state and the query. No magic words for this method. See https://learn.microsoft.com/en-us/uwp/api/windows.services.store.storecontext.getstoreproductsasync
serviceTicket (a.k.a. Azure token) is required. publisherUserID is not, but if supplied, goes back in the response "JWT" expiredtoken and servererror are special serviceTiket values
NewMux was inlined as with the contracs server mock Address no longer part of Settings s/EndpointOk/NewEndpoint/g
@EduardGomezEscandell showed that passing the pointer to defaultSettings was the reason for the double star in serverFactory.
Log messages must stay readable and complete. Structured logging fields are for data analysis. A human reading a log should not need to look into them to completely understand what's being reported.
CarlosNihelton
force-pushed
the
store-mock-udeng-1257
branch
from
14179dc to
0b36e86
Compare
This PR adds a replacement for the thin MS Store API wrapper by a not-so-thin client of the store mock server we presented in #268. For ease of implementation I opted for writing a Windows specific version of the mock client. You'll see that it's indeed not that hard to talk to REST API's using WinRT. Since we adopted a very consistent calling convention in our REST API, a single `call()` function models very well talking to any of the exposed endpoints, so that the `WinMockContext` methods just wrap around it passing the appropriate arguments. The actual HTTP call could be more elaborated, using streamed data to prevent against large responses, but I don't believe this is necessary, because we control the server and we know the responses are rather small. Since this client code is not trivial, I added a few test cases for it, but I didn't plug it in CI right away because running it is a bit cumbersome: 1. We need to acquire a port to run the store mock server 2. Export the environment variable `UP4W_MS_STORE_MOCK_ENDPOINT` with the address the mock server will run (`localhost:port`) 3. Run the store mock server passing that port and the testcase.yaml configuration file. 4. Export the environment variable `UP4W_TEST_WITH_MS_STORE_MOCK` so the replacement context takes the stage instead of the production one. 5. Configure and build with CMake 6. Run CTest. Not that complicated, but requires Go to build the store mock server and a few lines of powershell scripting to do the port thingy. So I found that this is not the appropriate moment for pluging this on in the CI. Also, the mock client is for supporting testing higher level components, and this is testing it, not the targets. Here's a gist of how I run on my computer: ```powershell cd ./storeapi/test $listener = new-object System.Net.Sockets.TcpListener("127.0.0.1",0) $listener.Start() $env:UP4W_TEST_WITH_MS_STORE_MOCK=1 cmake -S . -B .\out\build\ cmake --build .\out\build $env:UP4W_MS_STORE_MOCK_ENDPOINT="$($listener.LocalEndpoint.Address):$($listener.LocalEndopint.Port)"; $listener.Stop() go run ..\..\mocks\storeserver\storeserver\main.go -a $env:UP4W_MS_STORE_MOCK_ENDPOINT .\storeapi\test\testcase.yaml # On another terminal ctest --test-dir .\out\dir --output-on-failure ``` Fixes Udeng-1258
The details of the responses may change soon, but the most important thing about this PR is that it puts in place the code we need to run a REST server that will mock the MS Store API.
It reuses the bits extracted from the contracts server mock in #267 and reimplements the relevant parts to create the following endpoints:
/allauthenticatedusers- returns some representation of the user accounts found locally authenticated on the system./generateuserjwt- returns the user JWT if the query parameters check and the settings allow./getproducts- returns a collection of products - JSON content type./purchase- handles a purchase request, updating the in-memory server state if the transaction is accepted.That should be enough for us to implement the client API and some test cases & fixtures soon.