A MultiMap just comes back as:
"features":{"example-feature":{*
"params":{"param-name":["param-value1","param-value2"]}*
,"name":"example-feature","required":false},
"core":{*"params":{}*,"name":"core","required":true}}}
So accessible as *param-name[i]* in js, and easy enough for the client to
grab the first param value, if it's single valued.
If you return a list of param pairs, you'd have to potentially iterate over
the entire list, to find the parameter name/value pair, right? Seems like
the Multimap representation is cleaner.
The tests should into
org.apache.shindig.gadgets.servlet.GadgetsHandlerServiceTest, correct?
On Thu, Mar 31, 2011 at 2:06 AM, Ziv Horesh <[email protected]> wrote:
>
> The issue I concern with this change is the json representation of the
data.
> You can easily represent map or list, but not sure how mutimap will be
represented.
> Please add tests with multiple items with same key.
> My preferred solution here would be to return list of pairs and let the
client generate a map if needed.
> (Basically add something like the next function to Feature class:
List<FeatureParam> getParamPairs())
>
>
> On Wed, Mar 30, 2011 at 7:00 PM, <[email protected]> wrote:
>>
>> Reviewers: dev_shindig.apache.org,
>>
>> Description:
>> Currently when the container receives gadget metadata, that metadata
>> does not include the feature parameters. This prevents container-side
>> features that rely on gadget supplied parameters from being able to use
>> them. The patch adds getParams to the Feature interface in
>> org.apache.shindig.gadgets.servlet.GadgetsHandlerApi and adds Multimap
>> support to org.apache.shindig.protocol.conversion.BeanDelegator. The
>> resulting metadata includes feature params.
>>
>> Please review this at http://codereview.appspot.com/4314055/
>>
>> Affected files:
>>
java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java
>>
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java
>>
>>
>> ### Eclipse Workspace Patch 1.0
>> #P shindig-project
>> Index:
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java
>> ===================================================================
>> ---
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java
(revision 4252)
>> +++
java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetsHandlerApi.java
(working copy)
>> @@ -20,6 +20,8 @@
>>
>> import org.apache.shindig.common.uri.Uri;
>> import org.apache.shindig.protocol.conversion.BeanFilter.Unfiltered;
>> +
>> +import com.google.common.collect.Multimap;
>> // Keep imports clean, so it is clear what is used by API
>>
>> import java.util.List;
>> @@ -163,7 +165,7 @@
>> public String getName();
>> public boolean getRequired();
>> // TODO: Handle multi map if params are needed
>> - // public Multimap<String, String> getParams();
>> + public Multimap<String, String> getParams();
>> }
>>
>> public interface LinkSpec {
>> Index:
java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java
>> ===================================================================
>> ---
java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java
(revision 4252)
>> +++
java/common/src/main/java/org/apache/shindig/protocol/conversion/BeanDelegator.java
(working copy)
>> @@ -21,8 +21,10 @@
>> import com.google.common.base.Preconditions;
>> import com.google.common.collect.ImmutableList;
>> import com.google.common.collect.ImmutableMap;
>> +import com.google.common.collect.ImmutableMultimap;
>> import com.google.common.collect.ImmutableSet;
>> import com.google.common.collect.Maps;
>> +import com.google.common.collect.Multimap;
>>
>> import org.apache.commons.lang.StringUtils;
>> import org.apache.shindig.common.uri.Uri;
>> @@ -161,6 +163,21 @@
>> }
>> }
>>
>> + // Proxy each item in a map (map key is not proxied)
>> + if (source instanceof Multimap<?, ?>) {
>> + Multimap<?, ?> mapSource = (Multimap<?, ?>) source;
>> + if (!mapSource.isEmpty() && delegatedClasses.containsKey(
>> + mapSource.values().iterator().next().getClass())) {
>> + // Convert Map:
>> + ImmutableMultimap.Builder<Object, Object> mapBuilder =
ImmutableMultimap.builder();
>> + for (Map.Entry<?, ?> entry : mapSource.entries()) {
>> + mapBuilder.put(entry.getKey(),
createDelegator(entry.getValue(), apiInterface));
>> + }
>> + return (T) mapBuilder.build();
>> + } else {
>> + return (T) source;
>> + }
>> + }
>> // Proxy each item in a list
>> if (source instanceof List<?>) {
>> List<?> listSource = (List<?>) source;
>> @@ -250,6 +267,8 @@
>> type = paramType.getActualTypeArguments()[0];
>> } else if (Map.class.isAssignableFrom((Class<?>)
paramType.getRawType())) {
>> type = paramType.getActualTypeArguments()[1];
>> + } else if (Multimap.class.isAssignableFrom((Class<?>)
paramType.getRawType())) {
>> + type = paramType.getActualTypeArguments()[1];
>> }
>> }
>> return (Class<?>) type;
>> @@ -313,6 +332,18 @@
>> interfaceType = interfaceParamType.getActualTypeArguments()[1];
>> return validateTypes(dataType, interfaceType);
>> }
>> +
>> + if (Multimap.class.isAssignableFrom((Class<?>)
dataParamType.getRawType()) &&
>> + Multimap.class.isAssignableFrom((Class<?>)
interfaceParamType.getRawType())) {
>> + Type dataKeyType =
dataParamType.getActualTypeArguments()[0];
>> + Type interfaceKeyType =
interfaceParamType.getActualTypeArguments()[0];
>> + if (dataKeyType != interfaceKeyType ||
!PRIMITIVE_TYPE_CLASSES.contains(dataKeyType)) {
>> + return false;
>> + }
>> + dataType = dataParamType.getActualTypeArguments()[1];
>> + interfaceType =
interfaceParamType.getActualTypeArguments()[1];
>> + return validateTypes(dataType, interfaceType);
>> + }
>> // Only support Map and List generics
>> return false;
>> }
>>
>>
>
