feat: Provide a generic Tree.dump() function
#1366
Conversation
In many use-cases, a tree must be fetched for e.g. a workflow, a menu or similar. This helper function directly dumps a hierarchy/tree to a given level and with a given limit as JSON. Its better, to provide a simple function for this, instead of copy a bad function multiple times in several projects.
phorward
added
feature
Co-authored-by: agudermann <[email protected]>
This can also be called internally now without JSON dumps/loads skirmish.
src/viur/core/prototypes/tree.py
Outdated
| and for debugging purposes. | ||
|
|
||
| :param parententry: Parententry to dump; If not given, the fuction tries to figure out a single parent entry. | ||
| :param depth: Depth to dump children. Can only be a maxiumum of 3. |
There was a problem hiding this comment.
| :param depth: Depth to dump children. Can only be a maxiumum of 3. | |
| :param depth: Depth to dump children. Can only be a maximum of 3. |
src/viur/core/prototypes/tree.py
Outdated
| current.request.get().response.headers["Content-Type"] = "application/json" | ||
|
|
||
| if depth < 1 or depth > 3: | ||
| raise errors.NotAcceptable("'depth' out of bounds") |
There was a problem hiding this comment.
Why we not print out the borders?
|
@ArneGudermann I've updated the PR with your change request, can you please re-review? |
| current.request.get().response.headers["Content-Type"] = "application/json" | ||
|
|
||
| if depth < 1 or depth > 3: | ||
| raise errors.NotAcceptable("'depth' out of bounds") |
There was a problem hiding this comment.
I have already mentioned it, why not print the borders here?
| raise errors.NotAcceptable("'depth' out of bounds") | ||
|
|
||
| if limit < 1 or limit > 30: | ||
| raise errors.NotAcceptable("'limit' out of bounds") |
| if not utils.string.is_prefix(self.render.kind, "json"): | ||
| raise errors.BadRequest("Can only use this function on JSON-based renders") |
There was a problem hiding this comment.
Methods in a module should be render independent IMO. That's exactly the purpose of the renderer, to transform data from a generic to a specific output.
I would just fetch the render instances here and in dump() pass the result through the renderer.
This method could also be used for internal purposes to fetch data for non-json-renderers.
| return ret | ||
|
|
||
| @exposed | ||
| def dump( |
There was a problem hiding this comment.
This should not be under Helpers, but below the other @exposed methods.
| if not utils.string.is_prefix(self.render.kind, "json"): | ||
| raise errors.BadRequest("Can only use this function on JSON-based renders") |
There was a problem hiding this comment.
This is against the modul/renderer concept.
A method in the module should work with any renderer. The logic and the preprocessing take place generically in the module, the final processing into the target format in the renderer.
You have to make this method generic.
If it really only works / should only work within a renderer, you must implement it directly in this render (cf. ViRenderer's getStructure)
| if depth < 1 or depth > 3: | ||
| raise errors.NotAcceptable("'depth' out of bounds") | ||
|
|
||
| if limit < 1 or limit > 30: |
There was a problem hiding this comment.
in my opinion, this is an arbitrary limit without any logical derivation or basis for calculation.
I assume you want to prevent the request from becoming too large with large trees. But that is simply too arbitrary for any context.
Furthermore, you underestimate polynomial growth. Your limit of
Whereas the extension by one more level, but a reduction of the limit to 5 would still be a perfectly conceivable amount:
While the theoretical worst-case scenario is unlikely to be used in practice, it is quite realistic to have more than 3 levels and more than 30 entries on at least one (probably the last level; the leafs).
I would therefore suggest offering the limitation in case you only want to display the first n entries per level in your view and want to reload the rest, but not limiting it so strictly.
In addition, I would add an absolute limit so that a maximum of, for example, 1000 entries, regardless of their level and number, can be loaded with one request. However, this limit is then not available for the frontend, but only a (configurable) limitation by the backend.
In many use-cases, a tree must be fetched for e.g. a workflow, a menu or similar. This helper function directly dumps a hierarchy/tree to a given level and with a given limit as JSON.
Its better, to provide a simple function for this, instead of copy a bad function multiple times in several projects.