From 39cf1af7998ad4416c5c44fbfcceee36a584f028 Mon Sep 17 00:00:00 2001 From: rubenwardy Date: Tue, 22 Oct 2024 22:31:56 +0100 Subject: [PATCH] Add in-client views counter to stats pages --- app/blueprints/api/endpoints.py | 11 +++++++++- app/blueprints/packages/releases.py | 7 ++++-- app/flatpages/help/api.md | 2 ++ app/logic/graphs.py | 5 +++-- app/models/packages.py | 25 ++++++++++++++++++++- app/public/static/js/package_charts.js | 10 +++++++++ app/rediscache.py | 7 +++++- app/templates/macros/stats.html | 8 ++++++- app/tests/unit/utils/test_version.py | 30 ++++++++++++++++++++++++++ app/utils/version.py | 29 +++++++++++++++++++++++++ migrations/versions/d52f6901b707_.py | 28 ++++++++++++++++++++++++ 11 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 app/tests/unit/utils/test_version.py create mode 100644 app/utils/version.py create mode 100644 migrations/versions/d52f6901b707_.py diff --git a/app/blueprints/api/endpoints.py b/app/blueprints/api/endpoints.py index 375bc51d..73e6566f 100644 --- a/app/blueprints/api/endpoints.py +++ b/app/blueprints/api/endpoints.py @@ -30,7 +30,7 @@ from app.logic.graphs import get_package_stats, get_package_stats_for_user, get_ from app.markdown import render_markdown from app.models import Tag, PackageState, PackageType, Package, db, PackageRelease, Permission, \ MinetestRelease, APIToken, PackageScreenshot, License, ContentWarning, User, PackageReview, Thread, Collection, \ - PackageAlias, Language + PackageAlias, Language, PackageDailyStats from app.querybuilder import QueryBuilder from app.utils import is_package_page, get_int_or_abort, url_set_query, abs_url, is_yes, get_request_date, cached, \ cors_allowed @@ -39,6 +39,7 @@ from . import bp from .auth import is_api_authd from .support import error, api_create_vcs_release, api_create_zip_release, api_create_screenshot, \ api_order_screenshots, api_edit_package, api_set_cover_image +from ...rediscache import make_view_key, set_temp_key, has_key @bp.route("/api/packages/") @@ -99,6 +100,14 @@ def package_view(package): @is_package_page @cors_allowed def package_view_client(package: Package): + ip = request.headers.get("X-Forwarded-For") or request.remote_addr + if ip is not None and (request.headers.get("User-Agent") or "").startswith("Minetest"): + key = make_view_key(ip, package) + if not has_key(key): + set_temp_key(key, "true") + PackageDailyStats.notify_view(package) + db.session.commit() + protocol_version = request.args.get("protocol_version") engine_version = request.args.get("engine_version") if protocol_version or engine_version: diff --git a/app/blueprints/packages/releases.py b/app/blueprints/packages/releases.py index d0625c6a..9bb99ff1 100644 --- a/app/blueprints/packages/releases.py +++ b/app/blueprints/packages/releases.py @@ -13,6 +13,7 @@ # # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . + import os from flask import render_template, request, redirect, flash, url_for, abort @@ -31,6 +32,7 @@ from app.rediscache import has_key, set_temp_key, make_download_key from app.tasks.importtasks import check_update_config from app.utils import is_user_bot, is_package_page, nonempty_or_none, normalize_line_endings from . import bp, get_package_tabs +from app.utils.version import is_minetest_v510 @bp.route("/packages///releases/", methods=["GET", "POST"]) @@ -127,9 +129,10 @@ def download_release(package, id): ip = request.headers.get("X-Forwarded-For") or request.remote_addr if ip is not None and not is_user_bot(): - is_minetest = (request.headers.get("User-Agent") or "").startswith("Minetest") + is_minetest = (request.headers.get("User-Agent") or "").startswith("Minetest/") + is_v510 = is_minetest and is_minetest_v510(request.headers.get("User-Agent")) reason = request.args.get("reason") - PackageDailyStats.update(package, is_minetest, reason) + PackageDailyStats.notify_download(package, is_minetest, is_v510, reason) key = make_download_key(ip, release.package) if not has_key(key): diff --git a/app/flatpages/help/api.md b/app/flatpages/help/api.md index a5e6e9dc..f5a7f499 100644 --- a/app/flatpages/help/api.md +++ b/app/flatpages/help/api.md @@ -157,6 +157,7 @@ curl -X DELETE https://content.minetest.net/api/delete-token/ \ * `reason_new`: list of integers per day. * `reason_dependency`: list of integers per day. * `reason_update`: list of integers per day. + * `views_minetest`: list of integers per day. * GET `/api/package_stats/` * Returns last 30 days of daily stats for _all_ packages. * An object with the following keys: @@ -437,6 +438,7 @@ Example: * `reason_new`: list of integers per day. * `reason_dependency`: list of integers per day. * `reason_update`: list of integers per day. + * `views_minetest`: list of integers per day. ## Topics diff --git a/app/logic/graphs.py b/app/logic/graphs.py index b464568a..6dc85fbc 100644 --- a/app/logic/graphs.py +++ b/app/logic/graphs.py @@ -28,7 +28,7 @@ def daterange(start_date, end_date): keys = ["platform_minetest", "platform_other", "reason_new", - "reason_dependency", "reason_update"] + "reason_dependency", "reason_update", "views_minetest"] def flatten_data(stats): @@ -78,7 +78,8 @@ def get_package_stats_for_user(user: User, start_date: Optional[datetime.date], func.sum(PackageDailyStats.platform_other).label("platform_other"), func.sum(PackageDailyStats.reason_new).label("reason_new"), func.sum(PackageDailyStats.reason_dependency).label("reason_dependency"), - func.sum(PackageDailyStats.reason_update).label("reason_update")) \ + func.sum(PackageDailyStats.reason_update).label("reason_update"), + func.sum(PackageDailyStats.views_minetest).label("views_minetest")) \ .filter(PackageDailyStats.package.has(author_id=user.id)) if start_date: diff --git a/app/models/packages.py b/app/models/packages.py index 271acf04..2720e387 100644 --- a/app/models/packages.py +++ b/app/models/packages.py @@ -1437,8 +1437,11 @@ class PackageDailyStats(db.Model): reason_dependency = db.Column(db.Integer, nullable=False, default=0) reason_update = db.Column(db.Integer, nullable=False, default=0) + views_minetest = db.Column(db.Integer, nullable=False, default=0) + v510 = db.Column(db.Integer, nullable=False, default=0) + @staticmethod - def update(package: Package, is_minetest: bool, reason: str): + def notify_download(package: Package, is_minetest: bool, is_v510: bool, reason: str): date = datetime.datetime.utcnow().date() to_update = dict() @@ -1462,6 +1465,26 @@ class PackageDailyStats(db.Model): to_update[field_reason] = getattr(PackageDailyStats, field_reason) + 1 kwargs[field_reason] = 1 + if is_v510: + to_update["v510"] = PackageDailyStats.v510 + 1 + kwargs["v510"] = 1 + + stmt = insert(PackageDailyStats).values(**kwargs) + stmt = stmt.on_conflict_do_update( + index_elements=[PackageDailyStats.package_id, PackageDailyStats.date], + set_=to_update + ) + + conn = db.session.connection() + conn.execute(stmt) + + @staticmethod + def notify_view(package: Package): + date = datetime.datetime.utcnow().date() + + to_update = {"views_minetest": PackageDailyStats.views_minetest + 1} + kwargs = {"package_id": package.id, "date": date, "views_minetest": 1} + stmt = insert(PackageDailyStats).values(**kwargs) stmt = stmt.on_conflict_do_update( index_elements=[PackageDailyStats.package_id, PackageDailyStats.date], diff --git a/app/public/static/js/package_charts.js b/app/public/static/js/package_charts.js index 378e4005..30c65de5 100644 --- a/app/public/static/js/package_charts.js +++ b/app/public/static/js/package_charts.js @@ -228,6 +228,16 @@ async function load_data() { }; new Chart(ctx, config); } + + { + const ctx = document.getElementById("chart-views").getContext("2d"); + const data = { + datasets: [ + { label: "Luanti", data: getData(json.views_minetest) }, + ], + }; + setup_chart(ctx, data, annotations); + } } diff --git a/app/rediscache.py b/app/rediscache.py index 4bf27004..4cbf448f 100644 --- a/app/rediscache.py +++ b/app/rediscache.py @@ -15,6 +15,7 @@ # along with this program. If not, see . from . import redis_client +from .models import Package # This file acts as a facade between the rest of the code and redis, # and also means that the rest of the code avoids knowing about `app` @@ -23,10 +24,14 @@ from . import redis_client EXPIRY_TIME_S = 2*7*24*60*60 # 2 weeks -def make_download_key(ip, package): +def make_download_key(ip: str, package: Package): return "{}/{}/{}".format(ip, package.author.username, package.name) +def make_view_key(ip: str, package: Package): + return "view/{}/{}/{}".format(ip, package.author.username, package.name) + + def set_temp_key(key, v): redis_client.set(key, v, ex=EXPIRY_TIME_S) diff --git a/app/templates/macros/stats.html b/app/templates/macros/stats.html index 91298380..a536febe 100644 --- a/app/templates/macros/stats.html +++ b/app/templates/macros/stats.html @@ -2,7 +2,7 @@ - + {% endmacro %} @@ -118,6 +118,12 @@ +

