Refactor #729

asbjornvad · 2025-08-19T15:10:13Z

This is my attempt at refactoring the RiotSharp api wrapper with a more separated approach(and up to date). The goal is that each game, be it LoL, Valorant and LoR, has their own Nuget package, and use the base functionality provided in the Core package.

The very basic stuff as API-ratelimiting is implemented and seems to work. Things like method ratelimiting and caching is not implemented yet, but should be "easy" to add, once done. This branch is NOT ready!!!

The testing is also done in each project, so the Core functionality is tested with no regards to actual implementations.

Please note that this is the work of a software engineering student during sparetime, and there might be better approaches.
I hope that others will see this as a way to get the RiotSharp wrapper back to the top, and will contribute to that goal!

…effect.

All the Newtonsoft is now System.Text.Json. Some tests pass, some fail because of errors in JsonSerializer.

Starting refactoring of the ratelimiting and httprequesting.

…quester. Next task is trying to refactor some already working endpoints to work with the new requester.

…pplication ratelimiters use the IRateLimiter Interface since they fulfil the same duty. No base class for neither.

… This is simpler and the thread safety should be provided by the underlying implementation of MemoryCache. The number of tests are unsatisfactory as of now.

…ot of uncommented code to avoid build errors.

…hanged exception and Lease. Still needs some more stuff. A util method should be used to create and send requests to reduce code duplication.

Refactored `IRequester` to `IRiotRequester` across the codebase for improved clarity and alignment with Riot API domain-specific terminology. Updated static data endpoints, constructors, and method calls to reflect this change. Removed `summonerId`-based methods in favor of `puuid`, modernizing API usage. Deprecated the `ChampionMastery` class and related methods, introducing nullable return types and new methods in `IChampionMasteryEndpoint`. Added unit tests for Champion Mastery functionality using `Moq` and `NUnit`. Enhanced error handling, modularity, and code readability. Introduced `RateLimitedEndpointBase` for centralized rate-limiting and request handling. Updated project dependencies and improved test coverage for reliability. Appsettings file needs to be added in the test project root with an apikey.

For every TestFixture that shouldnt be run in CI, add the [Category("OnlyLocal")] At [TestFixture].

Needs cleanup

…into refactor

Updated .gitignore to exclude appsettings.json files in test directories. Removed hardcoded `ApiKey` from appsettings.json to enhance security and prevent sensitive information from being stored in version control.

Introduced Champion Mastery API with new endpoint, interface, and models to retrieve mastery data, scores, and top champions. Refactored namespaces to improve organization and updated exception handling for better error messages. Enhanced test coverage with a new test project targeting .NET 8.0 and added unit tests for Champion Mastery functionality. Adjusted visibility of JSON converters for external use and updated solution/project files to support new features.

… them. Use the Builder class to build.

Champion rotation endpoint is basically a copy paste of the old.

…ave Models folder.

…hange to the Champion mastery endpoint.

…nough to justify more work.

It has the implemented Match endpoint but it has no tests as of yet... A lot of nullable and required fields that might be wrong, the docs are pretty bad at specifying what is and is not in the json. Removed 2 txt files. Updated the Core project to nuget capability. So the Lol and account project references nuget instead of project.

… then makes a new batch of nuget packages.

asbjornvad · 2026-01-20T20:40:02Z

Notes:

Account project should be complete.
Core has a working requester and ApplicationRateLimiter. Still needs support for caching, MethodRateLimiter and possibly return of HttpResponse instead of desirialized content. Also needs support for RSO (Riot Sign On?).
Most of the LoL endpoints are implemented. However the official documentation is lacking when it comes to which fields are always returned and which are only sometimes. Therefore the "required" and "nullable" properties should be reconsidered. Tests pass, but other calls might fail.
I have dabbled with github actions and have two workflows. One that runs on pushes(build and test) and one that runs on created releases(push nuget). Push nuget tests the build to verify, and then pushes the 3 nuget packages to nuget.org(see AV.RiotSharp.Account, Core and LeagueOfLegends).

The overall architecture is differs slightly from the monolithic RiotAPI approach the current RiotSharp project uses.

BenFradet · 2026-01-21T10:16:53Z

thanks 👍 , do you consider the pr ready for review now?

asbjornvad · 2026-01-21T10:22:04Z

If the notes above are taken into account then yes.

…rated TestObject just to test other than string.

BenFradet

Great effort! 💪
A couple of notes:

