-
Notifications
You must be signed in to change notification settings - Fork 3
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
include download stats #14
base: master
Are you sure you want to change the base?
Changes from all commits
38d4137
dd70952
6e19fc6
8266aa6
9651dda
a3f239b
68b4197
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
using System; | ||
using System.Reflection; | ||
using Dapper; | ||
using Microsoft.AspNetCore.Authorization; | ||
|
@@ -70,8 +71,8 @@ public async Task<IActionResult> Plugins( | |
}; | ||
await using var conn = await ConnectionFactory.Open(); | ||
// This query probably doesn't have right indexes | ||
var rows = await conn.QueryAsync<(string plugin_slug, int[] ver, string settings, long id, string manifest_info, string build_info)>( | ||
$"SELECT lv.plugin_slug, lv.ver, p.settings, b.id, b.manifest_info, b.build_info FROM {getVersions}(@btcpayVersion, @includePreRelease) lv " + | ||
var rows = await conn.QueryAsync<(string plugin_slug, int[] ver, string settings, long id, string manifest_info, string build_info, long download_stat)>( | ||
$"SELECT lv.plugin_slug, lv.ver, p.settings, b.id, b.manifest_info, b.build_info, lv.download_stat FROM {getVersions}(@btcpayVersion, @includePreRelease) lv " + | ||
"JOIN builds b ON b.plugin_slug = lv.plugin_slug AND b.id = lv.build_id " + | ||
"JOIN plugins p ON b.plugin_slug = p.slug " + | ||
"WHERE b.manifest_info IS NOT NULL AND b.build_info IS NOT NULL " + | ||
|
@@ -88,6 +89,7 @@ public async Task<IActionResult> Plugins( | |
var v = new PublishedVersion | ||
{ | ||
ProjectSlug = r.plugin_slug, | ||
DownloadStat = r.download_stat, | ||
Version = string.Join('.', r.ver), | ||
BuildId = r.id, | ||
BuildInfo = JObject.Parse(r.build_info), | ||
|
@@ -106,24 +108,65 @@ public async Task<IActionResult> Download( | |
[ModelBinder(typeof(PluginVersionModelBinder))] PluginVersion version) | ||
{ | ||
await using var conn = await ConnectionFactory.Open(); | ||
var url = await conn.ExecuteScalarAsync<string?>( | ||
"SELECT b.build_info->>'url' FROM versions v " + | ||
|
||
var result = await conn.QueryFirstOrDefaultAsync<(string? Url, long BuildId)>( | ||
"SELECT b.build_info->>'url' AS Url, v.build_id AS BuildId " + | ||
"FROM versions v " + | ||
"JOIN builds b ON b.plugin_slug = v.plugin_slug AND b.id = v.build_id " + | ||
"WHERE v.plugin_slug=@plugin_slug AND v.ver=@version", | ||
new | ||
{ | ||
plugin_slug = pluginSlug.ToString(), | ||
version = version.VersionParts | ||
}); | ||
if (url is null) | ||
|
||
if (result.Url is null) | ||
return NotFound(); | ||
|
||
await conn.InsertEvent("Download", new JObject | ||
var clientIp = HttpContext.Connection.RemoteIpAddress?.ToString(); | ||
|
||
var lastEventType = await conn.GetLastEventTypeForIpAsync(clientIp, pluginSlug); | ||
var eventInsertionResponse = await conn.InsertEvent("Download", new JObject | ||
{ | ||
["pluginSlug"] = pluginSlug.ToString(), | ||
["version"] = version.ToString() | ||
}); | ||
return Redirect(url); | ||
}, clientIp, pluginSlug, result.BuildId); | ||
if (eventInsertionResponse || lastEventType?.ToLower() == "uninstall") | ||
{ | ||
await conn.RecordPluginDownloadStatistics(pluginSlug, "install", version.VersionParts); | ||
} | ||
return Redirect(result.Url); | ||
} | ||
|
||
|
||
[AllowAnonymous] | ||
[HttpGet("plugins/{pluginSlug}/versions/{version}/uninstall")] | ||
public async Task<IActionResult> Uninstall( | ||
[ModelBinder(typeof(PluginSlugModelBinder))] PluginSlug pluginSlug, | ||
[ModelBinder(typeof(PluginVersionModelBinder))] PluginVersion version) | ||
{ | ||
await using var conn = await ConnectionFactory.Open(); | ||
var buildId = await conn.QuerySingleOrDefaultAsync<long>( | ||
"SELECT build_id FROM versions WHERE plugin_slug = @plugin_slug AND ver = @version", | ||
new | ||
{ | ||
plugin_slug = pluginSlug.ToString(), | ||
version = version.VersionParts | ||
}); | ||
|
||
var clientIp = HttpContext.Connection.RemoteIpAddress?.ToString(); | ||
var lastEventType = await conn.GetLastEventTypeForIpAsync(clientIp, pluginSlug); | ||
var eventInsertionResponse = await conn.InsertEvent("Uninstall", new JObject | ||
{ | ||
["pluginSlug"] = pluginSlug.ToString(), | ||
["version"] = version.ToString() | ||
}, clientIp, pluginSlug, buildId); | ||
|
||
if (eventInsertionResponse || lastEventType?.ToLower() == "download") | ||
{ | ||
await conn.RecordPluginDownloadStatistics(pluginSlug, "delete", version.VersionParts); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. both branch same? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have merge the else with the original 'if' for both scenario |
||
} | ||
return NoContent(); | ||
} | ||
|
||
[HttpPost("plugins/{pluginSlug}/builds")] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,46 @@ | ||
ALTER TABLE versions ADD COLUMN download_stat BIGINT NOT NULL DEFAULT 0; | ||
|
||
ALTER TABLE evts | ||
ADD COLUMN plugin_slug TEXT, | ||
ADD COLUMN build_id BIGINT, | ||
ALTER COLUMN type TYPE VARCHAR(16), | ||
ADD COLUMN id BIGINT GENERATED ALWAYS AS IDENTITY PRIMARY KEY, | ||
ADD COLUMN ip VARCHAR(45), | ||
ADD CONSTRAINT unique_event UNIQUE (ip, plugin_slug, type); | ||
|
||
CREATE INDEX idx_evts_plugin_slug ON evts(plugin_slug); | ||
CREATE INDEX idx_evts_type ON evts(type); | ||
CREATE INDEX idx_evts_build_id ON evts(build_id); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might not be core for now, but should incase we would need to run queries/filtering on the event type it would come a good to have What do you think? |
||
|
||
DROP FUNCTION IF EXISTS get_latest_versions(btcpayVersion INT[], includePreRelease BOOLEAN); | ||
DROP FUNCTION IF EXISTS get_all_versions(btcpayVersion INT[], includePreRelease BOOLEAN); | ||
|
||
|
||
CREATE OR REPLACE FUNCTION get_latest_versions (btcpayVersion INT[], includePreRelease BOOLEAN) | ||
RETURNS TABLE(plugin_slug TEXT, ver INT[], build_id BIGINT, download_stat BIGINT) | ||
AS $$ | ||
WITH latest_versions AS | ||
( | ||
SELECT plugin_slug, MAX(ver) ver FROM versions | ||
WHERE (btcpayVersion IS NULL OR btcpay_min_ver <= btcpayVersion) AND (includePreRelease OR pre_release IS FALSE) | ||
GROUP BY plugin_slug | ||
) | ||
SELECT v.plugin_slug, v.ver, v.build_id, v.download_stat FROM latest_versions lv | ||
JOIN versions v USING (plugin_slug, ver) | ||
|
||
$$ LANGUAGE SQL STABLE; | ||
|
||
|
||
CREATE OR REPLACE FUNCTION get_all_versions (btcpayVersion INT[], includePreRelease BOOLEAN) | ||
RETURNS TABLE(plugin_slug TEXT, ver INT[], build_id BIGINT) | ||
RETURNS TABLE(plugin_slug TEXT, ver INT[], build_id BIGINT, download_stat BIGINT) | ||
AS $$ | ||
WITH latest_versions AS | ||
( | ||
SELECT plugin_slug, ver FROM versions | ||
WHERE (btcpayVersion IS NULL OR btcpay_min_ver <= btcpayVersion) AND (includePreRelease OR pre_release IS FALSE) | ||
ORDER BY plugin_slug, ver DESC | ||
) | ||
SELECT v.plugin_slug, v.ver, v.build_id | ||
SELECT v.plugin_slug, v.ver, v.build_id, v.download_stat | ||
FROM latest_versions lv | ||
JOIN versions v USING (plugin_slug, ver) $$ LANGUAGE SQL STABLE; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is no different from the other branch of the if? what is the intent here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on Rockstar proposed changes:
Only increment download_stat in versions.download_stat if it’s a fresh insert.
A case scenario that played in my head was that:
"If a user install a plugin, the download_stat would increase, and if the user uninstalls the same plugin the stat would reduce.
If the user reinstalls the same plugin, the download stats should pop back up.. "
So what I am trying to achieve on this endpoint is to check if the last event the user (ip) did on that plugin was an uninstall that would mean the user is trying to re-install the plugin. I am then increasing the download_stat by one