From 4b3d6f72ccfd0bc3a507b685deb51a95a1e58ad3 Mon Sep 17 00:00:00 2001 From: bill bittner Date: Thu, 10 Aug 2017 10:49:19 -0700 Subject: [PATCH 1/2] changed 'short url' to 'short id' --- controllers/serveController.js | 4 ++-- helpers/serveHelpers.js | 21 ++++++++++---------- routes/serve-routes.js | 30 ++++++++++++++--------------- views/partials/assetInfo.handlebars | 4 ++-- 4 files changed, 30 insertions(+), 29 deletions(-) diff --git a/controllers/serveController.js b/controllers/serveController.js index 919dab2b..3f9ea8dd 100644 --- a/controllers/serveController.js +++ b/controllers/serveController.js @@ -89,10 +89,10 @@ module.exports = { // get teh asset by claim Id }); }, - getAssetByShortUrl: function (shortUrl, name) { + getAssetByShortId: function (shortId, name) { return new Promise((resolve, reject) => { // get the full claimId - serveHelpers.getClaimIdFromShortUrl(shortUrl, name) + serveHelpers.getFullClaimIdFromShortId(shortId, name) // get the asset by the claimId .then(claimId => { resolve(getAssetByClaimId(claimId, name)); diff --git a/helpers/serveHelpers.js b/helpers/serveHelpers.js index 62a5447d..c6d6540d 100644 --- a/helpers/serveHelpers.js +++ b/helpers/serveHelpers.js @@ -2,7 +2,7 @@ const logger = require('winston'); const db = require('../models'); const lbryApi = require('./lbryApi'); -function determineShortUrl (claimId, height, claimList) { +function determineShortClaimId (claimId, height, claimList) { logger.debug('determining short url based on claim id and claim list'); // remove this claim from the claim list, if it exists claimList = claimList.filter(claim => { @@ -37,12 +37,12 @@ function determineShortUrl (claimId, height, claimList) { } } -function checkLocalDbForClaims (name, shortUrl) { +function checkLocalDbForClaims (name, shortId) { return db.File .findAll({ where: { name, - claimId: { $like: `${shortUrl}%` }, + claimId: { $like: `${shortId}%` }, }, }) .then(records => { @@ -102,7 +102,7 @@ module.exports = { const openGraphInfo = createOpenGraphInfo(fileInfo); res.status(200).render('showLite', { layout: 'show', fileInfo, openGraphInfo }); }, - getClaimIdFromShortUrl (shortUrl, name) { + getFullClaimIdFromShortId (shortId, name) { return new Promise((resolve, reject) => { logger.debug('getting claim_id from short url'); // use the daemon to check for claims list @@ -111,7 +111,7 @@ module.exports = { logger.debug('Number of claims from getClaimsList:', claims.length); // if no claims were found, check locally for possible claims if (claims.length === 0) { - return checkLocalDbForClaims(name, shortUrl); + return checkLocalDbForClaims(name, shortId); } else { return claims; } @@ -119,7 +119,7 @@ module.exports = { // handle the claims list .then(claims => { logger.debug('Claims ready for filtering'); - const regex = new RegExp(`^${shortUrl}`); + const regex = new RegExp(`^${shortId}`); const filteredClaimsList = claims.filter(claim => { return regex.test(claim.claim_id); }); @@ -143,15 +143,16 @@ module.exports = { }); }); }, - getShortUrlFromClaimId (claimId, height, name) { + getShortIdFromClaimId (claimId, height, name) { return new Promise((resolve, reject) => { - logger.debug('finding short url from claim_id'); + logger.debug('finding short claim id from full claim id'); // get a list of all the claims lbryApi.getClaimsList(name) // find the smallest possible unique url for this claim .then(({ claims }) => { - const shortUrl = determineShortUrl(claimId, height, claims); - resolve(shortUrl); + const shortId = determineShortClaimId(claimId, height, claims); + logger.debug('short claim id ===', shortId); + resolve(shortId); }) .catch(error => { reject(error); diff --git a/routes/serve-routes.js b/routes/serve-routes.js index e1eb3875..cc3c6962 100644 --- a/routes/serve-routes.js +++ b/routes/serve-routes.js @@ -1,6 +1,6 @@ const logger = require('winston'); -const { serveFile, showFile, showFileLite, getShortUrlFromClaimId } = require('../helpers/serveHelpers.js'); -const { getAssetByChannel, getAssetByShortUrl, getAssetByClaimId, getAssetByName } = require('../controllers/serveController.js'); +const { serveFile, showFile, showFileLite, getShortIdFromClaimId } = require('../helpers/serveHelpers.js'); +const { getAssetByChannel, getAssetByShortId, getAssetByClaimId, getAssetByName } = require('../controllers/serveController.js'); const { handleRequestError } = require('../helpers/errorHandlers.js'); const { postToStats, sendGoogleAnalytics } = require('../controllers/statsController.js'); const SERVE = 'SERVE'; @@ -11,12 +11,12 @@ const SHORTURL = 'SHORTURL'; const CLAIMID = 'CLAIMID'; const NAME = 'NAME'; -function getAsset (claimType, channelName, shortUrl, fullClaimId, name) { +function getAsset (claimType, channelName, shortId, fullClaimId, name) { switch (claimType) { case CHANNEL: return getAssetByChannel(channelName, name); case SHORTURL: - return getAssetByShortUrl(shortUrl, name); + return getAssetByShortId(shortId, name); case CLAIMID: return getAssetByClaimId(fullClaimId, name); case NAME: @@ -41,9 +41,9 @@ function serveOrShowAsset (fileInfo, method, headers, originalUrl, ip, res) { postToStats('show', originalUrl, ip, fileInfo.name, fileInfo.claimId, 'success'); return fileInfo; case SHOW: - return getShortUrlFromClaimId(fileInfo.claimId, fileInfo.height, fileInfo.name) - .then(shortUrl => { - fileInfo['shortUrl'] = shortUrl; + return getShortIdFromClaimId(fileInfo.claimId, fileInfo.height, fileInfo.name) + .then(shortId => { + fileInfo['shortId'] = shortId; showFile(fileInfo, res); postToStats('show', originalUrl, ip, fileInfo.name, fileInfo.claimId, 'success'); return fileInfo; @@ -62,12 +62,12 @@ function isValidClaimId (claimId) { return ((claimId.length === 40) && !/[^A-Za-z0-9]/g.test(claimId)); } -function isValidShortUrl (claimId) { +function isValidShortId (claimId) { return claimId.length === 1; // really it should evaluate the short url itself } -function isValidShortUrlOrClaimId (input) { - return (isValidClaimId(input) || isValidShortUrl(input)); +function isValidShortIdOrClaimId (input) { + return (isValidClaimId(input) || isValidShortId(input)); } module.exports = (app) => { @@ -77,7 +77,7 @@ module.exports = (app) => { let name = params.name; let claimType; let channelName = null; - let shortUrl = null; + let shortId = null; let fullClaimId = null; let method; let extension; @@ -101,7 +101,7 @@ module.exports = (app) => { method = SHOW; } /* patch for backwards compatability with spee.ch/name/claim_id */ - if (isValidShortUrlOrClaimId(name) && !isValidShortUrlOrClaimId(identifier)) { + if (isValidShortIdOrClaimId(name) && !isValidShortIdOrClaimId(identifier)) { let tempName = name; name = identifier; identifier = tempName; @@ -119,8 +119,8 @@ module.exports = (app) => { logger.debug('full claim id =', fullClaimId); claimType = CLAIMID; } else if (identifier.length < 40) { - shortUrl = identifier; - logger.debug('short url =', shortUrl); + shortId = identifier; + logger.debug('short claim id =', shortId); claimType = SHORTURL; } else { logger.error('The URL provided could not be parsed'); @@ -128,7 +128,7 @@ module.exports = (app) => { return; }; // 1. retrieve the asset and information - getAsset(claimType, channelName, shortUrl, fullClaimId, name) + getAsset(claimType, channelName, shortId, fullClaimId, name) // 2. serve or show .then(fileInfo => { if (!fileInfo) { diff --git a/views/partials/assetInfo.handlebars b/views/partials/assetInfo.handlebars index 7338aad1..feab0b56 100644 --- a/views/partials/assetInfo.handlebars +++ b/views/partials/assetInfo.handlebars @@ -15,11 +15,11 @@ {{!--short direct link to asset--}}
- Permanent Short Link + Permanent Short Link (most convenient)
- +
{{!-- html text for embedding asset--}} From 42924f0de887eaf6549cd85f3a35516687a0b58b Mon Sep 17 00:00:00 2001 From: bill bittner Date: Thu, 10 Aug 2017 13:04:31 -0700 Subject: [PATCH 2/2] updated short url algo to handle case of other claims but no matching prefix --- helpers/serveHelpers.js | 46 ++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/helpers/serveHelpers.js b/helpers/serveHelpers.js index c6d6540d..c251f1ee 100644 --- a/helpers/serveHelpers.js +++ b/helpers/serveHelpers.js @@ -4,36 +4,48 @@ const lbryApi = require('./lbryApi'); function determineShortClaimId (claimId, height, claimList) { logger.debug('determining short url based on claim id and claim list'); + logger.debug('claimlist starting length:', claimList.length); // remove this claim from the claim list, if it exists claimList = claimList.filter(claim => { return claim.claim_id !== claimId; }); - logger.debug('claim list length:', claimList.length); - // if there are no other claims, return the first letter of the claim id + logger.debug('claim list length without this claim:', claimList.length); + // If there are no other claims, return the first letter of the claim id... if (claimList.length === 0) { return claimId.substring(0, 1); - // otherwise determine the proper url + // ...otherwise determine the proper short id. } else { - let i = 0; const claimListCopy = claimList; - while (claimList.length !== 0) { // filter out matching claims + let i = 0; + // find the longest shared prefix (there is a better way to do this that filters, checks next filter, then filters (i.e. combine this step and next)) + while (claimList.length !== 0) { i++; claimList = claimList.filter(claim => { - return (claim.claim_id.substring(0, i) === claimId.substring(0, i)); + const otherClaimIdSegmentToCompare = claim.claim_id.substring(0, i); + const thisClaimIdSegmentToCompare = claimId.substring(0, i); + logger.debug('compare:', otherClaimIdSegmentToCompare, '===', thisClaimIdSegmentToCompare, '?'); + return (otherClaimIdSegmentToCompare === thisClaimIdSegmentToCompare); }); } - i -= 1; - const lastMatch = claimId.substring(0, i); - - const matchingClaims = claimListCopy.filter(claim => { - return (claim.claim_id.substring(0, i) === lastMatch); - }); - for (let j = 0; j < matchingClaims.length; j++) { - if (matchingClaims[j].height < height) { - return claimId.substring(0, i + 1); - } + // use that longest shared prefix to get only those competing claims + const lastMatchIndex = i - 1; + const lastMatch = claimId.substring(0, lastMatchIndex); + logger.debug('last match index:', lastMatchIndex, 'last match:', lastMatch); + if (lastMatchIndex === 0) { // if no other claims share a prefix, return with first letter. + return claimId.substring(0, 1); } - return claimId.substring(0, i); + const allMatchingClaimsAtLastMatch = claimListCopy.filter(claim => { + return (claim.claim_id.substring(0, lastMatchIndex) === lastMatch); + }); + // for those that share the longest shared prefix: see which came first in time. whichever is earliest, the others take the extra character + const sortedMatchingClaims = allMatchingClaimsAtLastMatch.sort((a, b) => { + return (a.height < b.height); + }); + // compare to the earliest one, if it is earlier, this claim takes the extra character + if (sortedMatchingClaims[0].height < height) { + return claimId.substring(0, lastMatchIndex + 1); + } + return claimId.substring(0, lastMatchIndex); } }