4
\$\begingroup\$

I've created a unique page counter using a PHP script. Pages could be videos, images, publications, etc.

Every time a new user views a page, I record the entry and increment the view counter.

I cannot increment the counter for the same user until 24 hours have passed.

The way I build it is by using the user IP address (I’ve also tried using cookies and sessions). I have 2 tables: one table that records the page type, page id, user IP address and the date visited, and another table that contains the final counter for that page. Like so:

page_view table:

CREATE TABLE `page_view` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `page_type` varchar(25) COLLATE utf8mb4_unicode_520_ci NOT NULL,
  `page_type_id` int(11) unsigned NOT NULL,
  `ip_address` varchar(45) COLLATE utf8mb4_unicode_520_ci NOT NULL,
  `date` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
  PRIMARY KEY (`id`),
  UNIQUE KEY `page_type_page_type_id_ip_address` (`page_type`,`page_type_id`,`ip_address`) USING BTREE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_520_ci;

page_view_counter:

CREATE TABLE `page_view_counter` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `page_type` varchar(25) COLLATE utf8mb4_unicode_520_ci NOT NULL,
  `page_type_id` int(11) unsigned NOT NULL,
  `views` int(11) NOT NULL DEFAULT '0',
  PRIMARY KEY (`id`),
  UNIQUE KEY `page_type_page_type_id` (`page_type`,`page_type_id`) USING BTREE
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_520_ci;

Here's my PHP code:

$page_type = $_POST['type'];
$page_type_id = (int)$_POST['typeId'];
$ip_address = $_SERVER['REMOTE_ADDR']; // retrieve the visitor's IP address

// check if the visitor visited the page
$q = "SELECT
            pv.page_type pvpagetype,
            pv.page_type_id pvpagetypeid,
            pv.ip_address pvipaddress,
            pv.date pvdate
        FROM
            page_view pv
        WHERE
            pv.page_type = ?
            AND pv.page_type_id = ?
            AND pv.ip_address = ?";

$result = fn_select_query($conn, $q);

// if visitor visited the page, make sure if enough time has passed since the last visit (24 hours)
if ($result->num_rows >= 1) {
    $rows = $result->fetch_all(MYSQLI_ASSOC);

    // get the current time as a DateTime object
    $current_date = new DateTime();

    // convert the db date to a DateTime object
    $last_visited_date = new DateTime($rows[0]['pvdate']);

    // calculate the diff between the current date and the last visited date
    $diff = $current_date->diff($last_visited_date);

    // calculate the total number of hours in the diff
    $interval = ($diff->days * 24) + $diff->h;

    // check if enough time has passed since the last visit (24 hours)
    if ($interval > 24) {
        // update the database with the current timestamp
        $q = "UPDATE
                    page_view pv
                SET
                    pv.date = ?
                WHERE
                    pv.page_type = ?
                    AND pv.page_type_id = ?
                    AND pv.ip_address = ?";

        fn_prepared_query($conn, $q);

        // incremented page views
        $q = "UPDATE
                    page_view_counter pvc
                SET
                    pvc.views = pvc.views + 1
                WHERE
                    pvc.page_type = ?
                    AND pvc.page_type_id = ?";

        fn_prepared_query($conn, $q);
    }
// visitor is visiting for the first time!
} else {
    $q = "INSERT INTO `page_view` (page_type, page_type_id, ip_address)
        VALUES (?, ?, ?)",

    // execute the query
    fn_prepared_query($conn, $q);

    // incremented page views
    $q = "INSERT INTO `page_view_counter` (page_type, page_type_id, views)
            VALUES (?, ?, 1) ON DUPLICATE KEY UPDATE views = views + 1",

    // execute the query
    fn_prepared_query($conn, $q);
}

The logic is simple:

  1. Check whether the visitor visited the page.
  2. If yes, then make sure 24 hours has passed before incrementing the counter.
  3. If visitor never visited, add a record with the user IP and the date and increment the counter.

The only thing missing is making sure the page_view table doesn't grow too big. I guess a function that deletes records that are, let's say, 60 days old.

Is there something I can improve? logic, code...?

