From 0b7939ae1a1e3ce55ee458d24fd5946542d9c14a Mon Sep 17 00:00:00 2001 From: Dan McGee Date: Tue, 31 Jul 2012 19:01:45 -0500 Subject: Rework package details dispatch code We had a variety of fallback paths that we took if a details page didn't exist for the combination of URL parts passed in. Before I go adding a few more possibilities, rework this so it is more flexible. It is now as simple as adding a method to the dispatch options list in order to have further fallback options. Signed-off-by: Dan McGee --- packages/views/display.py | 82 ++++++++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 37 deletions(-) (limited to 'packages') diff --git a/packages/views/display.py b/packages/views/display.py index 585e0e4e..1ea9ea48 100644 --- a/packages/views/display.py +++ b/packages/views/display.py @@ -14,10 +14,10 @@ def split_package_details(request, name, repo, arch): - arch = get_object_or_404(Arch, name=arch) + '''Check if we have a split package (e.g. pkgbase) value matching this + name. If so, we can show a listing page for the entire set of packages.''' arches = [ arch ] arches.extend(Arch.objects.filter(agnostic=True)) - repo = get_object_or_404(Repo, name__iexact=repo) pkgs = Package.objects.normal().filter(pkgbase=name, repo__testing=repo.testing, repo__staging=repo.staging, arch__in=arches).order_by('pkgname') @@ -39,15 +39,13 @@ def split_package_details(request, name, repo, arch): def recently_removed_package(request, name, repo, arch, cutoff=CUTOFF): - '''We're just steps away from raising a 404, but check our packages update - table first to see if this package has existed in this repo before. If so, - we can show a 410 Gone page and point the requester in the right - direction.''' - arch = get_object_or_404(Arch, name=arch) + '''Check our packages update table to see if this package has existed in + this repo before. If so, we can show a 410 Gone page and point the + requester in the right direction.''' arches = [ arch ] arches.extend(Arch.objects.filter(agnostic=True)) match = Update.objects.select_related('arch', 'repo').filter( - pkgname=name, repo__name__iexact=repo, arch__in=arches) + pkgname=name, repo=repo, arch__in=arches) if cutoff is not None: when = now() - cutoff match = match.filter(created__gte=when) @@ -59,43 +57,53 @@ def recently_removed_package(request, name, repo, arch, cutoff=CUTOFF): return None +def redirect_agnostic(request, name, repo, arch): + '''For arch='any' packages, we can issue a redirect to them if we have a + single non-ambiguous option by changing the arch to match any arch-agnostic + package.''' + if not arch.agnostic: + # limit to 2 results, we only need to know whether there is anything + # except only one matching result + pkgs = Package.objects.select_related( + 'arch', 'repo', 'packager').filter(pkgname=name, + repo=repo, arch__agnostic=True)[:2] + if len(pkgs) == 1: + return redirect(pkgs[0], permanent=True) + return None + + +def redirect_to_search(request, name, repo, arch): + pkg_data = [ + ('arch', arch.lower()), + ('repo', repo.lower()), + ('q', name), + ] + # only include non-blank values in the query we generate + pkg_data = [(x, y.encode('utf-8')) for x, y in pkg_data if y] + return redirect("/packages/?%s" % urlencode(pkg_data)) + + def details(request, name='', repo='', arch=''): if all([name, repo, arch]): + arch_obj = get_object_or_404(Arch, name=arch) + repo_obj = get_object_or_404(Repo, name__iexact=repo) try: pkg = Package.objects.select_related( 'arch', 'repo', 'packager').get(pkgname=name, - repo__name__iexact=repo, arch__name=arch) + repo=repo_obj, arch=arch_obj) return render(request, 'packages/details.html', {'pkg': pkg}) except Package.DoesNotExist: - arch_obj = get_object_or_404(Arch, name=arch) - # for arch='any' packages, we can issue a redirect to them if we - # have a single non-ambiguous option by changing the arch to match - # any arch-agnostic package - if not arch_obj.agnostic: - pkgs = Package.objects.select_related( - 'arch', 'repo', 'packager').filter(pkgname=name, - repo__name__iexact=repo, arch__agnostic=True) - if len(pkgs) == 1: - return redirect(pkgs[0], permanent=True) - # do we have a split package matching this criteria? - ret = split_package_details(request, name, repo, arch) - if ret is None: - # maybe we have a recently-removed package? - ret = recently_removed_package(request, name, repo, arch) - if ret is not None: - return ret - else: - # we've tried everything at this point, nothing to see - raise Http404 + # attempt a variety of fallback options before 404ing + options = (redirect_agnostic, split_package_details, + recently_removed_package) + for method in options: + ret = method(request, name, repo_obj, arch_obj) + if ret: + return ret + # we've tried everything at this point, nothing to see + raise Http404 else: - pkg_data = [ - ('arch', arch.lower()), - ('repo', repo.lower()), - ('q', name), - ] - # only include non-blank values in the query we generate - pkg_data = [(x, y.encode('utf-8')) for x, y in pkg_data if y] - return redirect("/packages/?%s" % urlencode(pkg_data)) + return redirect_to_search(request, name, repo, arch) def groups(request, arch=None): -- cgit v1.2.3-54-g00ecf