Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fuo协议Show命令增强-Add Features #380

Merged
merged 9 commits into from
Jul 2, 2020
5 changes: 5 additions & 0 deletions fuocore/cmds/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import logging

from fuocore.excs import FuoException
from fuocore.router import ShowCmdException

from .base import cmd_handler_mapping

from .help import HelpHandler # noqa
Expand Down Expand Up @@ -66,6 +68,9 @@ def exec_cmd(cmd, *args, app):
playlist=playlist,
live_lyric=live_lyric)
rv = handler.handle(cmd)
except ShowCmdException:
logger.exception('handle cmd({}) error'.format(cmd))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

像这种已经被正确处理了的错误,打个 warning/error 级别就可以了。

打 exception 的话,一般是意料之外的错误。比如未知的错误等。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

谢谢,学习到了

return False, 'show command not correct'
except Exception:
logger.exception('handle cmd({}) error'.format(cmd))
return False, 'internal server error'
Expand Down
148 changes: 133 additions & 15 deletions fuocore/cmds/show.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@
import logging
from urllib.parse import urlparse

from fuocore.router import Router
from fuocore.utils import reader_to_list, to_reader
from fuocore.router import Router

from .base import AbstractHandler


logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -46,47 +45,166 @@ def list_providers(req):

@route('/<provider>/songs/<sid>')
def song_detail(req, provider, sid):
provider_path_name = provider
provider = req.ctx['library'].get(provider)
song = provider.Song.get(sid)
return song

try:
song = provider.Song.get(sid)
except AttributeError:
return "No such provider : {}-{}".format(provider.name, provider_path_name)
# marshallow exception -- Validation Error
except Exception:
return "song identified by {} is unavailable in {} ".format(sid, provider.name)
else:
if song is None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果 song 是 None,直接返回个 None 是不是就可以了?用 None 来表示 id 找不到对应的歌曲。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

啊的确如此,但是最终呈现在客户端的命令结果什么也没有,身为用户我可能不清楚该命令是否成功执行,所以选择了返回一个提示

return "song {} is not in local library".format(sid)
return song
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

下面有几段代码都比较相似,可以考虑抽象一下 ~

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好嘞,我先想着看看



@route('/<provider>/songs/<sid>/lyric')
def lyric(req, provider, sid):
provider_path_name = provider
provider = req.ctx['library'].get(provider)
song = provider.Song.get(sid)
if song.lyric:
return song.lyric.content
return ''

try:
song = provider.Song.get(sid)
except AttributeError:
return "No such provider : {}-{}".format(provider.name, provider_path_name)
# marshallow exception -- Validation Error
except Exception:
return "song identified by {} is unavailable in {} ".format(sid, provider.name)
else:
if song is None:
return "song {} is not in local library".format(sid)
if song.lyric:
return song.lyric.content
return 'no lyric, enjoy it~'


@route('/<provider>/artists/<aid>')
def artist_detail(req, provider, aid):
provider_path_name = provider
provider = req.ctx['library'].get(provider)
return provider.Artist.get(aid)

try:
artist = provider.Artist.get(aid)
except AttributeError:
return "No such provider : {}-{}".format(provider.name, provider_path_name)
except Exception:
return "artist identified by {} is not found in {}".format(aid, provider.name)
else:
if artist is None:
return "artist {} is not in local library".format(aid)
return artist


@route('/<provider>/albums/<bid>')
def album_detail(req, provider, bid):
provider_path_name = provider
provider = req.ctx['library'].get(provider)
return provider.Album.get(bid)

try:
album = provider.Album.get(bid)
except AttributeError:
return "No such provider : {}-{}".format(provider.name, provider_path_name)
except Exception:
return "album identified by {} in {} is unavailable".format(bid, provider.name)
else:
if album is None:
return "album identified by {} in {} is unavailable".format(bid, provider.name)
return album


'''
------------------------------------
Original Route -- get User by uid
example : fuo show fuo://<provider>/users/12345678
------------------------------------
Issue #317
Description: fuo show nehancement -- show info about current user
example : fuo show fuo://<provider>/users/me
'''


