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.

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

v5.0.0 - Overall project restructuring & refactoring #205

Closed
wants to merge 75 commits into from

Conversation

cmyui
Copy link
Member

@cmyui cmyui commented Mar 24, 2022

v5.0.0 - Overall project restructuring & refactoring

This one comes with quite a few updates and improvements to the codebase.
(implementing more of my learnings from time away)

decoupling of logic & data into separate (functional) api layers

One of the main issues facing the codebase is it's tight coupling of logic & data.

Let's take the Beatmap class as an example - it;

If you look through bancho.py's codebase, you'll see this recurring theme implemented in many inconsistent ways.
(see if you can find the similarities & differences in the Player class for example)

This PR aims to decouple structural representation, data providers, and business logic into 3 separate api levels.

data structures will be represented as "models"

Models represent the format of a resource.
They allow you to think only about the structure of a resource - the attributes within it and its properties.

An example of a model may be the representation of a beamtap - for example;

from dataclasses import dataclass

@dataclass
class Beatmap:
    id: int
    set_id: int
    md5: str
    ranked_status: int

    circle_size: float
    health_drain: float
    approach_rate: float
    overall_difficulty: float

(you may also define simple properties, but nothing involving complex logic or external data points)

data providers will be represented as "repositories"

Repositories are an abstraction around the interaction with a resource.
They allow you to think of the interactions with the data providing service, as opposed to the actual implementation details of the underlying services - proving a standardized API for your business-logic layers.

Continuing with the theme of beatmaps, a simple repository may look something like

from typing import Mapping
from typing import Optional

import models
import services

## in-memory cache

id_cache: Mapping[int, models.Beatmap] = {}
md5_cache: Mapping[str, models.Beatmap] = {}

## reads

async def fetch_by_id(id: int) -> Optional[models.Beatmap]:
    if beatmap := id_cache.get(id):
        return beatmap

    beatmap_row = await services.database.fetch_one(
        "SELECT * FROM beatmaps WHERE id = :id",
        {"id": id},
    )
    if beatmap_row is None:
        return None

    beatmap = models.Beatmap(**beatmap_row)

    id_cache[beatmap.id] = beatmap

    return beatmap

## writes

## updates

async def update_status(id: int, new_status: int) -> None:
    await services.database.execute(
        "UPDATE beatmaps SET status = :status WHERE id = :id",
        {"status": new_status, "id": id},
    )

    id_cache[id].status = new_status


async def update_playcount(id: int, playcount: int) -> None:
    await services.database.execute(
        "UPDATE beatmaps SET playcount = :playcount WHERE id = :id",
        {"playcount": playcount, "id": id},
    )

    id_cache[id].playcount = playcount

## deletes

async def delete_by_id(id: int) -> None:
    await services.database.execute(
        "DELETE FROM beatmaps WHERE id = :id",
        {"id": id},
    )

    if id in id_cache:
        del id_cache[id]

(you may also have functions to fetch by other means, update, or create beatmaps)

business logic will be represented by specific "usecases"

Usecases define the logical interface exposed by a resource.
They allow you to interact with & make complex calculations & modifications possibly depending on multiple resources simultaneously.

Building on our previous examples further, some use cases of a beatmap may look something like

import models
import repositories

async def update_status(beatmap: models.Beatmap, new_status: int) -> None:
    await repositories.beatmaps.update_status(beatmap.id, new_status)


async def update_playcount(beatmap: models.Beatmap) -> None:
    await repositories.beatmaps.update_playcount(beatmap.id, beatmap.plays)

Splitting the objects into layers like this allows us to implement a standardized API across the codebase's multiple resources.

With this sort of orthogonal design, we can increase code reuse significantly, increase the level of abstraction the programmer works at, and use this api as a framework for writing unit & smoke tests for the regular user flow in the application trivially.

restful api restructuring

This PR also resolves the paths provided by the v1 API to be restful -- we've updated the paths to fit a resource-driven design.

# GET /players/{player_id}
# GET /players/{player_id}/stats
# GET /players/{player_id}/status
# GET /players/{player_id}/scores/best
# GET /players/{player_id}/scores/recent
# GET /players/{player_id}/most-played-beatmaps
# GET /players/leaderboard

# GET /beatmaps/{beatmap_id}
# GET /beatmaps/{beatmap_id}/scores

# GET /scores/{score_id}
# GET /scores/{score_id}/replay

# GET /matches/{match_id}

assume developers will use cloudflare (flex) for ssl

This simplifies the server setup itself quite significantly - no need for an ssl certificate, complex nginx setup or anything - just listen on port 80 and have cloudflare do the rest - implemented here.

disclaimer

Note that the examples provided in this are simplifications, and the exact implementation details of the PR are still being actively worked on - nothing is final!

@cmyui cmyui added enhancement New feature or request code quality Something is implemented poorly labels Mar 24, 2022
@cmyui cmyui self-assigned this Mar 24, 2022
@cmyui cmyui marked this pull request as draft March 24, 2022 06:49
@cmyui cmyui changed the title splitting objects into (objects, usecases, repos) (1/?) move logic & data store management out of objects (models) Apr 16, 2022
@cmyui
Copy link
Member Author

cmyui commented Apr 18, 2022

the code is getting much nicer

.env.example Show resolved Hide resolved
@alowave223
Copy link
Member

alowave223 commented Apr 22, 2022

Absolutely love how's cmyui learning new things and me catchig them from him, good work and i really like this new api, really restfull and easy to understand, also the usecases makes feel code more consistant and logicaly sorted(?), i mean you can easy understand what file you need to edit for that module or function, very cool. Keep it up!

@cmyui cmyui force-pushed the code-cleanup-refactor branch from 5adb1e3 to 1030729 Compare July 5, 2022 02:23
@cmyui
Copy link
Member Author

cmyui commented Jul 5, 2022

hm the example i gave for beatmaps doesn't work so well in the usecase since the usecases i showed are just 1:1 with the repositories lol.. maybe will think of a better example - it's certainly not always like this

@cmyui
Copy link
Member Author

cmyui commented Jul 24, 2022

while this has helped with orienting my vision for the project more accurately - it's not worth the time to try finishing this at the moment

i'll slowly be merging the ideas explored in this pr into the master branch in smaller, more atomized parts over time - the motivations still hold strong

@cmyui cmyui closed this Jul 24, 2022
@cmyui cmyui deleted the code-cleanup-refactor branch July 24, 2022 07:05
@cmyui cmyui mentioned this pull request Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working code quality Something is implemented poorly documentation Improvements or additions to documentation enhancement New feature or request performance Improvements to resource usage without changing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Identify and refactor logic/data coupling issues (primarily in classmethods of app/objects classes)
6 participants