Immutability

[lorenzleutgeb]

I'd like the DTOs generated by graphql-java-codegen to optionally be "immutable".

By "immutable" I mean that the generated classes do not have any setters. Very similar to to what Lombok does with its @Value annotation.

A generated class with this feature enabled should look like:

public class Person {
    // Everything as usual, except...
    // NO SETTERS IN HERE
}

If you approve the addition of this feature, I am happy to implement and PR it.

[kobylynskyi]

Hi @lorenzleutgeb
We can have another mapping config option (something like generateModelsImmutable with default = false)
We need to also consider that get() methods that return collection should return a copy of the collection.
If you would like to work on this feature, then please assign it to yourself. Thanks!

[lorenzleutgeb]

You're right. I might do it, but not today...

[joffrey-bion]

We need to also consider that get() methods that return collection should return a copy of the collection.

Maybe we could simply make an immutable copy of the collection at construction time?
That would avoid creating new objects at every get call (but it would eagerly duplicate collection properties even if never accessed).

[lorenzleutgeb]

@joffrey-bion it would be interesting to see benchmarks, this is certainly not the first time someone ha to make this decision... My uneducated guess would be to favour eager duplication in the constructor, because that would already break even in the case where every property is accessed at least once, right?

[xenoterracide]

I'm mostly looking for setter-less DTO's first, not too worried about the Array itself

[xenoterracide]

though maybe this code generator, could integrate with another code generator like immutables, don't know if that's possible.

[lorenzleutgeb]

Since this project already depends on Lombok, I quickly checked whether Lombok's @Value could be used. Turns out that it does not generate unmodifiable copies of collections, and even though requested, this feature will not be implemented. So, I'd like to say "let's just do what Lombok does" and propose to implement the feature without the rather complicated requirement of making collections unmodifiable.

[joffrey-bion]

@xenoterracide Yeah I wouldn't worry too much about defensive copies of collections, either.

@lorenzleutgeb Just to clarify a bit, the fact that this project depends on Lombok should be irrelevant, because we're talking about the generated code that will belong to the target project.
Also, Lombok is mostly a tool to help write and maintain more easily the code that people write by hand. Generating code that contains Lombok annotations is sort of pointless because it would need to then go through Lombok's generation, adding an extra step for no added maintainability.

[lorenzleutgeb]

Also, Lombok is mostly a tool to help write and maintain more easily the code that people write by hand. Generating code that contains Lombok annotations is sort of pointless because it would need to then go through Lombok's generation, adding an extra step for no added maintainability.

Yeah I'm aware. I didn't mean to suggest to generate code with Lombok annotations, but to generate code that is essentially equivalent to what Lombok would generate if @Value would be present.

[joffrey-bion]

@lorenzleutgeb Yes, I was just answering the first part of your comment. But I agree with you on your conclusion ;)

[kobylynskyi]

Hey, @lorenzleutgeb
In the description of this feature request, you mentioned that you might work on it. Do you still want to do that? I would appreciate your contribution. Thanks.

[lorenzleutgeb]

Yeah, I'd do it if you drop the requirement of instantiating unmodifiable collections and instead allow me to do it "the Lombok way", effectively just dropping setters but not having special getter implementations.

[kobylynskyi]

I am fine with Lombok approach

[lorenzleutgeb]

In order to avoid changes later on, let's also fix the "user interface": How would you like the configuration parameter to be called? I'd call it generateValue which might be a bit unintuitive.

[kobylynskyi]

How about ‘generateImmutableModels’ or ‘generateModelsImmutable’?

[lorenzleutgeb]

I think that generateImmutableModels (verb, adjective, noun) better fits other, already existing, names such as generateAsyncApi, so I'll choose that.

[lorenzleutgeb]

Just to confirm: Starting from version 2, together with the feature described in #148, MapStruct can now target immutable DTOs and will instantiate them via the builder 🎊