Skip to content

Commit

Permalink
Merge pull request #196 from pi-hole/fix/desc_sorting
Browse files Browse the repository at this point in the history
Improve sorting and displaying algorithms for top-clients and top-domains
  • Loading branch information
AzureMarker authored Jan 11, 2018
2 parents f6adb83 + ff6258f commit 561c55e
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 44 deletions.
81 changes: 44 additions & 37 deletions request.c
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,13 @@ void getTopDomains(char *client_message, int *sock)
{
char server_message[SOCKETBUFFERLEN];
int i, temparray[counters.domains][2], count=10, num;
bool blocked = command(client_message, ">top-ads"), audit = false, desc = false;
bool blocked = command(client_message, ">top-ads"), audit = false, asc = false;

// Exit before processing any data if requested via config setting
if(!config.query_display)
return;


// Match both top-domains and top-ads
if(sscanf(client_message, ">%*[^(](%i)", &num) > 0)
{
Expand All @@ -349,9 +350,9 @@ void getTopDomains(char *client_message, int *sock)
}

// Sort in descending order?
if(command(client_message, " desc"))
if(command(client_message, " asc"))
{
desc = true;
asc = true;
}

for(i=0; i < counters.domains; i++)
Expand All @@ -366,10 +367,10 @@ void getTopDomains(char *client_message, int *sock)
}

// Sort temporary array
if(desc)
qsort(temparray, counters.domains, sizeof(int[2]), cmpdesc);
else
if(asc)
qsort(temparray, counters.domains, sizeof(int[2]), cmpasc);
else
qsort(temparray, counters.domains, sizeof(int[2]), cmpdesc);


// Get filter
Expand Down Expand Up @@ -406,43 +407,40 @@ void getTopDomains(char *client_message, int *sock)
}
}

int skip = 0;
for(i=0; i < min(counters.domains, count+skip); i++)
int n = 0;
for(i=0; i < counters.domains; i++)
{
// Get sorted indices
int j = temparray[counters.domains-i-1][0];
int j = temparray[i][0];
validate_access("domains", j, true, __LINE__, __FUNCTION__, __FILE__);

// Skip this domain if there is a filter on it
if(excludedomains != NULL)
{
if(insetupVarsArray(domains[j].domain))
{
skip++;
continue;
}
}
if(excludedomains != NULL && insetupVarsArray(domains[j].domain))
continue;

// Skip this domain if already included in audit
if(audit && countlineswith(domains[j].domain, files.auditlist) > 0)
{
skip++;
continue;
}

if(blocked && showblocked && domains[j].blockedcount > 0)
{
if(audit && domains[j].wildcard)
sprintf(server_message,"%i %i %s wildcard\n",i,domains[j].blockedcount,domains[j].domain);
sprintf(server_message,"%i %i %s wildcard\n", n, domains[j].blockedcount, domains[j].domain);
else
sprintf(server_message,"%i %i %s\n",i,domains[j].blockedcount,domains[j].domain);
sprintf(server_message,"%i %i %s\n", n ,domains[j].blockedcount, domains[j].domain);
swrite(server_message, *sock);
n++;
}
else if(!blocked && showpermitted && (domains[j].count - domains[j].blockedcount) > 0)
{
sprintf(server_message,"%i %i %s\n",i,(domains[j].count - domains[j].blockedcount),domains[j].domain);
sprintf(server_message,"%i %i %s\n", n, (domains[j].count - domains[j].blockedcount), domains[j].domain);
swrite(server_message, *sock);
n++;
}

// Only count entries that are actually sent and return when we have send enough data
if(n > count)
break;
}
if(excludedomains != NULL)
clearSetupVarsArray();
Expand Down Expand Up @@ -482,8 +480,18 @@ void getTopClients(char *client_message, int *sock)
temparray[i][1] = clients[i].count;
}

// Sort in ascending order?
bool asc = false;
if(command(client_message, " asc"))
{
asc = true;
}

// Sort temporary array
qsort(temparray, counters.clients, sizeof(int[2]), cmpasc);
if(asc)
qsort(temparray, counters.clients, sizeof(int[2]), cmpasc);
else
qsort(temparray, counters.clients, sizeof(int[2]), cmpdesc);