\$\endgroup\$
6
  • 2
    \$\begingroup\$ How does fn_prepared_query() work? How does it know, what variable to bind? \$\endgroup\$ Jul 31, 2023 at 6:09
  • \$\begingroup\$ In addition to a code review, you might also want to get a law review to make sure you are actually allowed to store that information without explicit consent (IP addresses of consumer internet connections are personally identifiable information). \$\endgroup\$
    – Philipp
    Jul 31, 2023 at 12:02
  • \$\begingroup\$ @YourCommonSense it's a function that executes the query and returns the result set. I updated the question to add the full code. \$\endgroup\$
    – Marco
    Jul 31, 2023 at 13:59
  • \$\begingroup\$ Just using IP you won't distinguish between multiple people using the same company/university/etc internet \$\endgroup\$
    – Jimmy T.
    Jul 31, 2023 at 13:59
  • 1
    \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. \$\endgroup\$
    – pacmaninbw
    Jul 31, 2023 at 14:21

3 Answers 3

4
\$\begingroup\$

This critique is mostly about the DB schema, rather than the code.


Every time a new user views a page, I record the entry and increment the view counter.

Good.

The PV table records historical truths. Someone issued a GET request and got a 200 response. Very simple. We have confidence in those bits.

I cannot increment the counter for the same user until 24 hours have passed.

Now here is an item you might wish to finesse, in the interest of taking advantage of SQL queries. Consider doing a GROUP BY count query on: ip, day. And call that your "daily uniques". I'll grant you that it potentially overcounts by a factor of two, if all users do a GET just before midnight and another GET just after midnight. But it might be "good enough", it might be indicative of traffic trends.

Alternatively you might wish to produce a temporary reporting table based on GROUP BY ip, hour. This can still over report w.r.t. a strict 24-hour rule, but it does much better. And it offers distilled results, a much smaller number of rows than the raw page views. You may find it is more convenient to do reporting on daily uniques based on the hourly figures than on the raw logs.


Painted in broad strokes, here's some DB philosophy about recording events and reporting on them.

Keep them separate.

In a very simple append-only PageView table store one row per event. That's pretty much what you have now. Each row records something that is clearly true, making very few assumptions, and needing no fancy locking. It's unlikely to change when product requirements change.

Every hour or week or whatever, INSERT rows in a reporting table that summarize the raw rows using COUNT(*) in a GROUP BY query, and probably mentioning start / end timestamps. Dashboard apps can efficiently query that smaller amount of distilled rows. You are always free to DELETE reporting rows and re-create them, perhaps because you fixed a bug or because the Product Manager now wants to view weekly uniques instead of daily ones.

If you want to e.g. start a 24-hour timer upon seeing a new unique user, I recommend doing that at reporting time rather than in an online way. If some errant client wedges while holding a writer lock, you certainly wouldn't want all clients to hang waiting for it just to log that an event happened. I have seen that happen to production sites, and it isn't pretty. It's something we can design out of our solution from the very beginning.


I confess that I have no idea what your tables model.

The id PKs don't seem necessary, as compound PKs would suffice.

The PV table has a compound PK on (page_type, page_type_id, ip_address). I just don't understand how that could be, so probably I'm not understanding what page type and type ID are all about. Typically ip_address would be an attribute of a GET which requested some url at timestamp. Maybe page_type, page_type_id together express the notion of a web url.

The PV counter table seems to be just the output of a GROUP BY query, which ignores timestamps? I'm sure it's a useful reporting table, but it seems orthogonal to the OP question.


These queries seem quite similar to the common theme of a web "session", where often an active user session is defined as ending when we see it idle for at least 30 minutes.

Consider using a self JOIN to identify "initial" rows that have > 24 hours of preceding idle for the given IP. With such a reporting table in hand, you'd be in a better position to come up with a "picky" definition of an IP's GETs that fall into a 24-hour bucket, customized per-IP. It's not clear to me that you need to be that picky, though. It will be a more expensive query.


            pv.page_type pvpagetype,

Rather than gratuitously eliding _ underscores, I recommend you spell these aliases pv_page_type and similarly. The word boundaries may seem obvious to the author. But if you want to hire a maintenance engineer whose mother tongue is not your own, you may find that there are advantages to being explicit about word boundaries.