@route('/<provider>/users/<uid>')
def user_detail(req, provider, uid):
provider_path_name = provider
provider = req.ctx['library'].get(provider)
return provider.User.get(uid)

try:
if uid == 'me':
user = provider._user
else:
user = provider.User.get(uid)
except AttributeError:
return "No such provider : {}-{}".format(provider.name, provider_path_name)
except Exception:
return "No user identified by {} in {}".format(uid, provider.name)
else:
if user is not None:
return user
elif uid == 'me':
return "User is not logged in in current session(plugin) {}-{}".format(provider.name, provider_path_name)
else:
return "No user in local"


@route('/<provider>/playlists/<pid>')
def playlist_detail(req, provider, pid):
provider_path_name = provider
provider = req.ctx['library'].get(provider)
return provider.Playlist.get(pid)

try:
playlist = provider.Playlist.get(pid)
except AttributeError:
return "No such provider : {}-{}".format(provider.name, provider_path_name)
except Exception:
return "No playlist found by {} in {} ".format(pid, provider.name)
else:
if playlist is None:
return "No playlist found by {} in {} ".format(pid, provider.name)
return playlist


@route('/<provider>/playlists/<pid>/songs')
def playlist_songs(req, provider, pid):
provider_path_name = provider
provider = req.ctx['library'].get(provider)
playlist = provider.Playlist.get(pid)
songs = reader_to_list(to_reader(playlist, "songs"))
return songs

try:
playlist = provider.Playlist.get(pid)
except AttributeError:
return "No such provider : {}-{}".format(provider.name, provider_path_name)
except Exception:
return "No playlist found by {} in {} ".format(pid, provider.name)
else:
if playlist is None:
return "No playlist found by {} in {} ".format(pid, provider.name)
songs = reader_to_list(to_reader(playlist, "songs"))
return songs


'''
Issue #317
Description: fuo show enhancement -- show all albums of an artist identified by aid
example : fuo show fuo://<provider>/artists/<aid>/albums
'''


@route('/<provider>/artists/<aid>/albums')
def albums_of_artist(req, provider, aid):
provider_path_name = provider
provider = req.ctx['library'].get(provider)

try:
artist = provider.Artist.get(aid)
except AttributeError:
return "No such provider : {}-{}".format(provider.name, provider_path_name)
except Exception:
return "artist identified by {} is not found in {}".format(aid, provider.name)
else:
if artist is not None:
albums = reader_to_list(to_reader(artist, "albums"))
return albums
return "artist identified by {} is not found in {}".format(aid, provider.name)

25 changes: 22 additions & 3 deletions fuocore/router.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,22 @@
from urllib.parse import parse_qsl, urlsplit


class NotFound(Exception):
class ShowCmdException(Exception):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass


# Rule not found
class NotFound(ShowCmdException):
def __init__(self, rule):
self.rule = rule

def __str__(self):
# use backstrace lib (the format_exc/print_exc) causes fail
# maybe the coroutine issue ?
uri_msg = "Bad Uri Exception: fuo://{}".format(self.rule)
return uri_msg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我觉得可以直接在抛错的时候把文字内容加进去,比如

raise NotFound(f"Bad Uri Exception: fuo://{self.rule}")



def match(url, rules):
"""找到 path 对应的 rule,并解析其中的参数

Expand All @@ -30,7 +42,8 @@ def match(url, rules):
query = dict(parse_qsl(qs))
params = match.groupdict()
return rule, params, query
raise NotFound

raise NotFound(path)


def _validate_rule(rule):
Expand Down Expand Up @@ -96,7 +109,13 @@ def wrapper(*args, **kwargs):
return decorator

def dispatch(self, uri, ctx):
rule, params, query = match(uri, self.rules)
try:
rule, params, query = match(uri, self.rules)
except NotFound:
# pass it to the exception handle procedure in __init__.py
raise
return None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里只是 re-raise 了一下,看起来 try ... except... 不是很有必要?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对,代码删改到后面,变成这一段只是为了使得__init__.py能捕获得异常的信息,所以就简单的re-raise,应该有更好的方法来着


handler = self.handlers[rule]
req = Request(uri=uri, rule=rule, params=params, query=query, ctx=ctx)
return handler(req, **params)