// Get clients which the user doesn't want to see
char * excludeclients = read_setupVarsconf("API_EXCLUDE_CLIENTS");
Expand All @@ -494,31 +502,30 @@ void getTopClients(char *client_message, int *sock)
logg("Excluding %i clients from being displayed", setupVarsElements);
}

int skip = 0;
for(i=0; i < min(counters.clients, count+skip); i++)
int n = 0;
for(i=0; i < counters.clients; i++)
{
// Get sorted indices
int j = temparray[counters.clients-i-1][0];
int j = temparray[i][0];
validate_access("clients", j, true, __LINE__, __FUNCTION__, __FILE__);

// Skip this client if there is a filter on it
if(excludeclients != NULL)
{
if(insetupVarsArray(clients[j].ip) ||
insetupVarsArray(clients[j].name))
{
skip++;
continue;
}
}
if(excludeclients != NULL && (insetupVarsArray(clients[j].ip) || insetupVarsArray(clients[j].name)))
continue;

// Return this client if either
// - "withzero" option is set, and/or
// - the client made at least one query within the most recent 24 hours
if(includezeroclients || clients[j].count > 0)
{
sprintf(server_message,"%i %i %s %s\n",i,clients[j].count,clients[j].ip,clients[j].name);
sprintf(server_message,"%i %i %s %s\n", n, clients[j].count, clients[j].ip, clients[j].name);
swrite(server_message, *sock);
n++;
}

// Only count entries that are actually sent and return when we have send enough data
if(n > count)
break;
}
if(excludeclients != NULL)
clearSetupVarsArray();
Expand Down
44 changes: 37 additions & 7 deletions test/test_suite.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ load 'libs/bats-support/load'
[[ ${lines[11]} == "---EOM---" ]]
}

@test "Top Clients" {
@test "Top Clients (descending, default)" {
run bash -c 'echo ">top-clients" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
Expand All @@ -41,23 +41,53 @@ load 'libs/bats-support/load'
[[ ${lines[4]} == "---EOM---" ]]
}

@test "Top Domains" {
@test "Top Clients (ascending)" {
run bash -c 'echo ">top-clients asc" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} =~ "0 1 10.8.0.2" ]]
[[ ${lines[2]} =~ "1 2 127.0.0.1" ]]
[[ ${lines[3]} =~ "2 4 192.168.2.208" ]]
[[ ${lines[4]} == "---EOM---" ]]
}

@test "Top Domains (descending, default)" {
run bash -c 'echo ">top-domains" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} == "0 2 play.google.com" ]]
[[ ${lines[2]} == "1 1 example.com" ]]
[[ ${lines[2]} == "1 1 raspberrypi" ]]
[[ ${lines[3]} == "2 1 checkip.dyndns.org" ]]
[[ ${lines[4]} == "3 1 raspberrypi" ]]
[[ ${lines[4]} == "3 1 example.com" ]]
[[ ${lines[5]} == "---EOM---" ]]
}

@test "Top Domains (ascending)" {
run bash -c 'echo ">top-domains asc" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} == "0 1 raspberrypi" ]]
[[ ${lines[2]} == "1 1 checkip.dyndns.org" ]]
[[ ${lines[3]} == "2 1 example.com" ]]
[[ ${lines[4]} == "3 2 play.google.com" ]]
[[ ${lines[5]} == "---EOM---" ]]
}

@test "Top Ads" {
@test "Top Ads (descending, default)" {
run bash -c 'echo ">top-ads" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} == "0 1 addomain.com" ]]
[[ ${lines[2]} == "1 1 blacklisted.com" ]]
[[ ${lines[1]} == "0 1 blacklisted.com" ]]
[[ ${lines[2]} == "1 1 addomain.com" ]]
[[ ${lines[3]} == "---EOM---" ]]
}

@test "Top Ads (ascending)" {
run bash -c 'echo ">top-ads asc" | nc -v 127.0.0.1 4711'
echo "output: ${lines[@]}"
[[ ${lines[0]} == "Connection to 127.0.0.1 4711 port [tcp/*] succeeded!" ]]
[[ ${lines[1]} == "0 1 blacklisted.com" ]]
[[ ${lines[2]} == "1 1 addomain.com" ]]
[[ ${lines[3]} == "---EOM---" ]]
}

Expand Down

0 comments on commit 561c55e

Please sign in to comment.