try to refrain from swearing in the code 😅
try to minimize code duplication (e.g. around the converters and the ToApiString() functions)
try to remove commented out code

src/RiotSharp.Account/Endpoints/AccountEndpoint.cs

src/RiotSharp.Account/RiotAccount.cs

src/RiotSharp.Core.Tests/CacheTests/InMemoryCacheTest.cs

src/RiotSharp.Core/Caching/InMemoryCache.cs

BenFradet · 2026-01-25T20:58:48Z

src/RiotSharp.LeagueOfLegends/Endpoints/LeagueEndpoint/Enums/Converters/DivisionConverter.cs

+				"II" => Division.II,
+				"III" => Division.III,
+				"IV" => Division.IV,
+				_ => throw new JsonException("Invalid division value")


seems like some converters have a default value, some throw
maybe we should align them on an approach

BenFradet · 2026-01-25T20:59:10Z

src/RiotSharp.LeagueOfLegends/Endpoints/LeagueEndpoint/Enums/Converters/TierConverter.cs

+        {
+            serializer.Serialize(writer, ((Tier)value).ToString().ToUpper());
+        }
+        */


let's remove

BenFradet · 2026-01-25T21:00:22Z

src/RiotSharp.LeagueOfLegends/Endpoints/LeagueEndpoint/Enums/Converters/QueueConverter.cs

+			string stringValue = value switch
+			{
+				Queue.RankedSolo5x5 => "RANKED_SOLO_5x5",
+				Queue.RankedTeam3x3 => "RANKED_TEAM_3x3",
+				Queue.RankedTeam5x5 => "RANKED_TEAM_5x5",
+				Queue.TeamBuilderDraftRanked5x5 => "TEAM_BUILDER_DRAFT_RANKED_5x5",
+				Queue.TeamBuilderDraftUnranked5x5 => "TEAM_BUILDER_DRAFT_UNRANKED_5x5",
+				Queue.RankedFlexSR => "RANKED_FLEX_SR",
+				Queue.RankedFlexTT => "RANKED_FLEX_TT",
+				Queue.TeamBuilderRankedSolo => "TEAM_BUILDER_RANKED_SOLO",
+				Queue.RankedPremade3x3 => "RANKED_PREMADE_3x3",
+				Queue.RankedPremade5x5 => "RANKED_PREMADE_5x5",
+				Queue.Darkstar => "DARKSTAR_3x3",
+				_ => "RANKED_SOLO_5x5"
+			};


couldn't we leverage Queue.ToApiString()?

BenFradet · 2026-01-25T21:02:47Z

src/RiotSharp.LeagueOfLegends/Endpoints/MatchEndpoint/Models/EventsTimeLine.cs

