Handle named parameters more consistently

[ettersi]

The current behaviour of named parameters is quite confusing.

This parameter lookup succeeds.

groovy:000> rsp = new ParameterRsp(new ParameterReq())
===> ParameterRsp[]
groovy:000> rsp.set(new NamedParameter("bar"), 42, true)
===> null
groovy:000> rsp.get(new NamedParameter("bar"))
===> 42

This parameter lookup also succeeds.

groovy:000> rsp = new ParameterRsp(new ParameterReq())
===> ParameterRsp[]
groovy:000> rsp.set(new NamedParameter("foo.bar"), 42, true)
===> null
groovy:000> rsp.get(new NamedParameter("bar"))
===> 42

This one fails.

groovy:000> rsp = new ParameterRsp(new ParameterReq())
===> ParameterRsp[]
groovy:000> rsp.set(new NamedParameter("spam.foo.bar"), 42, true)
===> null
groovy:000> rsp.get(new NamedParameter("bar"))
===> null

Finally, the last lookup would have succeeded if I had instead done this.

package spam;

enum foo implements org.arl.fjage.param.Parameter { bar }

/////////////////////////////////////////////////////

rsp = new ParameterRsp(new ParameterReq())
rsp.set(spam.foo.bar, 42, true)
rsp.get(new NamedParameter("bar"))

The difference between the last two lookups is particularly problematic because someAgentId.get(new NamedParameter("bar")) internally switches between the two depending on whether the bar parameter is defined as a Java object somewhere in the classpath. Therefore, pretty harmless-looking Fjage code may produce different results without throwing an error depending on whether it is run using java XXX -cp A or java XXX -cp B.

[mchitre]

groovy:000> rsp = new ParameterRsp(new ParameterReq())
===> ParameterRsp[]
groovy:000> rsp.set(new NamedParameter("spam.foo.bar"), 42, true)
===> null
groovy:000> rsp.get(new NamedParameter("bar"))
===> null

failing is clearly a bug, if this works:

groovy:000> rsp = new ParameterRsp(new ParameterReq())
===> ParameterRsp[]
groovy:000> rsp.set(new NamedParameter("foo.bar"), 42, true)
===> null
groovy:000> rsp.get(new NamedParameter("bar"))
===> 42

[notthetup]

Something in

fjage/src/main/java/org/arl/fjage/param/ParameterRsp.java

Lines 151 to 161 in 0d4bc3e

	protected Parameter resolve(Parameter param) {
	if (!(param instanceof NamedParameter)) return param;
	String p = ((NamedParameter)param).name();
	if (this.param != null && this.param.name().equals(p)) return this.param;
	if (values != null) {
	for (Entry<Parameter, GenericValue> e : values.entrySet()) {
	if (e.getKey().name().equals(p)) return e.getKey();
	}
	}
	return param;
	}

or

fjage/src/main/java/org/arl/fjage/param/NamedParameter.java

Lines 82 to 92 in 0d4bc3e

	public boolean equals(Object obj) {
	if (!(obj instanceof NamedParameter)) return false;
	NamedParameter p = (NamedParameter)obj;
	String name1 = name;
	if (name1.contains(".")) name1 = name1.substring(name1.indexOf('.')+1);
	String name2 = p.name;
	if (name2.contains(".")) name2 = name2.substring(name2.indexOf('.')+1);
	if (!name1.equals(name2)) return false;
	if (ord >= 0 && p.ord >= 0 && p.ord != ord) return false;
	return true;
	}

[ettersi]

groovy:000> rsp = new ParameterRsp(new ParameterReq())
===> ParameterRsp[]
groovy:000> rsp.set(new NamedParameter("spam.foo.bar"), 42, true)
===> null
groovy:000> rsp.get(new NamedParameter("bar"))
===> null

failing is clearly a bug, if this works:

groovy:000> rsp = new ParameterRsp(new ParameterReq())
===> ParameterRsp[]
groovy:000> rsp.set(new NamedParameter("foo.bar"), 42, true)
===> null
groovy:000> rsp.get(new NamedParameter("bar"))
===> 42

I thought this was by design? #285 (comment)

I believe the intention here was that NodeInfo.location should be reduced to location, but org.arl.unet.nodeinfo.NodeInfoParam.location should not be reduced to location.

[mchitre]

Not quite by design, but a residue from some fix.

As noted in the comment there: "We don't use qualified named parameters". That's where this problem comes from, since this issue deals with qualified parameter names, which aren't supported. We could, if we see a use case for them. Alternatively we can document that we don't support, and better yet, warn or throw an exception if someone defines a named parameter with qualified names?

[ettersi]

My objective here is to make it possible and reasonably easy to retrieve parameters from a Java gateway. AFAICT, there are two options to make this happen.

• Implement all parameters (including the .ext. ones) in java. If we go down this route, then we might want to consider making gateways throw an error if they receive parameters that they don't know about.
• Make sure NamedParameters work even for parameters that aren't implemented in Java.

The second option sounds like less work to me, but maybe I'm missing something?

[mchitre]

The second option is doable today without having to use qualified names with named params. Right?

[ettersi]

The second option is doable today without having to use qualified names with named params. Right?

No, because of this.

groovy:000> rsp = new ParameterRsp(new ParameterReq())
===> ParameterRsp[]
groovy:000> rsp.set(new NamedParameter("spam.foo.bar"), 42, true)
===> null
groovy:000> rsp.get(new NamedParameter("bar"))
===> null

When you do e.g. node.get(new NamedParameter("location")), you get back a org.arl.unet.nodeinfo.NodeInfoParam.location, and so rsp.get(new NamedParameter("location")) on that is going to give you null unless you have org.arl.unet.nodeinfo.NodeInfoParam.location in your classpath.

[mchitre]

My vote for making NamedParameter work for fully-qualified parameter names, and essentially being equivalent to an Enum parameter.