\$\endgroup\$
2
  • \$\begingroup\$ I appreciate your feedback. I'm going to make some changes coz you’re right. I don't need an id PK and there is no need for a compound PK (page_type, page_type_id, ip_address). I’ll keep the compound PK in page_view_counter coz I need when I use ON DUPLICATE KEY UPDATE views = views + 1. Also, I'm using the page_type, page_type_id to track different objects. For example, an image with an id 356, or a video with an id 183 or a publication with an id 42 One table to track unique views for a images, videos, publication, etc.instead of having multiple tables for each object. \$\endgroup\$
    – Marco
    Jul 31, 2023 at 4:09
  • \$\begingroup\$ Forgot to mention, the PHP script counter runs with ajax so i don't interrupt the web page loading. \$\endgroup\$
    – Marco
    Jul 31, 2023 at 13:54
3
\$\begingroup\$

Your code should make your intentions clear

if ($result->num_rows >= 1) ... $result->fetch_all implies you are expecting multiple rows and loop over the whole result set. If I understand your idea correctly the select will at most return a single row (unique key on the query-parameters) So your code should reflect that: Query only for a single row and don't loop over the results.

Think about race conditions

If your application runs multi-threaded, always expect multiple requests to hit your application in parallel. What happens if two requests from the same IP address (multiple browser tabs, multiple users behind the same NAT, ...) hit your script at the same time? Your script will either throw errors and crash (unique key violation) or it might increase the counter multipile times in a single second for a single ip-address.

Databases are great at synchronization. You can run multiple queries in parallel and will always get consistent results. But this consistency is only guaranteed for a single transaction. If you run single statements without a transaction the database might change between those statements. This is a good use-case for "INSERT OR UPDATE" statements or "JOINED UPDATES" over multiple tables. The logic "only increment page_view if interval > 24" should be executed in a single SQL transaction, to guarantee consistency.

Model your use cases and make sure they fit

An IP address can be good approximation for unique users, but depending on the context it can also be really bad. For example in universities and company-networks usually all users will share a single external IP over NAT. So if these are your primary users, the page counter will vastly underrepresent the number of visits. Depending on how complex your algorithm should be you could use a mix of various methods (search term: fingerprinting) to make a good guess if views come from a single user. This can include a mix of IP-address, http headers, browser features, cookies, cache-policies, ...

And always think about how likely someone will want to attack your system and fake profile views (not very likely for a small internal statistic, very likely if page-views have a monetary incentive)

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Hey, thanks! i like the joined updates idea. Going to research about fingerprinting and fix the $result->fetch_all. \$\endgroup\$
    – Marco
    Jul 31, 2023 at 13:52
  • \$\begingroup\$ Forgot to mention, the PHP script counter runs with ajax so i don't interrupt the web page loading. \$\endgroup\$
    – Marco
    Jul 31, 2023 at 13:54
2
\$\begingroup\$

The main point of interest here is updating the page_view_counter table. Which, to my understanding, has to be done unconditionally, thanks to that unique index and ON DUPLICATE query.

Regarding the page_view table, I am not sure if you need a timestamp column here, given you are only interested in the date and it records the time of only one hit per day out of many. And if you make the date column of date type, you will be able to add it to unique index and therefore use the same query for insert and update.

Also, I have some small suggestions:

  1. Starting from the version 8.2, PHP got native implementation of your fn_select_query/fn_prepared_query functions, called mysqli::execute_query()

  2. Current SQL seems a bit redundant. For example, only one value is going to be used out of four selected. And usually table and column aliases are used only with JOINS

  3. Not going to be used in the code, but you can move that rather lengthy date calculations into SQL, like shown below, which will already give a number < 1 (or false) if a page hasn't been visited today

     SELECT TO_DAYS(pvdate)-TO_DAYS(NOW()) FROM page_view ...
    

Yet I believe everything can be done without any date calculations, with two simple queries, if date column will be made into date type and added to the unique index:

$q = "INSERT INTO `page_view` (page_type, page_type_id, ip_address, CURDATE())
      VALUES (?, ?, ?) ON DUPLICATE KEY UPDATE date=date";
$conn->execute_query($q, [$page_type, $page_type_id, $ip_address]);

// incremented page views
$q = "INSERT INTO `page_view_counter` (page_type, page_type_id, views)
        VALUES (?, ?, 1) ON DUPLICATE KEY UPDATE views = views + 1",
$conn->execute_query($q, [$page_type, $page_type_id]);

I prefer that strangely looking date=date UPDATE over INSERT IGNORE because according to my experience, the latter ignores not only unique constraint violation but other errors as well.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.