+{
+    /// <summary>
+    /// Timeline event data.
+    /// The documentation is fucking pisspoor!!! LIKE WTF Riot???


Suggested change

/// The documentation is fucking pisspoor!!! LIKE WTF Riot???

/// The documentation is bad!!! LIKE WTF Riot???

BenFradet · 2026-01-25T21:06:55Z

src/RiotSharp.LeagueOfLegends/Misc/Converters/QueueConverter.cs

seems like this is empty

… Next i would want to do the same for the RateLimiter, but that is a different beast...

…lection stuff found in the .net source repo.

asbjornvad · 2026-01-26T21:12:20Z

Thanks for the review. I have started fixing and will comment or resolve along the way.
I really appreciate that you take the time, this is a great learning experience for me(:

asbjornvad and others added 30 commits June 23, 2024 15:21

Changes

d5e548f

Changes

d2fbe75

More changes

01c8f2e

All sorts of changes. Comments or changes that actually does have an …

22b5d7b

…effect.

Unloaded ASP and old version of RiotSharp.

7418ac1

All the Newtonsoft is now System.Text.Json. Some tests pass, some fail because of errors in JsonSerializer.

Removed internal constructors in all the "JSON" classes

ee8a336

Outcomment yml-files

d99aae2

Deleted Build.Props as i dont think its needed

0da6325

Changes to some of the Misc files. Removed some warnings here and there.

7304f90

Starting refactoring of the ratelimiting and httprequesting.

Done work on the RequesterBase, ApplicationRateLimiter, RateLimitedRe…

632f7d9

…quester. Next task is trying to refactor some already working endpoints to work with the new requester.

Started work on methodratelimiter but not yet done. Both Method and A…

51371ff

…pplication ratelimiters use the IRateLimiter Interface since they fulfil the same duty. No base class for neither.

Trying a new workflow

3a8bb6a

Unloaded old testproject

43a3805

Changes to solution. Hopefully the old projects are no longer build.

ce1237c

The Cache is now called InMemoryCache and uses IMemoryCache to cache.…

a0cd7a6

… This is simpler and the thread safety should be provided by the underlying implementation of MemoryCache. The number of tests are unsatisfactory as of now.

Moved requesters and unloaded several of the old implementations. A l…

0b7e5af

…ot of uncommented code to avoid build errors.

Initial convertion of endpoints to use the new RateLimiting system. C…

6d18fa1

…hanged exception and Lease. Still needs some more stuff. A util method should be used to create and send requests to reduce code duplication.

Possibly fixed the pipeline.

d5daa23

For every TestFixture that shouldnt be run in CI, add the [Category("OnlyLocal")] At [TestFixture].

Small changes. Removed unused files.

d89b3de

Moved everything to src folder like it always should have been.

8b8d63f

Update dotnet.yml

090e98d

Added most of the files needed in Core.

9aea2d0

Needs cleanup

Merge branch 'refactor' of https://github.com/asbjornvad/RiotSharpFun …

6a9d60b

…into refactor

Update .gitignore and remove hardcoded API key

86e82d1

Updated .gitignore to exclude appsettings.json files in test directories. Removed hardcoded `ApiKey` from appsettings.json to enhance security and prevent sensitive information from being stored in version control.

Trying local nuget stuff

f774ce3

Trying to fix broken pipeline

31a0a69

Still trying to fix the pipeline

35cfdbe

asbjornvad added 19 commits July 5, 2025 22:01

Implemented classes that hold the endpoints and are the way to access…

827c645

… them. Use the Builder class to build.

Refactored the testfixture and is now a shared collection for all tests.

0edce3d

Champion rotation endpoint is basically a copy paste of the old.

Implemented Clash endpoint and refactored some namespaces.

2abc4dc

League endpoint implemented and initial testing written.

c522274

Access to all endpoints in LeagueEndpoint

c11fa78

Namespace fix and outputpath for local nuget package.

586ee59

Starting work on ChallengesEndpoint. Restructure other endpoints to h…

3d9dc6a

…ave Models folder.

Challenges endpoint done. minor changes to the langauge enum. Minor c…

d46a82c

…hange to the Champion mastery endpoint.

Status endpoint implemented. Crude but i dont think it will be used e…

bc7fd81

…nough to justify more work.

Unfinished Match endpoint. Wow riot, fix your docs man!!!

844dafb

Pipeline fix

0ec77d5

Testing the pipeline with real api key secret.

bb8c479

Unloaded old projects

d4eb3fb

Test of package cache

b7217f5

Caching requires lock.json so not viable

2e50ba8

More actions stuff. This should now allow me to create a release that…

53ca544

… then makes a new batch of nuget packages.

Should be sequential now.

4f91381

Removed old unused stuff.

8942fd8

asbjornvad marked this pull request as ready for review January 22, 2026 09:30

asbjornvad added 3 commits January 22, 2026 22:37

New action that is for single package push to nuget.

382c7dd

Github actions fix.

93ebc4e

Changes to the cache interface, and updated the tests. Random AI-gene…

8b6f13d

…rated TestObject just to test other than string.

BenFradet reviewed Jan 25, 2026

View reviewed changes

asbjornvad added 2 commits January 26, 2026 21:06

Added TODO implement RSO. Added a FakeClock for testing in the cache.…

91ac708

… Next i would want to do the same for the RateLimiter, but that is a different beast...

RateLimiter test now completes in <1 second thanks to some freaky ref…

eaa8306

…lection stuff found in the .net source repo.

	/// The documentation is fucking pisspoor!!! LIKE WTF Riot???
	/// The documentation is bad!!! LIKE WTF Riot???

Refactor #729

Are you sure you want to change the base?

Refactor #729

Uh oh!

Conversation

asbjornvad commented Aug 19, 2025

Uh oh!

asbjornvad commented Jan 20, 2026

Uh oh!

BenFradet commented Jan 21, 2026

Uh oh!

asbjornvad commented Jan 21, 2026

Uh oh!

BenFradet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BenFradet Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

BenFradet Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

BenFradet Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

BenFradet Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

BenFradet Jan 25, 2026

Choose a reason for hiding this comment

Uh oh!

asbjornvad commented Jan 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants