These are chat archives for Ruhrpottpatriot/GW2.NET

9th
Mar 2016
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 13:58
A
I’m happy. I just added the whole v2/achivements endpoint (except dailies, because of weird format) in just 45min or less. Most time was converter writing
my refactoring definetly was successful
Steven Liekens
@StevenLiekens
Mar 09 2016 14:15
does that mean we are ready for a major release?
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:16
not yet
WvW is missing, since @ZedTheYeti is working on it and other endpoints are also missing
also the negotiating code needs to be more robust
it has barely any error checking right now and api keys are not working right now
I’ll do the latter two things today evening or tomorrow
Oh, by the way:
Any idea how to decouple the repository queries from discoverable queries?
currently the Get() and Get(identifiers) method do a call to Discover() to update the ids
Steven Liekens
@StevenLiekens
Mar 09 2016 14:19
how so
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:20
but this means, if a repository is not implementing IDiscoverable (which is currently not the case, but it might be) Get() and Get(identifiers) will throw an exception for no apparent reason
Steven Liekens
@StevenLiekens
Mar 09 2016 14:21
/v2/guilds probably won't be discoverable
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:22
yes, but you should be able to reuqest one guild (which is possible, since Get(id)does no call to Discover()) and many (which is not possible, since Get(ids) does a call to Discover())
Steven Liekens
@StevenLiekens
Mar 09 2016 14:22
there might be bulk expansion for guilds
there probably will be
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:23
bulk expansion was what again?
Steven Liekens
@StevenLiekens
Mar 09 2016 14:23
ids=1,2,3
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:24
wait. I think did a mistake. Need to check smthg
Ok. I got it partially right.
Let’s look at IRepository<TKey, TDataModel, TData>.Get() first: To support querying all items, even if ?ids=all is not supported (which on some endpoints is true), we first do a call to IDiscoverable<TIn, TOut>.Doscover(). After that we query all ids (in parallel) and convert them
I did this, so I had maximum code reusage, but that meant coupling Get() to Discover()
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:30
Thought: Change Get()to query not by a list of ids, but by pagination. However: How do we get the number of pages in advance, so we can query all pages in parallel?
Steven Liekens
@StevenLiekens
Mar 09 2016 14:31
you can get the first page in a task and then get all remaining pages in a continuation task
the first page contains a response header that says how many pages there are
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:33
Good. I think that’s that for uncached queries
As for cached queries
At first I’m doing a Discover() and then call the cached version of Get(ids). In that method all items from the cache are retrieved with the ids present in the query. Every missing id is the queries from the server via the uncached Get(ids)method and added to the cache before returning the requested items.
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:41
Here is the problem: If I do a normal — uncached — Get()to retrieve missing items the cache is basically useless, since I’m querying all items anyway. On the other hand I have no sane way to determine the missing Ids without doing a discover.
A solution would be to remove the cached version of Get(), but this method is the method which would benefir from caching the most.
Any ideas?
Steven Liekens
@StevenLiekens
Mar 09 2016 14:56
I don't understand why Get() is not cached
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:56
currently it is
we have two interfaces
Steven Liekens
@StevenLiekens
Mar 09 2016 14:57
so one is the implementation that fills the cache
but not meant to be called from user code?
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 14:58
ICachedRepository and IRepository the first does a query cache and if some items are missing it calls the uncached interface and retrieves the missing items, adds them to the cache and then returns all items to the user
And both can be queried by the user
ICachedRepository implements IRepository. So by casting to IRepository the user can bypass the cache
Damn: I just queried the stats for my branch: "1299 files changed, 22277 insertions(+), 59465 deletions(-)"
Steven Liekens
@StevenLiekens
Mar 09 2016 15:00
sweet
Steven Liekens
@StevenLiekens
Mar 09 2016 15:02
why bother exposing the converters?
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:02
Because only that way the extension methods can access them
Steven Liekens
@StevenLiekens
Mar 09 2016 15:03
oh okay
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:03
public static async Task<TValue> GetAsync<TKey, TDataContract, TValue>(this ICachedRepository<TKey, TDataContract, TValue> repository, TKey identifier, CancellationToken cancellationToken)
        {
            ILocalizable localizableRepository = repository as ILocalizable;
            TValue item = localizableRepository != null
                              ? repository.Cache.GetByIdentifier(identifier).SingleOrDefault(i => Equals(((ILocalizable)i).Culture, localizableRepository.Culture))
                              : repository.Cache.GetByIdentifier(identifier).SingleOrDefault();

            // To avoid boxing, the best way to compare generics for equality is with EqualityComparer<T>.Default.
            // This respects IEquatable<T> (without boxing) as well as object.Equals, and handles all the Nullable<T> "lifted" nuances.
            // See: http://stackoverflow.com/questions/65351/null-or-default-comparison-of-generic-argument-in-c-sharp
            if (!EqualityComparer<TValue>.Default.Equals(item, default(TValue)))
            {
                return item;
            }

            TValue apiItem = await ((IRepository<TDataContract, TValue>)repository).GetAsync(identifier, cancellationToken);

            repository.Cache.Add(identifier, item);

            return apiItem;
        }
