Rework list api and use separator in rss redis key#445
Conversation
jackyzy823
changed the title
| let list = await getCachedList(id=(@"id")) | ||
| if list.id.len == 0: | ||
| resp Http404 | ||
| await cache(list) |
There was a problem hiding this comment.
Since cache is saved in previous getCachedList , it's not necessary to do it again here.
jackyzy823
force-pushed
the
rework-list
branch
from
0fe254e to
db23381
Compare
jackyzy823
force-pushed
the
rework-list
branch
from
db23381 to
083d5cc
Compare
jackyzy823
force-pushed
the
rework-list
branch
2 times, most recently
from
a0d480b to
39c0759
Compare
zedeus
left a comment
zedeus
left a comment
There was a problem hiding this comment.
Looks good, just some nits and RSS key changes
| import strutils | ||
| import uri |
There was a problem hiding this comment.
| import strutils | |
| import uri | |
| import strutils, uri |
| members: list{"member_count"}.getInt, | ||
| banner: list{"custom_banner_media", "media_info", "url"}.getImageStr | ||
| ) | ||
| if result.banner.len == 0: |
There was a problem hiding this comment.
I personally very much dislike the default banners, this should be removed
| let | ||
| cursor = getCursor() | ||
| key = $hash(genQueryUrl(query)) & cursor | ||
| key = "search/" & $hash(genQueryUrl(query)) & "/" & cursor |
There was a problem hiding this comment.
| key = "search/" & $hash(genQueryUrl(query)) & "/" & cursor | |
| key = "search:" & $hash(genQueryUrl(query)) & ":" & cursor |
| cursor = getCursor() | ||
| name = @"name" | ||
| key = name & cursor | ||
| key = "twitter/" & name & "/" & cursor |
There was a problem hiding this comment.
| key = "twitter/" & name & "/" & cursor | |
| key = "tweets:" & name & ":" & cursor |
| else: Query(fromUser: @[name]) | ||
|
|
||
| var key = @"name" & "/" & @"tab" | ||
| var key = "tab/" & @"name" & "/" & @"tab" & "/" |
There was a problem hiding this comment.
For consistency, I think it makes sense to put the tab name in front here, so we have e.g. rss:tweets:username, rss:media:username, rss:search:username.
| var key = "tab/" & @"name" & "/" & @"tab" & "/" | |
| var key = @"tab" & ":" & @"name" & ":" |
| slug = decodeUrl(@"slug") | ||
| list = await getCachedList(@"name", slug) | ||
| if list.id.len == 0: | ||
| resp Http404 |
There was a problem hiding this comment.
| resp Http404 | |
| resp Http404, showError("List \"" & @"slug" & "\" not found", cfg) |
| template respList*(list, timeline, vnode: typed) = | ||
| template respList*(list, timeline, title, vnode: typed) = | ||
| if list.id.len == 0: | ||
| resp Http404, showError("List \"" & @"list" & "\" not found", cfg) |
There was a problem hiding this comment.
| resp Http404, showError("List \"" & @"list" & "\" not found", cfg) | |
| resp Http404, showError("List \"" & @"id" & "\" not found", cfg) |
| result = await getGraphListById(id) | ||
| else: | ||
| result = await getGraphList(username, name) | ||
| result = await getGraphList(username, slug) |
There was a problem hiding this comment.
Since slug is now only for redirecting, these functions could be renamed to getGraphList and getGraphListBySlug
| else: await get(toLower("l:" & username & '/' & name)) | ||
| proc getCachedList*(username=""; slug=""; id=""): Future[List] {.async.} = | ||
| let list = if id.len == 0: redisNil | ||
| else: await get("l:" & id ) |
There was a problem hiding this comment.
| else: await get("l:" & id ) | |
| else: await get("l:" & id) |
| name: list{"name"}.getStr, | ||
| username: list{"user", "legacy", "screen_name"}.getStr, | ||
| userId: list{"user", "legacy", "id_str"}.getStr, | ||
| userId: list{"user", "rest_id"}.getStr, |
There was a problem hiding this comment.
Are you sure about this one? I remember there being some weirdness going on with id_str vs rest_id
There was a problem hiding this comment.
According to my observation , there's no id_str under legacy now. so i had to choose rest_id to fill userId.
jackyzy823
force-pushed
the
rework-list
branch
from
39c0759 to
ef7ad67
Compare
|
Thanks! |
2 participants
Rework on list related api
List's name( used for display) cannot (and shoud not) be simply converted toslug name(used in URL) by replace " " with "-"For example:
A display name
推特动物园of@starknightcannot used as slug name ,it's slug name is 1434331244116938757 (same as list id)another example
A display name
我的精神病家园of@cxiaojican be used as slugname. (maybe it is created in an early time)A display name
VC A-List's slug name is "vc-a-list"so we should use
/i/lists/<list_id>as primary route and redirect/<screen_name>/lists/<slug_name>to/i/lists/<list_id>(the same as twitter do)Instead of
/<screen_name>/lists/<display_name_or_some_slug_name_converted_from_display_name_by_adding_hyphen>/UI improvement:
Set html title for
Listview.use separator in rss redis key
for example :
previously a rss rediskey might be: rss:abcdef123456 .
So how to distinguish which part is username which part is cursor ? It could be username: abcdef123 cursor:456 or username: abcdef cursor:123456
so we should add separator between this params. I choose
/, but i think:is ok too. let me know what should be finally used.This may resolve #428 related issue.