{{ _("Views inside Luanti") }}

+

+ {{ _("Number of package page views inside the Luanti client. v5.10 and later only.") }} +

+ +

{{ _("Need more stats?") }}

{{ _("Check out the ContentDB Grafana dashboard for CDB-wide stats") }} diff --git a/app/tests/unit/utils/test_version.py b/app/tests/unit/utils/test_version.py new file mode 100644 index 00000000..07d4da3b --- /dev/null +++ b/app/tests/unit/utils/test_version.py @@ -0,0 +1,30 @@ +# ContentDB +# Copyright (C) rubenwardy +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +from app.utils.version import is_minetest_v510 + + +def test_is_minetest_v510(): + assert not is_minetest_v510("Minetest/5.9.1 (Windows/10.0.22621 x86_64)") + assert not is_minetest_v510("Minetest/") + assert not is_minetest_v510("Minetest/5.9.1") + + assert is_minetest_v510("Minetest/5.10.0") + assert is_minetest_v510("Minetest/5.10.1") + assert is_minetest_v510("Minetest/5.11.0") + assert is_minetest_v510("Minetest/5.10") + + assert not is_minetest_v510("Minetest/6.12") diff --git a/app/utils/version.py b/app/utils/version.py new file mode 100644 index 00000000..7ebda18c --- /dev/null +++ b/app/utils/version.py @@ -0,0 +1,29 @@ +# ContentDB +# Copyright (C) rubenwardy +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + + +def is_minetest_v510(user_agent: str) -> bool: + parts = user_agent.split(" ") + version = parts[0].split("/")[1] + try: + digits = list(map(lambda x: int(x), version.split("."))) + except ValueError: + return False + + if len(digits) < 2: + return False + + return digits[0] == 5 and digits[1] >= 10 diff --git a/migrations/versions/d52f6901b707_.py b/migrations/versions/d52f6901b707_.py new file mode 100644 index 00000000..698b9f49 --- /dev/null +++ b/migrations/versions/d52f6901b707_.py @@ -0,0 +1,28 @@ +"""empty message + +Revision ID: d52f6901b707 +Revises: daa040b727b2 +Create Date: 2024-10-22 21:18:23.929298 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = 'd52f6901b707' +down_revision = 'daa040b727b2' +branch_labels = None +depends_on = None + + +def upgrade(): + with op.batch_alter_table('package_daily_stats', schema=None) as batch_op: + batch_op.add_column(sa.Column('views_minetest', sa.Integer(), nullable=False, server_default="0")) + batch_op.add_column(sa.Column('v510', sa.Integer(), nullable=False, server_default="0")) + + +def downgrade(): + with op.batch_alter_table('package_daily_stats', schema=None) as batch_op: + batch_op.drop_column('views_minetest') + batch_op.drop_column('v510')