{"action":"created","issue":{"url":"https://api.github.com/repos/hypothesis/h/issues/2413","labels_url":"https://api.github.com/repos/hypothesis/h/issues/2413/labels{/name}","comments_url":"https://api.github.com/repos/hypothesis/h/issues/2413/comments","events_url":"https://api.github.com/repos/hypothesis/h/issues/2413/events","html_url":"https://github.com/hypothesis/h/pull/2413","id":97998920,"number":2413,"title":"URI normalisation","user":{"login":"nickstenning","id":3602,"avatar_url":"https://avatars.githubusercontent.com/u/3602?v=3","gravatar_id":"","url":"https://api.github.com/users/nickstenning","html_url":"https://github.com/nickstenning","followers_url":"https://api.github.com/users/nickstenning/followers","following_url":"https://api.github.com/users/nickstenning/following{/other_user}","gists_url":"https://api.github.com/users/nickstenning/gists{/gist_id}","starred_url":"https://api.github.com/users/nickstenning/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nickstenning/subscriptions","organizations_url":"https://api.github.com/users/nickstenning/orgs","repos_url":"https://api.github.com/users/nickstenning/repos","events_url":"https://api.github.com/users/nickstenning/events{/privacy}","received_events_url":"https://api.github.com/users/nickstenning/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2015-07-29T18:22:37Z","updated_at":"2015-08-06T12:38:14Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/hypothesis/h/pulls/2413","html_url":"https://github.com/hypothesis/h/pull/2413","diff_url":"https://github.com/hypothesis/h/pull/2413.diff","patch_url":"https://github.com/hypothesis/h/pull/2413.patch"},"body":"Add URI normalisation behind a feature flag\r\n\r\nThis is the first step in a major cleanup of how we internally normalise\r\nURIs referenced by annotations. The URIs in question divide into two\r\ngroups:\r\n\r\n1. URIs stored on annotations which refer to the annotated resource.\r\n\r\n2. URIs discovered in the annotated resource or elsewhere, which can be\r\n   used as hints to tell us which other URIs refer to the same\r\n   underlying document (e.g. to alternate formats of the annotated\r\n   resource).\r\n\r\nWe use both sets of these URIs when a user visits a page and activates\r\nHypothesis:\r\n\r\n- the client sends a search request, which includes the URI of the page\r\n  on which it was activated.\r\n- the server must find all annotations made on that page, but *also* all\r\n  annotations made on alternate versions of the page, and return these\r\n  to the client.\r\n\r\nGetting this right is both important and hard.\r\n\r\nIt's important because poor implementation will result in a poor\r\nexperience of using Hypothesis. Users don't (usually) want to see\r\nannotations from documents other than the one they're viewing, and users\r\n(usually) don't want any of the annotations on the page they're viewing\r\nto be missing.\r\n\r\nIt's hard because in simple terms, a user expects that when they visit\r\nthe \"same page,\" they will see the same annotations. But \"same page\" is\r\nnot a well-defined concept on the web.\r\n\r\nIndeed, one of the so-called \"axioms of web architecture\" is the\r\n[opacity of URIs](http://www.w3.org/DesignIssues/Axioms.html#opaque),\r\nwhich means that a priori, there are only a limited number of\r\nnormalisations of a URI which are guaranteed to preserve reference.\r\n\r\nAt the time of writing, the standard which defines permitted\r\nnormalisations is RFC 3987[1], AKA the \"Internationalized Resource\r\nIdentifiers (IRIs)\" RFC.\r\n\r\n[1]: https://tools.ietf.org/html/rfc3987#section-5\r\n\r\nDealing correctly with internationalised identifiers is something we\r\nwill probably need to do in due course. For now, dealing correctly with\r\nURIs will be a substantial improvement on the status quo. IRIs won't be\r\nreferred to again. The relevant standard which defines permitted\r\nnormalisations of URIs is RFC3986[2].\r\n\r\n[2]: https://tools.ietf.org/html/rfc3986#section-6\r\n\r\nIt's worth reading the above if you're trying to understand the starting\r\npoint for this code, but it's also worth understanding that in reality,\r\nwe have to be a lot more \"heuristic\" than these documents allow if we\r\nwant to ensure a good user experience. The \"opacity of URIs\", sadly,\r\nmust go out the window (for URLs, at least) if we want to provide a good\r\nexperience with Hypothesis on the real web.\r\n\r\nFor example, in practice, it may be the case that lexically sorting\r\nURL query parameters is almost always a permissible (and thus\r\nexperience-improving) normalisation. But such an operation is clearly\r\nnot permitted by the URI normalisation specifications.\r\n\r\nSo, overall, here's the approach taken by this commit:\r\n\r\n- We introduce the concept of a URI normalisation, which maps an input\r\n  URI idempotently to a normalised version of that URI.\r\n\r\n- When storing new annotations, we store normalised versions of the\r\n  'source' property of targets (that identify the annotated document).\r\n\r\n- When searching via the API:\r\n\r\n  + the value associated with the \"uri\" keyword, if passed, is used to\r\n    look up alternate URIs.[^1]\r\n\r\n  + Together, the query URI and alternate URIs returned from the\r\n    previous step form a set for searching.\r\n\r\n  + Each of the URIs in the search set are normalised.\r\n\r\n  + The annotation database is queried to find those annotations where\r\n    the normalised target source field is an exact match for any of the\r\n    resulting set of normalised URIs.\r\n\r\n- We continue to store, unmodified, the URIs as passed by the client, so\r\n  that we can, if necessary, alter the URI normalisation operation in\r\n  the future.\r\n\r\nA later commit will introduce a mechanism to regenerate the normalised\r\nfields in the search index to facilitate both release of this feature in\r\na deployment with legacy annotations lacking such fields, but also the\r\nability to later update the URI normalisation procedure.\r\n\r\nThe addition of normalised fields to annotations is performed\r\nunconditionally, but the alternate search procedure is guarded by a\r\nfeature flag.\r\n\r\n[^1]: Ideally we would look up such \"URI equivalences\" using a\r\nnormalised URI, but this is complicated by the implementation of\r\n`annotator-store` at the moment, so we don't yet do so.\r\n\r\nFixes #2240.\r\nAffects (but doesn't necessarily fix) #2217. (See comments later)."},"comment":{"url":"https://api.github.com/repos/hypothesis/h/issues/comments/128350631","html_url":"https://github.com/hypothesis/h/pull/2413#issuecomment-128350631","issue_url":"https://api.github.com/repos/hypothesis/h/issues/2413","id":128350631,"user":{"login":"nickstenning","id":3602,"avatar_url":"https://avatars.githubusercontent.com/u/3602?v=3","gravatar_id":"","url":"https://api.github.com/users/nickstenning","html_url":"https://github.com/nickstenning","followers_url":"https://api.github.com/users/nickstenning/followers","following_url":"https://api.github.com/users/nickstenning/following{/other_user}","gists_url":"https://api.github.com/users/nickstenning/gists{/gist_id}","starred_url":"https://api.github.com/users/nickstenning/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nickstenning/subscriptions","organizations_url":"https://api.github.com/users/nickstenning/orgs","repos_url":"https://api.github.com/users/nickstenning/repos","events_url":"https://api.github.com/users/nickstenning/events{/privacy}","received_events_url":"https://api.github.com/users/nickstenning/received_events","type":"User","site_admin":false},"created_at":"2015-08-06T12:38:14Z","updated_at":"2015-08-06T12:38:14Z","body":"I've addressed most of the outstanding issues. I've also done a little bit of analysis to understand how this is going to affect the existing database of annotations -- see [the comparative dataset of unique URLs before/after normalisation](https://hypothes-is.slack.com/files/nick/F08MUJE02/urls.csv) (this is deliberately not available to people outside the Hypothesis organisation because it's from the full dataset, not public-only annotations).\r\n\r\nOne possible concern I have is around dropping fragments. As it happens we [already do this client-side](https://github.com/hypothesis/h/blob/master/h/static/scripts/annotator/guest.coffee#L106) and so this PR doesn't change behaviour there, but it certainly seems to lead to a subpar experience on pages such as these: \r\n\r\n- http://gap2.alexandriaarchive.org/gapvis/index.html#book/1/read/16\r\n- http://gap2.alexandriaarchive.org/gapvis/index.html#book/1/read/788/142\r\n- http://gap2.alexandriaarchive.org/gapvis/index.html#book/1/read/80\r\n\r\nWe have a load of the above pages annotated in our database, and at the moment we're basically loading all the annotations for the entire site on every page...\r\n\r\nFor now I think it's safe enough to merge this and we can test out and try different normalisation strategies before committing and flipping this on for everyone."}}