disallow mutation of record types by clients

[gavinking]

We have some record types with array members. We should prevent mutation of the arrays by returning clones to the client.

@njr-11 @otaviojava do we need to do something similar in Jakarta Data?

[njr-11]

@njr-11 @otaviojava do we need to do something similar in Jakarta Data?

I checked and Jakarta Data mostly covers this, but I did identify 3 places that we can fix. I will open a PR there.

[njr-11]

This looks good. But also note there is another way for clients to mutate it. They could do,

mapping = new CompoundMapping(elements);
...
elements[0] = ...

Although someone doing that is probably trying to cause trouble.

[gavinking]

But also note there is another way for clients to mutate it

Yeah true so maybe the constructor should also clone.

But I was more worried about them trying to mutate the ones returned by EMF.getResultSetMappings(), where it's the provider creating the ResultSetMappings.

Anyway, good catch, I will fix it anyway.

[gavinking]

I will fix it anyway.

I'm now also cloning things in record constructors.

Keep finding the whole record construct to be far less useful than I had hoped. By the time you actually add all the extra boilerplate code to make them deeply immutable, they may as well just have been a class :-(

[njr-11]

Switched the assignee back to Gavin who authored this PR. It is likely I was mistakenly added as assignee when attempting to set the reviewer.

[beikov]

How about changing the record components to be lists instead of arrays to avoid the copying on read access? If we use List.copyOf() all the way, the copies are even avoided.

[gavinking]

How about changing the record components to be lists instead of arrays to avoid the copying on read access? If we use List.copyOf() all the way, the copies are even avoided.

Aaaahmm, sure, we could do that. Looks like we already used that pattern in at least one place in the Data APIs.

Having a tough time deciding if it's on balance better. It's slightly better for performance, slightly worse for type safety, so I dunno 🤷‍♂️