pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
f4e9e15618ae2b11ebcdd6777e88fc97a3f6ac12
odoo_source/addons/website/tests/test_website_visitor.py
T
Romain Derie d348bed1ad [IMP] website, *: use upsert to improve visitor perf 
* im_livechat, test_event_full, website_blog, website_crm,
  website_event, website_event_track, website_event_track_quiz,
  webite_livechat, website_sale

There is 6 main changes in this commit:

1. Using raw SQL Upsert instead of the ORM methods. While raw SQL should
generally be avoided, it makes sense for such a low level behavior which
is impacting every flows.
Indeed, tracking visitors is a generic behavior done on all pages and
controllers. It is important to optimize it to reduce processing time
and SQL Queries.
Benchmark of that change alone:
> Rendering a tracked page improves from ~19.5ms to ~17ms (using `ab`
  with 1000 loop) and the requests involved in the tracking process are
  reduced from 8 SQL Queries to 3:
  - 1 request to upsert the visitor
  - 1 request to fetch the visitor data
  - 1 request to add the tracking record

2. Adding in that upsert query the `visitor.track` insert, creating both
records in one go, bringing the query count from 3 to 2.

3. Refactoring of the `parent_id` behavior that was introduced in stable
with [1]. The purpose was to keep track of multiple visitor linked to a
same user to merge the tracking together. Especially useful for tracking
a same visitor on different devices (when logged in).
Only one visitor was kept as active, others would be archived and their
tracks would be set/moved to the main partner.
Removing those duplicate visitor was not possible because those archived
duplicated visitor were holding the devices notification push token.
Since [2], those token were moved to their own table, all related to the
main visitor.
We can then now safely remove those duplicate visitors after merging
their track to the main visitor. Thus, the `parent_id` field is no more
useful. Removing it removes a layer of complexity.
Note that thanks to this part, the `active` field can also be removed.

4. Deeper functionnal change, inspired from Plausible: The access_token
is no more stored in a cookie but is the result of a hashing method
based on <IP Adress, User Agent>.
The reason behind that change is that, in an upcoming refactoring,
sessions won't be stored anymore unless absolutely needed (login, add to
cart..). It will also ship a no cookies policy, trying to get rid of all
cookies.
This change is bringing some functional changes:
- Since the IP is included in the hash to generate the token, it means
  that:
  A. If an anonymous user switch IP (eg from 4G to wifi), it is
     considered as a new visitor.
  B. If 2 anonymous users with the exact same user agent (same browser,
     same browser version, same exact os or phone) are on the same IP,
     those will be considered as the same visitor.
- Since the request host is not included in the hash, it means that
  visiting a DB from 2 differents URLs (domain and/or ip) on the same
  device and same browser will result in a shared visitor.
  It shouldn't imply any issue as this is A. not wrong and B. mostly
  used for tests.
As all this is only related to non logged in user, it shouldn't be a
real issue as anonymous visitors are not supposed to be meant to be
business critical, even if we use them for "a bit more" than simple
analytics data.

5. The access_token is now replaced by the partner_id once the user logs
in, so:
- We don't need to either search on the partner_id field or the
access_token field (depending if the user is logged in or not), we can
only use the access_token row/field to do both.
- On logout, everything works out of the box as the access_token will be
regenerated since there is no partner_id anymore.
- On login, if an access_token matches the user's partner_id, that
visitor is returned.
If there is no such token, a new visitor is created for that partner_id.
In both 2 cases, tracks are moved to that visitor and the anonymous
visitor is removed.
- We can remove the code that was in charge of checking if the
access_token / visitor cookie was wrong (coming from another user eg,
different user login on same device). Indeed, such collision is not
possible anymore as the access_token automatically match the logged in
user.
- We can remove the code that was in charge of checking if the
access_token / visitor cookie was wrong (coming from a logged in user
while the current visitor is not loggedin). Such collision is not
possible anymore as the access_token is (re)generated as an anonymous
token (hash) when not logged in.

6. There is no more check to prevent a track to be created if there was
already a track for that URL in the last 30 minutes.
While this can easily be re-introduced (one CTE on the upsert), it was
adding ~100ms (from ~20 to ~110ms) to the request on a big database as
Odoo where there is ~100 millions tracks and ~100 millions visitors.
It has been validated that it was not a real issue as it is not
fundamentally wrong. If a visitor visited 20 times a product or a
specific page in that short amount of time, you might want to know that
because the user is most likely interested by it.

Changes (1+2), 3, (4+5) and 6 are all independant from each other and
could have existed on their own.

[1]: https://github.com/odoo/odoo/commit/c6b8a44b970a46dcd87a4e2cb1ad52fa340b209f
[2]: https://github.com/odoo/enterprise/pull/16781/commits/f75090fe8b42484e89e933976e8441d2f5eb9415

task-2867045

closes odoo/odoo#87857

Related: odoo/enterprise#28004
Related: odoo/upgrade#3566
Signed-off-by: Romain Derie (rde) <rde@odoo.com>
2022-06-07 16:31:20 +02:00

393 lines
17 KiB
Python
Raw Blame History

# coding: utf-8
# Part of Odoo. See LICENSE file for full copyright and licensing details.
import random
from contextlib import contextmanager
from datetime import datetime, timedelta
from unittest.mock import patch
from odoo.addons.base.tests.common import HttpCaseWithUserDemo
from odoo.addons.website.models.website_visitor import WebsiteVisitor
from odoo.tests import common, tagged
class MockVisitor(common.BaseCase):
@contextmanager
def mock_visitor_from_request(self, force_visitor=False):
def _get_visitor_from_request(model, *args, **kwargs):
return force_visitor
with patch.object(WebsiteVisitor, '_get_visitor_from_request',
autospec=True, wraps=WebsiteVisitor,
side_effect=_get_visitor_from_request) as _get_visitor_from_request_mock:
yield
@tagged('-at_install', 'post_install', 'website_visitor')
class WebsiteVisitorTests(MockVisitor, HttpCaseWithUserDemo):
def setUp(self):
super(WebsiteVisitorTests, self).setUp()
self.website = self.env['website'].search([
('company_id', '=', self.env.user.company_id.id)
], limit=1)
untracked_view = self.env['ir.ui.view'].create({
'name': 'UntackedView',
'type': 'qweb',
'arch': '''<t name="Homepage" t-name="website.base_view">
<t t-call="website.layout">
I am a generic page²
</t>
</t>''',
'key': 'test.base_view',
'track': False,
})
tracked_view = self.env['ir.ui.view'].create({
'name': 'TrackedView',
'type': 'qweb',
'arch': '''<t name="Homepage" t-name="website.base_view">
<t t-call="website.layout">
I am a generic page
</t>
</t>''',
'key': 'test.base_view',
'track': True,
})
tracked_view_2 = self.env['ir.ui.view'].create({
'name': 'TrackedView2',
'type': 'qweb',
'arch': '''<t name="OtherPage" t-name="website.base_view">
<t t-call="website.layout">
I am a generic second page
</t>
</t>''',
'key': 'test.base_view',
'track': True,
})
[self.untracked_page, self.tracked_page, self.tracked_page_2] = self.env['website.page'].create([
{
'view_id': untracked_view.id,
'url': '/untracked_view',
'website_published': True,
},
{
'view_id': tracked_view.id,
'url': '/tracked_view',
'website_published': True,
},
{
'view_id': tracked_view_2.id,
'url': '/tracked_view_2',
'website_published': True,
},
])
self.user_portal = self.env['res.users'].search([('login', '=', 'portal')])
self.partner_portal = self.user_portal.partner_id
if not self.user_portal:
self.env['ir.config_parameter'].sudo().set_param('auth_password_policy.minlength', 4)
self.partner_portal = self.env['res.partner'].create({
'name': 'Joel Willis',
'email': 'joel.willis63@example.com',
})
self.user_portal = self.env['res.users'].create({
'login': 'portal',
'password': 'portal',
'partner_id': self.partner_portal.id,
'groups_id': [(6, 0, [self.env.ref('base.group_portal').id])],
})
def _get_last_visitor(self):
return self.env['website.visitor'].search([], limit=1, order="id DESC")
def assertPageTracked(self, visitor, page):
""" Check a page is in visitor tracking data """
self.assertIn(page, visitor.website_track_ids.page_id)
self.assertIn(page, visitor.page_ids)
def assertVisitorTracking(self, visitor, pages):
""" Check the whole tracking history of a visitor """
for page in pages:
self.assertPageTracked(visitor, page)
self.assertEqual(
len(visitor.website_track_ids),
len(pages)
)
def assertVisitorDeactivated(self, visitor, main_visitor):
""" Method that checks that a visitor has been de-activated / merged
with other visitor, notably in case of login (see User.authenticate() as
well as Visitor._merge_visitor() ). """
self.assertFalse(visitor.exists(), "The anonymous visitor should be deleted")
self.assertTrue(visitor.website_track_ids < main_visitor.website_track_ids)
def test_visitor_creation_on_tracked_page(self):
""" Test various flows involving visitor creation and update. """
def authenticate(login, pwd):
# We can't call `self.authenticate` because that tour util is
# regenerating a new session.id before calling the real
# `authenticate` method.
# But we need the session id in the `authenticate` method because
# we need to retrieve the visitor before the authentication, which
# require the session id.
res = self.url_open('/web/login')
csrf_anchor = '<input type="hidden" name="csrf_token" value="'
self.url_open('/web/login', timeout=200, data={
'login': login,
'password': pwd,
'csrf_token': res.text.partition(csrf_anchor)[2].partition('"')[0],
})
existing_visitors = self.env['website.visitor'].search([])
existing_tracks = self.env['website.track'].search([])
self.url_open(self.untracked_page.url)
self.url_open(self.tracked_page.url)
self.url_open(self.tracked_page.url)
new_visitor = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
new_track = self.env['website.track'].search([('id', 'not in', existing_tracks.ids)])
self.assertEqual(len(new_visitor), 1, "1 visitor should be created")
self.assertEqual(len(new_track), 2, "There should be 2 tracked page")
self.assertEqual(new_visitor.visit_count, 1)
self.assertEqual(new_visitor.website_track_ids, new_track)
self.assertVisitorTracking(new_visitor, self.tracked_page + self.tracked_page)
# ------------------------------------------------------------
# Admin connects
# ------------------------------------------------------------
authenticate(self.user_admin.login, 'admin')
visitor_admin = new_visitor
# visit a page
self.url_open(self.tracked_page_2.url)
# check tracking and visitor / user sync
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(len(new_visitors), 1, "There should still be only one visitor.")
self.assertVisitorTracking(visitor_admin, self.tracked_page + self.tracked_page + self.tracked_page_2)
self.assertEqual(visitor_admin.partner_id, self.partner_admin)
self.assertEqual(visitor_admin.name, self.partner_admin.name)
# ------------------------------------------------------------
# Portal connects
# ------------------------------------------------------------
self.url_open('/web/session/logout')
authenticate(self.user_portal.login, 'portal')
self.assertFalse(
self.env['website.visitor'].search([('id', 'not in', (existing_visitors | visitor_admin).ids)]),
"No extra visitor should be created")
# visit a page
self.url_open(self.tracked_page.url)
self.url_open(self.untracked_page.url)
self.url_open(self.tracked_page_2.url)
# new visitor is created
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(len(new_visitors), 2, "One extra visitor should be created")
visitor_portal = new_visitors[0]
self.assertEqual(visitor_portal.partner_id, self.partner_portal)
self.assertEqual(visitor_portal.name, self.partner_portal.name)
self.assertVisitorTracking(visitor_portal, self.tracked_page | self.tracked_page_2)
# ------------------------------------------------------------
# Back to anonymous
# ------------------------------------------------------------
# portal user disconnects
self.url_open('/web/session/logout')
# visit some pages
self.url_open(self.tracked_page.url)
self.url_open(self.untracked_page.url)
self.url_open(self.tracked_page_2.url)
# new visitor is created
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(len(new_visitors), 3, "One extra visitor should be created")
visitor_anonymous = new_visitors[0]
self.assertFalse(visitor_anonymous.name)
self.assertFalse(visitor_anonymous.partner_id)
self.assertVisitorTracking(visitor_anonymous, self.tracked_page | self.tracked_page_2)
visitor_anonymous_tracks = visitor_anonymous.website_track_ids
# ------------------------------------------------------------
# Admin connects again
# ------------------------------------------------------------
authenticate(self.user_admin.login, 'admin')
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(new_visitors, visitor_admin | visitor_portal)
visitor_admin = self.env['website.visitor'].search([('partner_id', '=', self.partner_admin.id)])
# tracks are linked
self.assertTrue(visitor_anonymous_tracks < visitor_admin.website_track_ids)
self.assertEqual(len(visitor_admin.website_track_ids), 5, "There should be 5 tracked page for the admin")
# ------------------------------------------------------------
# Back to anonymous
# ------------------------------------------------------------
# admin disconnects
self.url_open('/web/session/logout')
# visit some pages
self.url_open(self.tracked_page.url)
self.url_open(self.untracked_page.url)
self.url_open(self.tracked_page_2.url)
# new visitor created
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(len(new_visitors), 3, "One extra visitor should be created")
visitor_anonymous_2 = new_visitors[0]
self.assertFalse(visitor_anonymous_2.name)
self.assertFalse(visitor_anonymous_2.partner_id)
self.assertVisitorTracking(visitor_anonymous_2, self.tracked_page | self.tracked_page_2)
visitor_anonymous_2_tracks = visitor_anonymous_2.website_track_ids
# ------------------------------------------------------------
# Portal connects again
# ------------------------------------------------------------
authenticate(self.user_portal.login, 'portal')
# one visitor is deleted
new_visitors = self.env['website.visitor'].search([('id', 'not in', existing_visitors.ids)])
self.assertEqual(new_visitors, visitor_admin | visitor_portal)
# tracks are linked
self.assertTrue(visitor_anonymous_2_tracks < visitor_portal.website_track_ids)
self.assertEqual(len(visitor_portal.website_track_ids), 4, "There should be 4 tracked page for the portal user")
# simulate the portal user comes back 30min later
for track in visitor_portal.website_track_ids:
track.write({'visit_datetime': track.visit_datetime - timedelta(minutes=30)})
# visit a page
self.url_open(self.tracked_page.url)
visitor_portal.invalidate_model(['website_track_ids'])
# tracks are created
self.assertEqual(len(visitor_portal.website_track_ids), 5, "There should be 5 tracked page for the portal user")
# simulate the portal user comes back 8hours later
visitor_portal.write({'last_connection_datetime': visitor_portal.last_connection_datetime - timedelta(hours=9)})
self.url_open(self.tracked_page.url)
visitor_portal.invalidate_model(['visit_count'])
# check number of visits
self.assertEqual(visitor_portal.visit_count, 2, "There should be 2 visits for the portal user")
def test_clean_inactive_visitors(self):
inactive_visitors = self.env['website.visitor'].create([{
'lang_id': self.env.ref('base.lang_en').id,
'country_id': self.env.ref('base.be').id,
'website_id': 1,
'last_connection_datetime': datetime.now() - timedelta(days=8),
'access_token': 'f9d2b14b21be669518b14a9590cb62ed',
}, {
'lang_id': self.env.ref('base.lang_en').id,
'country_id': self.env.ref('base.be').id,
'website_id': 1,
'last_connection_datetime': datetime.now() - timedelta(days=15),
'access_token': 'f9d2d261a725da7f596574ca84e52f47',
}])
active_visitors = self.env['website.visitor'].create([{
'lang_id': self.env.ref('base.lang_en').id,
'country_id': self.env.ref('base.be').id,
'website_id': 1,
'last_connection_datetime': datetime.now() - timedelta(days=1),
'access_token': 'f9d2526d9c15658bdc91d2119e54b554',
}, {
'lang_id': self.env.ref('base.lang_en').id,
'country_id': self.env.ref('base.be').id,
'website_id': 1,
'partner_id': self.partner_demo.id,
'last_connection_datetime': datetime.now() - timedelta(days=15),
'access_token': self.partner_demo.id,
}])
self._test_unlink_old_visitors(inactive_visitors, active_visitors)
def _test_unlink_old_visitors(self, inactive_visitors, active_visitors):
""" This method will test that the visitors are correctly deleted when inactive.
- inactive_visitors: all visitors that should be unlinked by the CRON
'_cron_unlink_old_visitors'
- active_visitors: all visitors that should NOT be cleaned because they are either active
or have some important data linked to them (partner, ...) and we want to keep them.
We use this method as a private tool so that sub-module can also test the cleaning of visitors
based on their own sets of conditions. """
WebsiteVisitor = self.env['website.visitor']
self.env['ir.config_parameter'].sudo().set_param('website.visitor.live.days', 7)
# ensure we keep a single query by correct usage of "not inselect"
# (+1 query to fetch the 'ir.config_parameter')
with self.assertQueryCount(2):
WebsiteVisitor.search(WebsiteVisitor._inactive_visitors_domain())
inactive_visitor_ids = inactive_visitors.ids
active_visitor_ids = active_visitors.ids
WebsiteVisitor._cron_unlink_old_visitors()
if inactive_visitor_ids:
# all inactive visitors should be deleted
self.assertFalse(bool(WebsiteVisitor.search([('id', 'in', inactive_visitor_ids)])))
if active_visitor_ids:
# all active visitors should be kept
self.assertEqual(active_visitors, WebsiteVisitor.search([('id', 'in', active_visitor_ids)]))
def test_link_to_visitor(self):
""" Visitors are 'linked' together when the user, previously not connected, authenticates
and the system detects it already had a website.visitor for that partner_id.
This can happen quite often if the user switches browsers / hardwares.
When 'linking' visitors together, the new visitor is archived and all its relevant data is
merged within the main visitor. See 'website.visitor#_merge_visitor' for more details.
This test ensures that all the relevant data are properly merged.
We build this logic with sub-methods so that sub-modules can easily add their own data and
test that they are correctly merged."""
[main_visitor, linked_visitor] = self.env['website.visitor'].create([
self._prepare_main_visitor_data(),
self._prepare_linked_visitor_data()
])
linked_visitor._merge_visitor(main_visitor)
self.assertVisitorDeactivated(linked_visitor, main_visitor)
def _prepare_main_visitor_data(self):
return {
'lang_id': self.env.ref('base.lang_en').id,
'country_id': self.env.ref('base.be').id,
'website_id': 1,
'access_token': self.partner_admin.id,
'website_track_ids': [(0, 0, {
'page_id': self.tracked_page.id,
'url': self.tracked_page.url
})]
}
def _prepare_linked_visitor_data(self):
return {
'lang_id': self.env.ref('base.lang_en').id,
'country_id': self.env.ref('base.be').id,
'website_id': 1,
'access_token': '%032x' % random.randrange(16**32),
'website_track_ids': [(0, 0, {
'page_id': self.tracked_page_2.id,
'url': self.tracked_page_2.url
})]
}
Reference in New Issue View Git Blame Copy Permalink