Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for 1.04 multidimensional array encodings #161

Closed
bjakke opened this issue Sep 13, 2018 · 5 comments
Closed

Support for 1.04 multidimensional array encodings #161

bjakke opened this issue Sep 13, 2018 · 5 comments
Milestone

Comments

@bjakke
Copy link
Contributor

bjakke commented Sep 13, 2018

1.04 allows multidimensional arrays outside of Variants, e.g. as Structure fields. Part 6 section 5.2.5 adds logic on how such arrays are binaryencoded:

Multi- dimensional Arrays are encoded as an Int32 Array containing the dimensions followed by
a list of all the values in the Array. The total number of values is equal to the product of the
dimensions. The number of values is 0 if one or more dimension is less than or equal to 0. The
process for reconstructing the multi - dimensional array is described in 5.2.2.16.

So it somewhat behaves like multidimensional arrays inside a Variant. However with one crucial difference: in variant there is a field for the lenght on the actual value, in this new form that is missing and needs to be calculated.

Within a Variant a multidimensional array of String objects would be encoded as Int32, (each String individually in order per Variant spec), Int32 for ArrayDimensions lenght, (each Int32 separately for the array). This is equal for 2x normal 1-dim Array encoding rules of 5.2.5, as simpified: Int32, String array data, Int32, Int32 array data.

However the generic the multidim array encoding rule for the same multidim array of Strings is instead Int32, Int32 array data, String array data. This means it cannot be resolved by using a combination of 1-dim array encoding rules in Structure serializers, therefore we need new logic for encoding/decoding N dimensional arrays (and with a note that the 1-dim array encoding stays as is, i.e. the new logic only applies to 2+ dim arrays).

@bjakke
Copy link
Contributor Author

bjakke commented Sep 19, 2018

(note that the above is for binary encodings, it is a bit different in each encoding type)

The logic needs to be able to differentiate multidim arrays within Variant and outside of Variant, as in JSON encoding (#162) the form for some reason is different (inside Variant: always one-dim with Dimensions, i.e. about same as in Binary form; outside Variant: nested JSON arrays).

@bjakke
Copy link
Contributor Author

bjakke commented Sep 19, 2018

Also current IEncoder/Decoder interfaces and impls do define each basic type as scalar and 1-dim-array. However multidims cannot be done so (as there is N possible dimensions) as java does not have such construct (method taking variable dimensions, except Object). This means multidims have to be handled via the generic put(String fieldName, Object value, optional Class<?> type) methods.

It might be better to deprecate or remove all the other methods from those interfaces and just leave the generic one. Possibly converting the current specifics ones as abstract base class for different encoders (if such abstraction can be done). That way the base can hopefully just define scalars for the base types with a somewhat generic array handling code.

@bjakke
Copy link
Contributor Author

bjakke commented Sep 19, 2018

Also it seems there is almost identical versions of the put:

void put(String fieldName, Object o) throws EncodingException;
void put(String fieldName, Object o, Class<?> clazz) throws EncodingException;
	
void putObject(String fieldName, Object o) throws EncodingException;	
void putObject(String fieldName, Class<?> c, Object o) throws EncodingException;	

They do behave with different logic but the end result should be same. Only one of those versions should exists, and preferably it is just the generic put method.

Note: Stack codegen does use the more specific versions of the methods, if they are removed that needs some changing.

Also there are some communication specifics (Hello, ErrorMessage, Acknowledge) that are IEncodeables, but not Structures, therefore the handling order matters (i.e. Structures has to be checked before generic IEncodeables). Also some of them could maybe be handled outside of that interface as it primarily works for custom Structure encoders via AbstractStructure subtyping, and those do not need to handle Messages.

@bjakke
Copy link
Contributor Author

bjakke commented Oct 1, 2018

The following mapping in BinaryDecoder was removed as part of the refactorings (at least initially for now)

		if (clazz.equals(Object[].class)) {
			Variant[] varArray = getVariantArray(fieldName);
			Object[] objArray = new Object[varArray.length];
			for (int i = 0; i < varArray.length; i++)
				objArray[i] = varArray[i].getValue();
			return (T) objArray;
		}


		if (clazz.equals(Object.class)) {
			return (T) getVariant(fieldName).getValue();
		}

As they lacked counterpart in EncoderUtils.put logic (which is now in BinaryEncoder as part of #168 ), which would indicate that they were not in use. Same logic can be done by requesting Variant.class (or any dimensional array class version) and getting values from Variants manually.

@bjakke
Copy link
Contributor Author

bjakke commented Nov 6, 2018

Added the above logic back (commit was for #168) . Also included in the BinaryEncoder now as well. Logic is the same as with Variant.class encoding, but it puts/takes the Object to/from Variant. Can be used as alternative mapping vs. Variant for BaseDataType.

@bjakke bjakke added this to the 1.4.0 milestone Nov 13, 2018
@bjakke bjakke closed this as completed Nov 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant