From 7bdd826360b5f687e16356b480817f53645eeb71 Mon Sep 17 00:00:00 2001 From: mpl Date: Fri, 2 Mar 2018 00:00:14 +0100 Subject: [PATCH] pkg/importer: pick the "best" importer account when several exist We still need to figure out the root cause of the problem, i.e. why a sha224 importer node was created in the first place, when a valid sha1 one already existed. Next CL. Updates #1069 Change-Id: I00b4aab9a4f67b9c4ee198774378630615f6d11d --- pkg/importer/importer.go | 65 ++++++++++++++++--- .../ui/blob_item_twitter_content.js | 9 +-- server/camlistored/ui/thumber.js | 3 + 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/pkg/importer/importer.go b/pkg/importer/importer.go index e02bfa733..377afb26c 100644 --- a/pkg/importer/importer.go +++ b/pkg/importer/importer.go @@ -964,18 +964,20 @@ func (im *importer) Node() (*Object, error) { attrImporterType, im.name, ) res, err := im.host.search.Query(context.TODO(), &search.SearchQuery{ - Limit: 10, // only expect 1 + Limit: 10, // might be more than one because of multiple blob hash types Expression: expr, }) if err != nil { return nil, err } - if len(res.Blobs) > 1 { - return nil, fmt.Errorf("Ambiguous; too many permanodes matched query %q: %v", expr, res.Blobs) + best, err := im.bestNode(res) + if err != nil { + return nil, err } - if len(res.Blobs) == 1 { - return im.host.ObjectFromRef(res.Blobs[0].Blob) + if best != nil { + return best, nil } + o, err := im.host.NewObject() if err != nil { return nil, err @@ -992,6 +994,47 @@ func (im *importer) Node() (*Object, error) { return o, nil } +// bestNode picks the most populated importer node amongst res.Blobs, or nil if +// len(res.Blobs) == 0 +// It is the caller's responsibility to create the correct request so that +// res.Blobs are all importer nodes. +func (im *importer) bestNode(res *search.SearchResult) (*Object, error) { + if len(res.Blobs) == 0 { + // let the caller create a fresh node + return nil, nil + } + var best *Object + for _, desb := range res.Blobs { + o, err := im.host.ObjectFromRef(desb.Blob) + if err != nil { + return nil, err + } + if best == nil { + best = o + continue + } + if best.Attr(attrClientID) == "" && o.Attr(attrClientID) != "" { + best = o + continue + } + if best.Attr(attrClientSecret) == "" && o.Attr(attrClientSecret) != "" { + best = o + continue + } + if best.Attr("title") == "" && o.Attr("title") != "" { + best = o + continue + } + // all other things being equal, pick the most recent one, as it's more likely + // to be a sha224 than a sha1 + if best.modtime.Before(o.modtime) { + best = o + continue + } + } + return best, nil +} + // importerAcct is a long-lived type representing account type importerAcct struct { im *importer @@ -1259,8 +1302,9 @@ type Object struct { h *Host pn blob.Ref // permanode ref - mu sync.RWMutex - attr map[string][]string + mu sync.RWMutex + attr map[string][]string + modtime time.Time } // PermanodeRef returns the permanode that this object wraps. @@ -1441,9 +1485,10 @@ func (h *Host) ObjectFromRef(permanodeRef blob.Ref) (*Object, error) { return nil, fmt.Errorf("permanode %v had no DescribedPermanode in Describe response", permanodeRef) } return &Object{ - h: h, - pn: permanodeRef, - attr: map[string][]string(db.Permanode.Attr), + h: h, + pn: permanodeRef, + attr: map[string][]string(db.Permanode.Attr), + modtime: db.Permanode.ModTime, }, nil } diff --git a/server/camlistored/ui/blob_item_twitter_content.js b/server/camlistored/ui/blob_item_twitter_content.js index 0743d0953..4a3cf61d4 100644 --- a/server/camlistored/ui/blob_item_twitter_content.js +++ b/server/camlistored/ui/blob_item_twitter_content.js @@ -99,9 +99,10 @@ cam.BlobItemTwitterContent.getHandler = function(blobref, searchSession, href) { // It's OK to not have any content. Tweets can be just images or whatever. var content = cam.permanodeUtils.getSingleAttr(m.permanode, 'content'); - var imageMeta = cam.permanodeUtils.getSingleAttr(m.permanode, 'camliContentImage'); - if (imageMeta) { - imageMeta = searchSession.getResolvedMeta(imageMeta); + var imageMetaBr = cam.permanodeUtils.getSingleAttr(m.permanode, 'camliContentImage'); + var imageMeta = null; + if (imageMetaBr) { + imageMeta = searchSession.getResolvedMeta(imageMetaBr); } return new cam.BlobItemTwitterContent.Handler(content, Date.parse(date), href, imageMeta, username); @@ -112,7 +113,7 @@ cam.BlobItemTwitterContent.Handler = function(content, date, href, imageMeta, us this.date_ = date; this.href_ = href; this.username_ = username; - this.thumber_ = imageMeta ? new cam.Thumber.fromImageMeta(imageMeta) : null; + this.thumber_ = imageMeta ? cam.Thumber.fromImageMeta(imageMeta) : null; }; cam.BlobItemTwitterContent.Handler.prototype.getAspectRatio = function() { diff --git a/server/camlistored/ui/thumber.js b/server/camlistored/ui/thumber.js index 21c545145..21308035c 100644 --- a/server/camlistored/ui/thumber.js +++ b/server/camlistored/ui/thumber.js @@ -31,6 +31,9 @@ cam.Thumber = function(pathname, opt_aspect) { cam.Thumber.SIZES = [64, 128, 256, 375, 500, 750, 1000, 1500, 2000]; cam.Thumber.fromImageMeta = function(imageMeta) { + if (!imageMeta || !imageMeta.image || !imageMeta.blobRef || !imageMeta.file || !imageMeta.file.fileName) { + return null; + } return new cam.Thumber(goog.string.subs('thumbnail/%s/%s', imageMeta.blobRef, (imageMeta.file && imageMeta.file.fileName) || imageMeta.blobRef + '.jpg'), imageMeta.image.width / imageMeta.image.height); };