Skip to content

Commit

Permalink
Add transmission semantic for URI attribute values (#8826)
Browse files Browse the repository at this point in the history
* Add transmission semantic for URI attribute values

Fixes #8813

* Add some comments, correct javadocs

* Fix GWT tests

* Use one magic method instead of another

* Check the type of JsonValue before read it
  • Loading branch information
Denis authored Aug 12, 2020
1 parent 5cd56f6 commit 80e3194
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 33 deletions.
8 changes: 4 additions & 4 deletions flow-client/src/main/java/com/vaadin/client/WidgetUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public static String toPrettyJson(JsonValue json) {
* {@code value}.
* <p>
* If {@code value} is {@code null} then {@code attribute} is removed,
* otherwise {@code value.toString()} is set as its value.
* otherwise {@code value} is set as its value.
*
* @param element
* the DOM element owning attribute
Expand All @@ -136,11 +136,11 @@ public static String toPrettyJson(JsonValue json) {
* the value to update
*/
public static void updateAttribute(Element element, String attribute,
Object value) {
String value) {
if (value == null) {
DomApi.wrap(element).removeAttribute(attribute);
} else {
DomApi.wrap(element).setAttribute(attribute, value.toString());
DomApi.wrap(element).setAttribute(attribute, value);
}
}

Expand Down Expand Up @@ -227,7 +227,7 @@ public static native boolean hasJsProperty(Object object, String name)
/**
* Checks if the given value is explicitly undefined. <code>null</code>
* values returns <code>false</code>.
*
*
* @param property
* the value to be verified
* @return <code>true</code> is the value is explicitly undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.core.client.Scheduler;

import com.vaadin.client.ApplicationConfiguration;
import com.vaadin.client.Command;
import com.vaadin.client.Console;
import com.vaadin.client.ElementUtil;
Expand Down Expand Up @@ -290,9 +291,9 @@ private native void bindPolymerModelProperties(StateNode node,
private native void hookUpPolymerElement(StateNode node, Element element)
/*-{
var self = this;
var originalPropertiesChanged = element._propertiesChanged;
if (originalPropertiesChanged) {
element._propertiesChanged = function (currentProps, changedProps, oldProps) {
$entry(function () {
Expand All @@ -301,16 +302,16 @@ private native void hookUpPolymerElement(StateNode node, Element element)
originalPropertiesChanged.apply(this, arguments);
};
}
var tree = [email protected]::getTree()();
var originalReady = element.ready;
element.ready = function (){
originalReady.apply(this, arguments);
@com.vaadin.client.PolymerUtils::fireReadyEvent(*)(element);
// The _propertiesChanged method which is replaced above for the element
// doesn't do anything for items in dom-repeat.
// Instead it's called with some meaningful info for the <code>dom-repeat</code> element.
Expand All @@ -319,7 +320,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
// which changes this method for any dom-repeat instance.
var replaceDomRepeatPropertyChange = function(){
var domRepeat = element.root.querySelector('dom-repeat');
if ( domRepeat ){
// If the <code>dom-repeat</code> element is in the DOM then
// this method should not be executed anymore. The logic below will replace
Expand All @@ -333,12 +334,12 @@ private native void hookUpPolymerElement(StateNode node, Element element)
// if dom-repeat is found => replace _propertiesChanged method in the prototype and mark it as replaced.
if ( !domRepeat.constructor.prototype.$propChangedModified){
domRepeat.constructor.prototype.$propChangedModified = true;
var changed = domRepeat.constructor.prototype._propertiesChanged;
domRepeat.constructor.prototype._propertiesChanged = function(currentProps, changedProps, oldProps){
changed.apply(this, arguments);
var props = Object.getOwnPropertyNames(changedProps);
var items = "items.";
var i;
Expand All @@ -359,7 +360,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
if( currentPropsItem && currentPropsItem.nodeId ){
var nodeId = currentPropsItem.nodeId;
var value = currentPropsItem[propertyName];
// this is an attempt to find the template element
// which is not available as a context in the protype method
var host = this.__dataHost;
Expand All @@ -370,7 +371,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
while( !host.localName || host.__dataHost ){
host = host.__dataHost;
}
$entry(function () {
@SimpleElementBindingStrategy::handleListItemPropertyChange(*)(nodeId, host, propertyName, value, tree);
})();
Expand All @@ -381,7 +382,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
};
}
};
// dom-repeat doesn't have to be in DOM even if template has it
// such situation happens if there is dom-if e.g. which evaluates to <code>false</code> initially.
// in this case dom-repeat is not yet in the DOM tree until dom-if becomes <code>true</code>
Expand All @@ -396,7 +397,7 @@ private native void hookUpPolymerElement(StateNode node, Element element)
element.addEventListener('dom-change',replaceDomRepeatPropertyChange);
}
}
}-*/;

private static void handleListItemPropertyChange(double nodeId,
Expand Down Expand Up @@ -630,16 +631,21 @@ private void updateVisibility(JsArray<EventRemover> listeners,
private void updateVisibility(Element element, NodeMap visibilityData,
Boolean visibility) {
storeInitialHiddenAttribute(element, visibilityData);
WidgetUtil.updateAttribute(element, HIDDEN_ATTRIBUTE, visibility);
updateAttributeValue(
visibilityData.getNode().getTree().getRegistry()
.getApplicationConfiguration(),
element, HIDDEN_ATTRIBUTE, visibility);
}

private void restoreInitialHiddenAttribute(Element element,
NodeMap visibilityData) {
MapProperty initialVisibility = storeInitialHiddenAttribute(element,
visibilityData);
if (initialVisibility.hasValue()) {
WidgetUtil.updateAttribute(element, HIDDEN_ATTRIBUTE,
initialVisibility.getValue());
updateAttributeValue(
visibilityData.getNode().getTree().getRegistry()
.getApplicationConfiguration(),
element, HIDDEN_ATTRIBUTE, initialVisibility.getValue());
}
}

Expand Down Expand Up @@ -734,7 +740,10 @@ private void updateStyleProperty(MapProperty mapProperty, Element element) {

private void updateAttribute(MapProperty mapProperty, Element element) {
String name = mapProperty.getName();
WidgetUtil.updateAttribute(element, name, mapProperty.getValue());
updateAttributeValue(
mapProperty.getMap().getNode().getTree().getRegistry()
.getApplicationConfiguration(),
element, name, mapProperty.getValue());
}

private EventRemover bindChildren(BindingContext context) {
Expand Down Expand Up @@ -1366,6 +1375,35 @@ private EventRemover bindClientCallableMethods(BindingContext context) {
(Element) context.htmlNode, context.node);
}

private static void updateAttributeValue(
ApplicationConfiguration configuration, Element element,
String attribute, Object value) {
if (value == null || value instanceof String) {
WidgetUtil.updateAttribute(element, attribute, (String) value);
} else {
JsonValue jsonValue = WidgetUtil.crazyJsoCast(value);
if (JsonType.OBJECT.equals(jsonValue.getType())) {
JsonObject object = (JsonObject) jsonValue;
assert object.hasKey(
NodeProperties.URI_ATTRIBUTE) : "Implementation error: JsonObject is recieved as an attribute value for '"
+ attribute + "' but it has no "
+ NodeProperties.URI_ATTRIBUTE + " key";
String uri = object.getString(NodeProperties.URI_ATTRIBUTE);
if (configuration.isWebComponentMode()) {
String baseUri = configuration.getServiceUrl();
baseUri = baseUri.endsWith("/") ? baseUri : baseUri + "/";
WidgetUtil.updateAttribute(element, attribute,
baseUri + uri);
} else {
WidgetUtil.updateAttribute(element, attribute, uri);
}
} else {
WidgetUtil.updateAttribute(element, attribute,
value.toString());
}
}
}

private static EventExpression getOrCreateExpression(
String expressionString) {
if (expressionCache == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.function.Consumer;
import java.util.function.Supplier;

import com.vaadin.client.ApplicationConfiguration;
import com.vaadin.client.ClientEngineTestBase;
import com.vaadin.client.Registry;
import com.vaadin.client.WidgetUtil;
Expand Down Expand Up @@ -66,6 +67,8 @@ protected void gwtSetUp() throws Exception {
registry = new Registry() {
{
set(ConstantPool.class, new ConstantPool());
set(ApplicationConfiguration.class,
new ApplicationConfiguration());
}
};

Expand Down Expand Up @@ -138,7 +141,8 @@ public void testClientCallablePromises() {
delayTestFinish(100);
}

private static native void addThen(Object promise, Consumer<String> callback)
private static native void addThen(Object promise,
Consumer<String> callback)
/*-{
promise.then($entry(function(value) {
callback.@Consumer::accept(*)(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.vaadin.client.flow;

import com.vaadin.client.ApplicationConfiguration;
import com.vaadin.client.ClientEngineTestBase;
import com.vaadin.client.ExistingElementMap;
import com.vaadin.client.Registry;
Expand Down Expand Up @@ -91,6 +92,8 @@ protected void gwtSetUp() throws Exception {
{
set(ConstantPool.class, new ConstantPool());
set(ExistingElementMap.class, new ExistingElementMap());
set(ApplicationConfiguration.class,
new ApplicationConfiguration());
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.ArrayList;
import java.util.List;

import com.vaadin.client.ApplicationConfiguration;
import com.vaadin.client.ClientEngineTestBase;
import com.vaadin.client.ExistingElementMap;
import com.vaadin.client.InitialPropertiesHandler;
Expand Down Expand Up @@ -58,6 +59,7 @@ private static class TestRegistry extends Registry {
ExistingElementMap existingElementMap) {
this.constantPool = constantPool;
this.existingElementMap = existingElementMap;
set(ApplicationConfiguration.class, new ApplicationConfiguration());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.shared.Registration;

import elemental.json.Json;
import elemental.json.JsonObject;

/**
* Map for element attribute values.
*
Expand Down Expand Up @@ -65,8 +68,7 @@ public ElementAttributeMap(StateNode node) {
* the value
*/
public void set(String attribute, String value) {
unregisterResource(attribute);
put(attribute, value);
doSet(attribute, value);
}

/**
Expand Down Expand Up @@ -103,7 +105,19 @@ public Serializable remove(String attribute) {
*/
@Override
public String get(String attribute) {
return (String) super.get(attribute);
Serializable value = super.get(attribute);
if (value == null || value instanceof String) {
return (String) value;
} else {
// If the value is not a string then current impl only uses
// JsonObject
assert value instanceof JsonObject;
JsonObject object = (JsonObject) value;
// The only object which may be set by the current imlp contains
// "uri" attribute, only this situation is expected here.
assert object.hasKey(NodeProperties.URI_ATTRIBUTE);
return object.getString(NodeProperties.URI_ATTRIBUTE);
}
}

/**
Expand Down Expand Up @@ -142,7 +156,11 @@ private void doSetResource(String attribute,
} else {
targetUri = StreamResourceRegistry.getURI(resource);
}
set(attribute, targetUri.toASCIIString());
JsonObject object = Json.createObject();
object.put(NodeProperties.URI_ATTRIBUTE, targetUri.toASCIIString());
// don't use sring as a value, but wrap it into an object to let know
// the client side about specific nature of the value
doSet(attribute, object);
}

private void ensurePendingRegistrations() {
Expand Down Expand Up @@ -225,6 +243,11 @@ public void execute() {
}));
}

private void doSet(String attribute, Serializable value) {
unregisterResource(attribute);
put(attribute, value);
}

private void unsetResource(String attribute) {
ensureResourceRegistrations();
StreamRegistration registration = resourceRegistrations.get(attribute);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,14 @@ public final class NodeProperties {
*/
public static final String VISIBILITY_HIDDEN_PROPERTY = "hidden";

/**
* The property in Json object which marks the object as special value
* transmitting URI (not just any string).
* <p>
* Used in the {@link ElementAttributeMap}.
*/
public static final String URI_ATTRIBUTE = "uri";

private NodeProperties() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,18 @@
*/
package com.vaadin.flow.webcomponent;

import java.io.ByteArrayInputStream;
import java.util.Optional;

import com.vaadin.flow.component.AbstractField;
import com.vaadin.flow.component.AttachEvent;
import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.Tag;
import com.vaadin.flow.component.dependency.JsModule;
import com.vaadin.flow.component.html.Anchor;
import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.html.Span;
import com.vaadin.flow.server.StreamResource;

@JsModule("./src/Dependency.js")
public class ClientSelectComponent extends Div {
Expand All @@ -42,13 +45,26 @@ public ClientSelectComponent() {
select.addValueChangeListener(this::setValue);

add(select, message);
Anchor getPdf = new Anchor();
getPdf.setText("Download PDF");
getPdf.setId("link");
getPdf.setHref(createPdfStreamResource());
getPdf.getElement().setAttribute("download", true);
add(getPdf);
}

@Override
protected void onAttach(AttachEvent attachEvent) {
add(new DepElement());
}

private StreamResource createPdfStreamResource() {
StreamResource streamResource = new StreamResource("label.pdf",
() -> new ByteArrayInputStream(new byte[] { 1 }));
streamResource.setContentType("application/pdf");
return streamResource;
}

private void setValue(
AbstractField.ComponentValueChangeEvent<Select, String> event) {
String messageText = "No selection";
Expand Down
Loading

0 comments on commit 80e3194

Please sign in to comment.