bancho.py icon indicating copy to clipboard operation
bancho.py copied to clipboard

Using api to submit a score and the functions around it

Open TrueRou opened this issue 2 years ago • 9 comments

What the function is: Upload a score with (or without) replay using our api

What is the function for:

  1. Server staff will have the ability to handle player's wishings to submit a score (network failure scores, other server scores for example)
  2. We have already made a admin panel page in guweb to submit the score with (or without) replay

What I did:

  1. Using fastapi Dependency Injection to authorized the player and its permission scope
  2. Add the post method to handle score submission
  3. Add a sql table (scores_foreign) to keep a record on the foreign scores (which will help to identify these manually uploaded scores in someday in the future)
  4. Add a middleware to fix CORS problems instead of nginx add-header

Why do we need scores_foreign table: Any foreign external scores we submitted is not passing by osu-submit-modular-selector, which means they can't be checked whether it is legal in a very powerful way. So the record is necessary to be saved in order to check these scores (by osuapi or by the source server api of the score) in the future.

Why I use middleware instead of nginx add-header: api_key need to be stored in the header of the post request if needs authorization. Because of safety and security, the post request with a authorization in its header can not access when Access-Control-Allow-Origin is * QQ-20220814164552.jpg Middleware can solve it gracefully

What else: I edited the migrations.sql but the version (3.6.1) maybe not fitted. Change it if required

TrueRou avatar Aug 14 '22 08:08 TrueRou

imo there's no point in this, as if for example the player's connection was lost, the score would submit again if they kept their game open

7ez avatar Aug 14 '22 09:08 7ez

imo there's no point in this, as if for example the player's connection was lost, the score would submit again if they kept their game open

true but some other servers like ezppfarm planned on adding a feature where players can transfer scores from other servers but rate limited to 1 per month. could be smth cool to add on other servers too, seems like the demand is there.

minisbett avatar Aug 14 '22 10:08 minisbett

Hi, this is a required feature to be compatible with our frontend fork (which has a lot of new features and fixes).

we can implement some feature availability checks in the future if this feature is not desired by a server's choice.

arily avatar Aug 25 '22 08:08 arily

Hi, this is a required feature to be compatible with our frontend fork (which has a lot of new features and fixes).

we can implement some feature availability checks in the future if this feature is not desired by a server's choice.

is there any reason why you cannot just add this to your fork and just do upkeep when necessary? features like this are specialised for specific usecases and i don't see the appeal in having it as a core feature

g1-1-1 avatar Aug 25 '22 08:08 g1-1-1

Hi, this is a required feature to be compatible with our frontend fork (which has a lot of new features and fixes).

we can implement some feature availability checks in the future if this feature is not desired by a server's choice.

is there any reason why you cannot just add this to your fork and just do upkeep when necessary? features like this are specialised for specific usecases and i don't see the appeal in having it as a core feature

We had a conversation with cmyui and we are willing to be the new maintainer of guweb. The fork will become official once it's compatible with Bancho.py.

Thus we opened these prs. We cannot leave a feature which only works with our custom server.

Cmyui agreed on adding this feature and it's each server's own responsibility to use it or not.

arily avatar Aug 25 '22 09:08 arily

hm i'm okay adding this but i'm not too much a fan of the extra scores table

i think i'd rather have something like adding a column to the existing tables to show where the score was submitted from, and perhaps we could add a column to store admin notes for a score?

thoughts?

cmyui avatar Aug 30 '22 04:08 cmyui

If most of the time(I mean more than 99.999% of the scores) will not be added in this way, add a new column and pre-allocate the columns for saving the data feels a bit wasteful.

For a relational database it's seems perfectly fine to me. Use joins or create a view against scores and the newly added table should be the way relational databases are designed.

I am not by any means an expert in sql so it's just my thought. If our database was in nosql you will be absolutely right.

arily avatar Aug 30 '22 05:08 arily

yes it’s relational but it doesn’t mean each method of submission should be in a separate database, it’s more wasteful than columns imo as it means every score-related query would require an (extra) join to get the custom submitted scores.

don’t see the point in using a table over columns

tsunyoku avatar Aug 30 '22 08:08 tsunyoku

Ahhh sorry I misread - @tsunyoku the scores table is still used, this is just an extra table for tracking the foreign scores - we wouldn't need to join the table generally as they're still inserted into scores if you read the code

my bad

cmyui avatar Aug 30 '22 13:08 cmyui

I tend to believe that this PR is no longer in the right direction now that APIv2 is a thing and the upcoming authorization changes.

NiceAesth avatar Jan 13 '23 21:01 NiceAesth