That way the user initializes the repo with the converters and can query everything by repository.Get(someId)
sometimes he has to specify the types, but in most cases they can be inferred by the compiler
Steven Liekens
@StevenLiekens
Mar 09 2016 15:05
?lang is still per repository object instead of per request?
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:06
yep
Steven Liekens
@StevenLiekens
Mar 09 2016 15:06
does that sit well with you?
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:06
we simply postulate, that if an repository is localizable every item is localizable
each request still contains the lang parameter if it is supported
but the cache is an 1:n implementation
if the item is localizable we save multiple items per id
But I could change the methods to support per request Cultures
Steven Liekens
@StevenLiekens
Mar 09 2016 15:09
I think you should
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:10
However: What about queries which don’t support localisation? The extension method code should be the same for them too
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:15
Thought: To avoid a hacky solution, by slapping ?lang on every request, a repository still would have to implement ILocalizable. I could add an optional language parameter to the extension methods. If that parameter is != null we use that language, in every other case we use the language from the CultureInfoproperty on the repository
If the repository does not implement Ilocalizablewe simply ignore the parameter alltogether.
Does that sound acceptable?
Steven Liekens
@StevenLiekens
Mar 09 2016 15:31
I'm still not sure where is the right place to put it if not both
it's a value that is set per request (as opposed to being part of the baseUrl) so that makes me think it should be specified per request
but ultimately it is the endpoint that determines if ?lang is supported or not
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:33
yep
Steven Liekens
@StevenLiekens
Mar 09 2016 15:34
if we had /v2/:lang/items endpoints then clearly the language would be set per repository
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:35
But that would make the backend totally horrible… and it is a clusterfuck already
Steven Liekens
@StevenLiekens
Mar 09 2016 15:37
but since it's not, that pushes me towards having a lang parameter per request
it would also prevent some usability problems like when a user changes the repository Culture while there is still a task in the middle of a Get()
maybe causing the remainder of the items to have the wrong language
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:40
no it does not
not the way i coded it xD
at it should be that way
might need a unit test
Ok, it shouldn’t be that way, since we are creating a set of task before executing the first of them
This message was deleted
Steven Liekens
@StevenLiekens
Mar 09 2016 15:51
what if the Culture changes between creating the first and the last task?
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:52
that would be in a very small time, since creating the tasks takes almost no time. But to be sure we could get the culture when first entering the method and save it in a local variable
that should prevent this problem
Steven Liekens
@StevenLiekens
Mar 09 2016 15:53
mhm
what if the Culture changes between calling the method and saving the Culture to a local variable?
:)
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:54
then the user will get all requests in the new culture
Steven Liekens
@StevenLiekens
Mar 09 2016 15:54
the odds of that happening are probably measured in nanoseconds
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 15:54
yeah
irrelevant
Robert Logiewa
@Ruhrpottpatriot
Mar 09 2016 20:20
We had retries in the ServiceClient implementation
did we set a fixed number or could the user choose?