Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Commit

Permalink
Fix #2376, remove URL collection
Browse files Browse the repository at this point in the history
Adds and calculates shot.origin
Makes some logic that used shot.url conditional
Database migration to make data.url nullable
  • Loading branch information
ianb committed Mar 21, 2017
1 parent b5c828b commit a9a076a
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 13 deletions.
1 change: 0 additions & 1 deletion addon/webextension/selector/documentMetadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ window.documentMetadata = (function () {

return function documentMetadata() {
let result = {};
result.url = location.href;
result.docTitle = document.title;
result.siteName = findSiteName();
result.openGraph = getOpenGraph();
Expand Down
2 changes: 1 addition & 1 deletion addon/webextension/selector/shooter.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ window.shooter = (function () { // eslint-disable-line no-unused-vars
backend,
randomString(RANDOM_STRING_LENGTH) + "/" + domainFromUrl(location),
{
url: location.href
origin: window.shot.originFromUrl(location.href)
}
);
shot.update(documentMetadata());
Expand Down
1 change: 1 addition & 0 deletions server/db-patches/patch-14-15.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE data ALTER COLUMN url DROP NOT NULL;
1 change: 1 addition & 0 deletions server/db-patches/patch-15-14.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE data ALTER COLUMN url SET NOT NULL;
2 changes: 1 addition & 1 deletion server/src/dbschema.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const pgpatcher = require("pg-patcher");
const path = require("path");
const mozlog = require("mozlog")("dbschema");

const MAX_DB_LEVEL = exports.MAX_DB_LEVEL = 14;
const MAX_DB_LEVEL = exports.MAX_DB_LEVEL = 15;

exports.forceDbVersion = function (version) {
mozlog.info("forcing-db-version", {db: db.constr, version});
Expand Down
5 changes: 4 additions & 1 deletion server/src/pages/shot/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,10 @@ class Body extends React.Component {
<a className="block-button button secondary" href={ myShotsHref } onClick={this.onClickMyShots.bind(this)}>{ myShotsText }</a>
<div className="shot-info">
<EditableTitle title={shot.title} isOwner={this.props.isOwner} />
<div className="shot-subtitle">Saved from &nbsp;<a className="subtitle-link" href={ shotRedirectUrl } onClick={ this.onClickOrigUrl.bind(this, "navbar") }>{ linkTextShort }</a> <span className="clock-icon"/> { timeDiff } { expiresDiff } </div>
<div className="shot-subtitle">
{ linkTextShort ? <span>Saved from &nbsp;<a className="subtitle-link" href={ shotRedirectUrl } onClick={ this.onClickOrigUrl.bind(this, "navbar") }>{ linkTextShort }</a></span> : null }
<span className="clock-icon"/> { timeDiff } { expiresDiff }
</div>
</div>
</div>
<div className="more-shot-actions right">
Expand Down
9 changes: 6 additions & 3 deletions server/src/servershot.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,12 @@ class Shot extends AbstractShot {
oks.push({setHead: null});
oks.push({setBody: null});
let searchable = this._makeSearchableText(7);
let url = json.fullUrl || json.url || json.origin;
return db.queryWithClient(
client,
`INSERT INTO data (id, deviceid, value, url, title, searchable_version, searchable_text)
VALUES ($1, $2, $3, $4, $5, $6, ${searchable.query})`,
[this.id, this.ownerId, JSON.stringify(json), json.url, title, searchable.version].concat(searchable.args)
[this.id, this.ownerId, JSON.stringify(json), url, title, searchable.version].concat(searchable.args)
).then((rowCount) => {
return clipRewrites.commit(client);
}).then(() => {
Expand Down Expand Up @@ -245,9 +246,11 @@ class Shot extends AbstractShot {
}
queryParts.push(`setweight(to_tsvector(${addText(t)}), '${weight}') /* ${name} */`);
}
let domain = this.url.replace(/^.*:/, "").replace(/\/.*$/, "");
if (this.url) {
let domain = this.url.replace(/^.*:/, "").replace(/\/.*$/, "");
addWeight(domain, 'B', 'domain');
}
addWeight(this.title, 'A', 'title');
addWeight(domain, 'B', 'domain');
if (this.openGraph) {
let openGraphProps = `
site_name description
Expand Down
76 changes: 70 additions & 6 deletions shared/shot.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,42 @@ function isUrl(url) {
return (/^https?:\/\/[a-z0-9\.\-]+[a-z0-9](:[0-9]+)?\/?/i).test(url);
}

function assertUrl(url) {
if (! url) {
throw new Error("Empty value is not URL");
}
if (! isUrl(url)) {
let exc = new Error("Not a URL");
exc.scheme = url.split(":")[0];
throw exc;
}
}

function assertOrigin(url) {
assertUrl(url);
if (url.search(/^https?:/i) != -1) {
let match = (/^https?:\/\/[^/:]+\/?$/i).exec(url);
if (! match) {
throw new Error("Bad origin, might include path");
}
}
}

function originFromUrl(url) {
if (! url) {
return null;
}
if (url.search(/^https?:/i) == -1) {
// Non-HTTP URLs don't have an origin
return null;
}
let match = (/^https?:\/\/[^/:]+/i).exec(url);
if (match) {
return match[0];
}
return null;
}

/** Check if the given object has all of the required attributes, and no extra
attributes exception those in optional */
function checkObject(obj, required, optional) {
Expand Down Expand Up @@ -168,7 +204,12 @@ class AbstractShot {
assert((/^[a-zA-Z0-9]+\/[a-z0-9\.-]+$/).test(id), "Bad ID (should be alphanumeric):", JSON.stringify(id));
this._backend = backend;
this._id = id;
this.url = attrs.url;
this.origin = attrs.origin || null;
this.fullUrl = attrs.fullUrl || null;
if ((! attrs.fullUrl) && attrs.url) {
console.warn("Received deprecated attribute .url");
this.fullUrl = attrs.url;
}
this.docTitle = attrs.docTitle || null;
this.userTitle = attrs.userTitle || null;
this.createdDate = attrs.createdDate || Date.now();
Expand Down Expand Up @@ -281,11 +322,30 @@ class AbstractShot {
}

get url() {
return this._url;
return this.fullUrl || this.origin;
}
set url(val) {
assert(val && isUrl(val), "Bad URL:", val);
this._url = val;
throw new Error(".url is read-only");
}

get fullUrl() {
return this._fullUrl;
}
set fullUrl(val) {
if (val) {
assertUrl(val);
}
this._url = val || undefined;
}

get origin() {
return this._origin;
}
set origin(val) {
if (val) {
assertOrigin(val);
}
this._origin = val || undefined;
}

get filename() {
Expand All @@ -304,6 +364,9 @@ class AbstractShot {
}

get urlDisplay() {
if (! this.url) {
return null;
}
if (this.url.search(/^https?/i) != -1) {
let txt = this.url;
txt = txt.replace(/^[a-z]+:\/\//i, "");
Expand Down Expand Up @@ -507,15 +570,15 @@ class AbstractShot {
}

AbstractShot.prototype.REGULAR_ATTRS = (`
url docTitle userTitle createdDate favicon images
origin fullUrl docTitle userTitle createdDate favicon images
siteName openGraph twitterCard documentSize
fullScreenThumbnail abTests
`).split(/\s+/g);

// Attributes that will be accepted in the constructor, but ignored/dropped
AbstractShot.prototype.DEPRECATED_ATTRS = (`
microdata history ogTitle createdDevice head body htmlAttrs bodyAttrs headAttrs
readable hashtags comments showPage isPublic resources deviceId
readable hashtags comments showPage isPublic resources deviceId url
`).split(/\s+/g);

AbstractShot.prototype.RECALL_ATTRS = (`
Expand Down Expand Up @@ -659,4 +722,5 @@ AbstractShot.prototype.Clip = _Clip;

if (typeof exports != "undefined") {
exports.AbstractShot = AbstractShot;
exports.originFromUrl = originFromUrl;
}

0 comments on commit a9a076a

Please sign in to comment.