Onderscheid op accept header toelaten #86

claeyswo · 2023-02-06T13:30:20Z

Nu ziet een regel er zo uit:

- match: '^/archeologie/metaaldetectievondstmeldingen/(?P<id>\d+)$'
      redirect: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/beheer/#/{id}'

We zouden iets zoals dit willen ondersteunen:

- match: '^/archeologie/metaaldetectievondstmeldingen/(?P<id>\d+)$'
      redirect: 
        - text/html: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/beheer/#/{id}'
        - application/json: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/metaaldetectievondsten/{id}'

Misschien niet meer dan hier extra check op u[redirect] is list => filteren op u[redirect][request.accept] of zoets?

The text was updated successfully, but these errors were encountered:

goessebr · 2023-02-06T14:58:38Z

Zelfde fix is nodig voor toevalsvondsten
(cfr. https://github.com/OnroerendErfgoed/inventaris/issues/5711#issuecomment-1419123095 en https://github.com/OnroerendErfgoed/inventaris/issues/5710#issuecomment-1419119400)

Indien JSON dan willen we naar https://test-loket.onroerenderfgoed.be/archeologie/vondstmeldingen/toevalsvondsten/545 (huidige standaard gedrag), indien HTML dan willen we naar /beheer (https://test-loket.onroerenderfgoed.be/archeologie/vondstmeldingen/toevalsvondsten/beheer) met het juiste ID geopend

koenedaele · 2023-02-08T20:58:21Z

Wat als er een mime-type gevraagd wordt dat niet gedefinieerd is?

koenedaele · 2023-02-08T21:02:58Z

Lijkt me een implementatie van 4.3 in https://www.w3.org/TR/cooluris/ te zijn.

claeyswo · 2023-02-09T07:34:37Z

Wat als er een mime-type gevraagd wordt dat niet gedefinieerd is?

altijd default voorzien
eerste uit de lijst
404
iets anders?

goessebr · 2023-02-10T05:58:49Z

Voorstel in pseudo code:

Indien type(redirect) == string: we aanvaarden alle Accept headers en vertalen naar de opgegeven waarde (huidig gedrag)
Indien type(redirect) == list:
- Indien opgevraagde type niet aanwezig als key in de configlist: 406 Not Acceptable (https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/406).
- Er is ook een key "default" of iets gelijkaardig mogelijk, zodat we niet alle keys moeten opgeven als we slechts voor 1 mime-type een ander gedrag willen, maar alle andere mime-types naar dezelfde URL willen doorverwijzen. Dus indien "default" aanwezig in de lijst, treed er nooit een 406 op.

koenedaele · 2023-02-10T19:04:39Z

Akkoord met voorstel van Bram (met de default optie). Was even vergeten dat er een HTTP 406 was. Mooie oplossing!

Wim-De-Clercq · 2023-02-15T14:42:19Z

Mag ik nog een kleine syntax change in de yaml voorstellen:

- match: '^/archeologie/metaaldetectievondstmeldingen/(?P<id>\d+)$'
  redirect: 
    text/html: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/beheer/#/{id}'
    application/json: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/metaaldetectievondsten/{id}'

Dan zit onder redirect een dict ipv list met dicts.

redirect = {
  "text/html": "redirecturl",
  "application/json": "redirecturl",
}

vs

redirect = [
  {"text/html": "redirecturl"},
  {"application/json": "redirecturl"},
]

Dat eerste lijkt mij logischer, en je kan dan ook geen dubbele mimetypes hebben als bonus.

Als de request geen accept header definieert (en dus eigenlijk zegt alles te accepteren). Wil ik dan random het eerste nemen, of wil ik manueel op zoek gaan of text/html bestaat eerst?

Issue #86

goessebr · 2023-02-16T07:02:59Z

Als de request geen accept header definieert (en dus eigenlijk zegt alles te accepteren). Wil ik dan random het eerste nemen, of wil ik manueel op zoek gaan of text/html bestaat eerst?

Indien er een default key is voorzien -> redirect naar de waarde die daar is opgegeven.
Indien geen default key is voorzien zou ik een 406 opgegeven in dit geval.

Wim-De-Clercq · 2023-02-16T08:11:26Z

Zou dat niet gaan overlappen met browser calls? Browsers sturen altijd */* op. En ik vrees dat dat ook een default zou zijn voor pyramid accept header... kzal effe zien nog..

Wim-De-Clercq · 2023-02-16T08:23:07Z

Ja, dat zou issues geven denk ik. Omdat voor pyramid geen header alles matcht.

alleja, ik zou wel kunnen kijken naar de class van die header, als die AcceptNoHeader is ...

Wim-De-Clercq · 2023-02-16T09:22:58Z

Na daily: Ik begreep bram zijn opmerking verkeerd. Hij bedoelde dat er in de yaml ergens een "default" key zou zijn. Dat is wel mogelijk.

goessebr · 2023-02-17T10:50:59Z

https://test-id.erfgoed.net/archeologie/metaaldetectievondstmeldingen/716 geeft 500 fout

[2023-02-17 11:50:05 +0100] [722001] [ERROR] Error handling request /archeologie/metaaldetectievondstmeldingen/716
Traceback (most recent call last):
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/tweens.py", line 13, in _error_handler
    response = request.invoke_exception_view(exc_info)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/view.py", line 795, in invoke_exception_view
    raise HTTPNotFound
pyramid.httpexceptions.HTTPNotFound: The resource could not be found.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/gunicorn/workers/gthread.py", line 271, in handle
    keepalive = self.handle_request(req, conn)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/gunicorn/workers/gthread.py", line 323, in handle_request
    respiter = self.wsgi(environ, resp.start_response)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/wsgicoers.py", line 212, in __call__
    return self.application(environ, custom_start_response)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/router.py", line 270, in __call__
    response = self.execution_policy(environ, self)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/router.py", line 279, in default_execution_policy
    return request.invoke_exception_view(reraise=True)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/view.py", line 794, in invoke_exception_view
    reraise_(*exc_info)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/compat.py", line 179, in reraise
    raise value
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/router.py", line 277, in default_execution_policy
    return router.invoke_request(request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/router.py", line 249, in invoke_request
    response = handle_request(request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/tweens.py", line 43, in excview_tween
    response = _error_handler(request, exc)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/tweens.py", line 17, in _error_handler
    reraise(*exc_info)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/compat.py", line 179, in reraise
    raise value
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/tweens.py", line 41, in excview_tween
    response = handler(request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/router.py", line 147, in handle_request
    response = _call_view(
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/view.py", line 683, in _call_view
    response = view_callable(context, request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/config/views.py", line 169, in __call__
    return view(context, request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/config/views.py", line 188, in attr_view
    return view(context, request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/config/views.py", line 214, in predicate_wrapper
    return view(context, request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/viewderivers.py", line 401, in viewresult_to_response
    result = view(context, request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/pyramid/viewderivers.py", line 144, in _requestonly_view
    response = view(request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/urihandler/views.py", line 13, in redirect
    redirect = request.uri_handler.handle(uri, request)
  File "/var/projects/joeri/venv/lib/python3.8/site-packages/urihandler/handler.py", line 33, in handle
    redirect = u["redirect"].format(**m.groupdict())
AttributeError: 'dict' object has no attribute 'format'

Dit is de yaml

    - match: '^/archeologie/metaaldetectievondstmeldingen/(?P<id>\d+)$'
      redirect:
        text/html: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/beheer/#/{id}'
        application/json: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/metaaldetectievondsten/{id}'

Wim-De-Clercq · 2023-02-17T11:01:04Z

Ik denk dat de urihandler package nog niet goed geupdate is als ik de lijn nummers zo zie.

goessebr · 2023-02-17T11:05:25Z

hm, ok. Ik bekijk het

goessebr · 2023-02-17T13:42:59Z

Het probleem was dat de versie van de urihandler package niet veranderd is op develop waardoor het package niet werd geupdated

Na een pip uninstall urihandler en nieuwe install was bovenstaande probleem voorbij

Nog 1 ander probleem

Dit is mijn config

    - match: '^/archeologie/metaaldetectievondstmeldingen/(?P<id>\d+)$'
      redirect:
        text/html: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/beheer/#/{id}'
        application/json: 'https://@@archeologie.url@@/metaaldetectievondstmeldingen/metaaldetectievondsten/{id}'
        default: 'https://@@inventaris.url@@/erfgoedobjecten/{id}'

Ik krijg een 406 voor https://dev-id.erfgoed.net/archeologie/metaaldetectievondstmeldingen/1 en Accept: application/xml, ik verwacht de erfgoedobjecten url

Wim-De-Clercq · 2023-02-17T13:47:35Z

Ah, dat heb ik fout begrepen dan. Ik had in mijn hoofd dat default enkel was voor het geval van dat er geen accept header aanwezig was.

Issue #86

goessebr · 2023-02-17T14:03:46Z

Ah, dat heb ik fout begrepen dan. Ik had in mijn hoofd dat default enkel was voor het geval van dat er geen accept header aanwezig was.

ah nee, default is de opvangbak van alle headers. Als alle gedefinieerde keys zijn gecontroleerd en we hebben geen match met onze header, dan is dit de redirect voor alles. Eerst moeten dus wel eerst alle keys gecontroleerd hebben want met voorkeur matchen we op het juiste type.

Issue #86

claeyswo assigned Wim-De-Clercq Feb 6, 2023

claeyswo added the backend label Feb 6, 2023

claeyswo added this to the Sprint 200 milestone Feb 6, 2023

claeyswo modified the milestones: Sprint 200, Sprint 201 Feb 13, 2023

Wim-De-Clercq added a commit that referenced this issue Feb 15, 2023

Add support for accept header filtering.

017fd64

Issue #86

Wim-De-Clercq mentioned this issue Feb 15, 2023

Add support for accept header filtering. #88

Merged

goessebr added the test label Feb 17, 2023

Wim-De-Clercq added a commit that referenced this issue Feb 17, 2023

Use default redirect when nothing matches.

ca0abe7

Issue #86

Wim-De-Clercq added a commit that referenced this issue Feb 17, 2023

Use default redirect when nothing matches.

45697a7

Issue #86

Wim-De-Clercq added a commit that referenced this issue Feb 17, 2023

Use default redirect when nothing matches.

779633b

Issue #86

goessebr closed this as completed Feb 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Onderscheid op accept header toelaten #86

Onderscheid op accept header toelaten #86

claeyswo commented Feb 6, 2023

goessebr commented Feb 6, 2023 •

edited

Loading

koenedaele commented Feb 8, 2023

koenedaele commented Feb 8, 2023

claeyswo commented Feb 9, 2023

goessebr commented Feb 10, 2023

koenedaele commented Feb 10, 2023

Wim-De-Clercq commented Feb 15, 2023 •

edited

Loading

goessebr commented Feb 16, 2023

Wim-De-Clercq commented Feb 16, 2023 •

edited

Loading

Wim-De-Clercq commented Feb 16, 2023 •

edited

Loading

Wim-De-Clercq commented Feb 16, 2023 •

edited

Loading

goessebr commented Feb 17, 2023

Wim-De-Clercq commented Feb 17, 2023

goessebr commented Feb 17, 2023

goessebr commented Feb 17, 2023

Wim-De-Clercq commented Feb 17, 2023

goessebr commented Feb 17, 2023

Onderscheid op accept header toelaten #86

Onderscheid op accept header toelaten #86

Comments

claeyswo commented Feb 6, 2023

goessebr commented Feb 6, 2023 • edited Loading

koenedaele commented Feb 8, 2023

koenedaele commented Feb 8, 2023

claeyswo commented Feb 9, 2023

goessebr commented Feb 10, 2023

koenedaele commented Feb 10, 2023

Wim-De-Clercq commented Feb 15, 2023 • edited Loading

goessebr commented Feb 16, 2023

Wim-De-Clercq commented Feb 16, 2023 • edited Loading

Wim-De-Clercq commented Feb 16, 2023 • edited Loading

Wim-De-Clercq commented Feb 16, 2023 • edited Loading

goessebr commented Feb 17, 2023

Wim-De-Clercq commented Feb 17, 2023

goessebr commented Feb 17, 2023

goessebr commented Feb 17, 2023

Wim-De-Clercq commented Feb 17, 2023

goessebr commented Feb 17, 2023

goessebr commented Feb 6, 2023 •

edited

Loading

Wim-De-Clercq commented Feb 15, 2023 •

edited

Loading

Wim-De-Clercq commented Feb 16, 2023 •

edited

Loading

Wim-De-Clercq commented Feb 16, 2023 •

edited

Loading

Wim-De-Clercq commented Feb 16, 2023 •

edited

Loading