Closed scypio closed 5 years ago
@scypio thanks for the ticket. Looks great. I'd add following statements/design decisions regarding implementation:
Each Knot that processes Fragments:
failed
in case any failure happened during processing of one fragmentfailed
by other Knots processed it before@scypio Additionally:
Fragment
failure
flag on the given fragment, and if it's failed follows the strategy for the given fragment: e.g. ignore appending fragment output to the client response (empty strategy), or appends the static failure (that's already in the Fragment
, thanks to the splitter)@scypio, @marcinczeczko few thoughts/designs from me:
I like very much the idea of having a place in markup to define the fallback. I agree that this should be later consumed as a separate Fragment
by Knots and that it can be reused among other snippet fragments as the fallback content.
The trick here is to introduce a new fragment type in a smart way, that will enable further extensions to be added to the Knot.x easily in the future. What we currently have, is FragmentSplitter
implementation that distinguish Knot.x fragments and raw HTML fragments. I suggest keepieng to this simple idea and extend the mechanics that enrich Fragment
data by its type using e.g. ServiceLoader
.
This would look like this:
<knotx:XYZ>
where knotx
is the namespace that will be used by the splitter to extinguish dynamic fragments that will be later processed (everything other is RAW
) and XYZ
is the type of the dynamic fragment (e.g. snippet
for existing functionality, fallback
for the feature you want to implement and maybe some auth
or other types in the future).ServiceLoader
(thanks to that registering new dynamic type will not require releasing new version of Knot.x and will be easy to customize).FragmentFactory
interface will define a method toFragment(MatchResult result, String markup)
Fragment
can change and be more generic to support other data structures (names to TBD), e.g.:
class Fragment {
String content;
String type;
JsonObject attributes;
...
}
Example:
snippet
type will no longer store list of knots
in the list in Fragment
. Instead it will be stored in the attributes
field. SnippetFragmentFactory
implementation will make sure of creating proper Fragment
instance and setting type = "snippet";
attributes.put("knots", new JsonArray(knots));
FallbackFragmentFactory
can do something like
type = "fallback";
attributes.put("fallbackId", fallbackId)
JsonObject
have additional advantage of easy serializing and deserializing messages on the Event Bus.fallback strategies looks nice but still extending snippets based on markup syntax is not a way to go in my opinion / not enough universal, see https://github.com/Cognifide/knotx/issues/462#issuecomment-428934565
how about
{{!--#knotxSnippet:productsServerSide(knots="databridge,handlebars", databridge-name="someSearch")--}}`
(ok)
{{!--!knotxSnippet:productsServerSide--}}
(fallback)
{{!--/knotxSnippet:productsServerSide--}}
based on {{#each ...}}...{{else}}...{{/each}}
<script type="application/json">
{{!--#knotxSnippet:productsServerSide(knots="databridge,handlebars", databridge-name="someSearch")--}}`
{{{_response}}}
{{!--!knotxSnippet:productsServerSide--}}
{
"results": [],
"numFound": 0
}
{{!--/knotxSnippet:productsServerSide--}}
</script>
Fallback is totally re-worked in Knot.x 2. Please see https://github.com/Knotx/knotx-fragments-handler/blob/master/core/README.md for more details
Problem statement Pages may contain multiple knotx snippets. Failure in processing a single snippet may result in the whole page returning 500 Error respononse.
Let's assume that a page contains two snippets:
When databridge / unstable-service fails the whole rendering process is terminated with a 500 Error.
Proposed solution I would like to:
Blank fallback strategy
would just skip rendering of the failed snippet altogether.Static fallback strategy
would render a static markup defined as a snippet attribute.For example, should following fragment fail:
the text
No data available
would be rendered as a fallback.Alternatives I wondered if fallback could be defined as an embedded node,
I decided I would rather not process the internals of the snippet thou.
Additional context Once the fallback feature is implemented in the knotx core we would like to take advantage of it in the data-bridge module.
e: edited to rename the fallback attribute to just "data-knotx-fallback"