@PreserveOnRefresh support for Embedded Flow Components

[ujoni]

Adding @PreserveOnRefresh annotation to WebComponentExporter allows the user to preserve the contents of all the embedded web components derived from that exporter.

The identity of the component tied to the windows name, generated component id, and (optionally) id provided by the web component on the page. Since the component cache is session scoped, if the user defines two instances of the web component on different pages (but same session and the ordering of exported web components are same on both pages), the contents will be cross-preserved, unless the user provides id attributes for those web components.

This is an experimental feature, so the implementation has been done without API changes.

This change is 

[vaadin-bot]

Remove useless curly braces around statement

[caalador]

Reviewed 14 of 14 files at r1.
Reviewable status: 8 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @ujoni)

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 187 at r1 (raw file):

Quoted 4 lines of code…

            getPage().retrieveExtendedClientDetails(extendedClientDetails -> {
                attachOrCreateWebComponentWrapper(
                        webComponentConfiguration.get(), event, true);
            });

Why is this in retrieve extended details as the details aren't used even though retreived?
For clarity on the functionality attachOrCreate should take in the extendedDetails so for non preserve we would give internals.getExtended and for this we would give the extendede result.
Or better yet just give the calculated hash.

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 191 at r1 (raw file):

    }

    private void attachOrCreateWebComponentWrapper(

Break to attachOrCreate and Create where non preserved calls only create and preserved calls attachOrCreate which uses create.

    private void attachOrCreateWebComponentWrapper(
            WebComponentConfiguration<? extends Component> configuration,
            WebComponentConnectEvent event, final String hash) {
        Element elementToAttach = null;

        Optional<Element> old = getRegistry().get(hash);
        if (old.isPresent()) {
            elementToAttach = old.get().removeFromTree();
        }

        // did not have an element in the cache, create a new one
        if (elementToAttach == null){
            attachOrCreateWebComponentWrapper(configuration, event);
        }

        getRegistry().put(hash, elementToAttach);
    }
    private void attachOrCreateWebComponentWrapper(
            WebComponentConfiguration<? extends Component> configuration,
            WebComponentConnectEvent event) {

        Element rootElement = new Element(event.getTag());
        WebComponentBinding binding = configuration
                    .createWebComponentBinding(Instantiator.get(this),
                            rootElement, event.getAttributeJson());
        WebComponentWrapper wrapper = new WebComponentWrapper(rootElement,
                    binding);

        elementToAttach = wrapper.getElement();

        attachComponentToUI(elementToAttach, event.webComponentElementId);
    }

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 363 at r1 (raw file):

            Objects.requireNonNull(identifier);
            Objects.requireNonNull(element);
            if (!cache.containsKey(identifier)) {

Use ReentrantLock for put and get to not get mistakes due to threading.

    private final ReentrantLock lock = new ReentrantLock(true);

....
try {
lock.lock();
            if (!cache.containsKey(identifier)) {
                cache.put(identifier, element);
            }
}finally{
lock.unlock();
}

[ujoni]

Reviewable status: 8 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @caalador, @ujoni, and @vaadin-bot)

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 187 at r1 (raw file):

Previously, caalador wrote…

            getPage().retrieveExtendedClientDetails(extendedClientDetails -> {
                attachOrCreateWebComponentWrapper(
                        webComponentConfiguration.get(), event, true);
            });

Why is this in retrieve extended details as the details aren't used even though retreived?
For clarity on the functionality attachOrCreate should take in the extendedDetails so for non preserve we would give internals.getExtended and for this we would give the extendede result.
Or better yet just give the calculated hash.

Done.

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 191 at r1 (raw file):

Previously, caalador wrote…

Break to attachOrCreate and Create where non preserved calls only create and preserved calls attachOrCreate which uses create.

    private void attachOrCreateWebComponentWrapper(
            WebComponentConfiguration<? extends Component> configuration,
            WebComponentConnectEvent event, final String hash) {
        Element elementToAttach = null;

        Optional<Element> old = getRegistry().get(hash);
        if (old.isPresent()) {
            elementToAttach = old.get().removeFromTree();
        }

        // did not have an element in the cache, create a new one
        if (elementToAttach == null){
            attachOrCreateWebComponentWrapper(configuration, event);
        }

        getRegistry().put(hash, elementToAttach);
    }
    private void attachOrCreateWebComponentWrapper(
            WebComponentConfiguration<? extends Component> configuration,
            WebComponentConnectEvent event) {

        Element rootElement = new Element(event.getTag());
        WebComponentBinding binding = configuration
                    .createWebComponentBinding(Instantiator.get(this),
                            rootElement, event.getAttributeJson());
        WebComponentWrapper wrapper = new WebComponentWrapper(rootElement,
                    binding);

        elementToAttach = wrapper.getElement();

        attachComponentToUI(elementToAttach, event.webComponentElementId);
    }

Done.

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 360 at r1 (raw file):

Previously, vaadin-bot (Vaadin Bot) wrote…

Document this public method by adding an explicit description.

Done.

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 363 at r1 (raw file):

Previously, caalador wrote…

Use ReentrantLock for put and get to not get mistakes due to threading.

    private final ReentrantLock lock = new ReentrantLock(true);

....
try {
lock.lock();
            if (!cache.containsKey(identifier)) {
                cache.put(identifier, element);
            }
}finally{
lock.unlock();
}

Done. With a slightly different approach

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 364 at r1 (raw file):

Previously, vaadin-bot (Vaadin Bot) wrote…

Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in com.vaadin.flow.component.webcomponent.WebComponentUI$SessionEmbeddedComponentRegistry.put(String, Element)

Done.

---

flow-server/src/main/java/com/vaadin/flow/component/webcomponent/WebComponentUI.java, line 368 at r1 (raw file):

Previously, vaadin-bot (Vaadin Bot) wrote…

Document this public method by adding an explicit description.

Done.

[caalador]

Reviewed 1 of 1 files at r2.
Dismissed @ujoni and @vaadin-bot from 3 discussions.
Reviewable status: 3 unresolved discussions, 0 of 1 LGTMs obtained (waiting on @ujoni)

[ujoni]

This would be compatible with 2.0.x line, except that a new way to use @PreserveOnRefresh annotation has been introduced. As the annotation can now be attached to WebComponentExporter extenders, the change is for 2.1.x.

Before merge:

• Update @PreserveOnRefresh documentation, unless we want to keep this feature as experimental for a while.

[ujoni]

Remember to write vaadin/flow-and-components-documentation#800 for this feature to be considered complete.

[vaadin-bot]

SonarQube analysis reported 1 issue

• 1 minor

Watch the comments in this conversation to review them.

[caalador]

Reviewed 9 of 10 files at r3, 1 of 1 files at r5.
Dismissed @vaadin-bot from a discussion.
Reviewable status: 1 unresolved discussion, 1 of 1 LGTMs obtained (waiting on @ujoni)

[ujoni]

Dismissed @vaadin-bot from a discussion.
Reviewable status: complete! all discussions resolved, 1 of 1 LGTMs obtained