Lombok: [FEATURE] Improve builder to allow to build from super class object

[TehBakker]

Would be great to be able to instantiate a builder from super class Object.

I've my class like this for now

@Getter
@Setter
@SuperBuilder
@EqualsAndHashCode
@ToString
public class PetData {
private String name;
}
@Getter
@Setter
@SuperBuilder
@EqualsAndHashCode(callSuper = true)
@ToString(callSuper = true)
public class DogData extends PetData {
private String dogRace;
}
If I've an object Pet that is alrady built, I would like to be able to initiate a Dog builder from that Pet object

private Dog buildDogFromPet(Pet myPet) {
return Dog.builder(myPet).dogRace("Auggie").build();
}
Or through a "super" setter.

[janrieke]

_Preliminary remark_: I'm not a lombok team member, but I contributed the @SuperBuilder feature.

So you have a cat, want to do some surgery and make a dog out of it, but it should keep its name, right?
I think it is theoretically possible with @SuperBuilder. However, I see two major problems:

1. How do we configure this feature? I mean, why should it be only the direct superclass that can be converted? Any superclass is legitimate if you consider this feature legitimate. Furthermore, as we cannot resolve types when lombok runs, we need that information in the annotations of both superclass and subclass, and it must be consistent. This will be beyond any comprehensibility, and I foresee hundreds of bug reports where people simply do not understand the configuration.
2. I do not think this is a very common use case. On the other hand, it would require a lot of work to implement such a feature and to maintain it. The cost/benefit ratio seems very high.

But -- good news -- I am pretty sure you can do it manually by using the toBuilder feature and customizing the builder classes of both superclass and subclass. You'll probably need at least one new method in each of them that is similar to the generated protected B $fillValuesFrom(C instance) method. Have a look at the delombok output to get a starting point.

If you can't get it to work, feel free to submit a question to StackOverflow, tag it with "lombok" and I'll have a look at it.

[TehBakker]

Actualy I've a Pet and I want to build a Dog from it.
As in my real code Pet is completly legit, I just want to be able to build the dog builder object from the pet object with the existing pet fields already set in my dog builder, so I just have to complete it with Dog specific fields and avoid unneeded manual copy of each pet field.

[janrieke]

Technically it makes no difference if you have a pet or any subclass of it as starting point. Also the solution I proposed stays the same.

[damiengontier]

I've the same or so needs. I often create POJO and extends it to add an id field and make it an entity for database.

One thing that could help this kind of copy would be to have this kind of builder param, or having a method on the builder that takes anything and try to get fields like:

private Dog buildDogFromPet(Pet myPet) {
  Dog.builder.from(myPet).build();
}

Because else, you'll have to create a constructor from parent object like:

public Dog(Pet pet) {
  super(pet.name);
}

Which is ok for one field or two but boring for more.

Like, directly from my code:

public JobEntity(Job job) {
  super(job.getUid(), job.getName(), job.getJar(), job.getEntrypoint(), job.getJob(), job.getParameters(), job.getConfiguration(), job.getFlags());
}

I've always IDE warning for "too long lines" juste because of this case.

[janrieke]

I do not doubt this could be a legitimate use case. I think it's just too complex to implement in Lombok considering the crazy generics stuff @SuperBuilder already generates right now. And it probably requires incomprehensible configuration via annotation params.

Together with the fact that it can be implemented manually by customizing the (abstract) builder class, I'd vote for 'no', but as I wrote I'm not a Lombok team member.

[rzwitserloot]

Yup, the room to expand what superbuilder does out of the box is quite limited, given that it's already fairly complicated. I don't see a feasible route without using resolution, and only extremely awesome, very common and massive boilerplate busting features warrant requiring resolution. Is there any way to do it without?

The two simplest ways I can think of both run into trouble:

Let the parent class generate it

@SuperBuilder(toSubtypeBuilder = {Dog.class}) class Pet {
    String name;
}

@SuperBuilder class Dog {
    String breed;
}

turns into:

class Pet {
    private final String name;
    // all the usual superbuilder stuff

    public Dog.DogBuilder<UHOH!> toDogBuilder() {
        return Dog.builder().name(this.name);
    }
}

The problem is that we don't know what generics Dog has. If we can assume it has none, or it has the exact same generics that the class we're in (in this case, Pet) has, we can possibly pull this off, though especially for normal builder, we have no idea what the various classes and builder methods are called, and we'll also run into trouble figuring out what prefixes to use.

The alternative strategy is to let the child generate is, which is even more problematic:

@SuperBuilder class Pet {
    String name;
}

@SuperBuilder(withSubtypeBuilder = {Pet.class}) class Dog {
    String breed;
}

turns into:

class Dog {
    private final String breed;
    // all the usual superbuilder stuff

    public static Dog.DogBuilder<ThisWeKnow> toDogBuilder(Pet pet) {
        return Dog.builder().name(pet.getName() /* UHOH*/);
    }
}

here we have no idea that name is a thing without being able to see the signatures of Pet, which we can't know about without resolution.

This feature regrettably is not worth keeping on the rolls as we'd likely never get around to it, unless we can find a plausible, easy to understand way to do it without resolution.

I'll leave the feature request up for another 2 weeks in case someone thinks of a way to get there, but if not: The use case is clear, but we cannot deliver this feature in a way that is maintainable and offers the minimum quality we require for lombok features, hence, we can't accept it :(

[janrieke]

I may have found a solution (I'm not yet fully convinced that there will be no problems with it, e.g. generics on the superclass?). It requires some modifications on how we generate the internal toBuilder methods, but as those are marked with a "$" it should be clear that anyone who messes around with those might get broken code on a lombok update.

Here's my solution (for Child extends Parent):

• protected B $fillValuesFrom(final C instance) will become protected B $fillValuesFrom(final Parent instance). This implies that builder subclasses do not override that method anymore, but simply overload it.
  
  
  
  • A nice side-effect of this is that we can get rid of the static helper method $fillValuesFromInstanceIntoBuilder, because we can integrate it into $fillValuesFrom.
  
  
• Add a new method to the Child class:

public static Child.ChildBuilder<?, ?> toBuilder(Parent instance) {
    return new Child.ChildBuilderImpl().$fillValuesFrom(instance);
}

This $fillValuesFrom method call will resolve to ParentBuilder::$fillValuesFrom at compile-time.

With this, we do not need any more information when generating the superclass' builder. It is sufficient to add something like @SuperBuilder(withSupertypeToBuilder = {Parent.class}) to the subclass.

[rzwitserloot]

@janrieke how do you handle the chain, though? Let's say I have:

@SuperBuilder class GrandParent {}
@SuperBuilder class Parent {}
@SuperBuilder class Child {}

How would the child class know to generate public static Child.ChildBuilder<?, ?> toBuilder(GrandParent instance) { ... }?

[janrieke]

If you put a @SuperBuilder(withSupertypeToBuilder = {GrandParent.class}) onto Child, it would just additionally generate this:

public static Child.ChildBuilder<?, ?> toBuilder(GrandParent instance) {
    return new Child.ChildBuilderImpl().$fillValuesFrom(instance);
}

That $fillValuesFrom method call will resolve to GrandParentBuilder::$fillValuesFrom at compile-time. I just manually checked that that code works. Of course that means that if you let lombok only generate a toBuilder(GrandParent) method and put in a Parent instance into it, this will _not_ copy the fields from Parent.

Problem is if GrandParent has type params. If GrandParent is the direct ancestor, we can copy them from the extends clause. But if it's not, like in this case, we cannot know that. So we'd just have to generate it in its raw form, potentially having raw-type warnings pop up for the user (and of course possibly causing cast exceptions at runtime).

[rzwitserloot]

What with how complex it is and how it's not going to work well for certain cases (generics on parent), and requires some fairly convoluted args, I'm not so sure this is a worthwhile addition to lombok.

If you're (@janrieke) excited about this addition by all means, I'll trust your judgement, but I'm tempted to write this one off as too hairy to dance around lack of resolution to deliver on it.

[janrieke]

I'm not too excited, but what I like is that this approach would actually simplify the current toBuilder() code even without the new feature.

So when I find the time for it, I think I'd first make the simplifications and thoroughly test that. Then it would be simple for users to manually customize the builder to add this. I'm also not convinced whether we should add this feature due to the type param issue.

[rzwitserloot]

I'll leave this open and on low-prio for a while. For future self / other curators of the buglist: If this has no further traffic for a few months, feel free to presume it can be closed.

[Jamby93]

I've the same or so needs. I often create POJO and extends it to add an id field and make it an entity for database.

One thing that could help this kind of copy would be to have this kind of builder param, or having a method on the builder that takes anything and try to get fields like:

private Dog buildDogFromPet(Pet myPet) {
  Dog.builder.from(myPet).build();
}

Because else, you'll have to create a constructor from parent object like:

public Dog(Pet pet) {
  super(pet.name);
}

Which is ok for one field or two but boring for more.

Like, directly from my code:

public JobEntity(Job job) {
  super(job.getUid(), job.getName(), job.getJar(), job.getEntrypoint(), job.getJob(), job.getParameters(), job.getConfiguration(), job.getFlags());
}

I've always IDE warning for "too long lines" juste because of this case.

That's exactly the same reason I came across this issue. To generalize this, we have created a really dirty function like this:

    private <B, C extends B> C copyProperties(B a, C b) {
        for (Field field : getAllFields(a.getClass())) {
            if (isStatic(field.getModifiers()))
                continue;
            try {
                field.setAccessible(true);
                field.set(b, field.get(a));
            } catch (IllegalAccessException e) {
                e.printStackTrace();
            }
        }
        return b;
    }

    private static Iterable<? extends Field> getAllFields(Class<?> aClass) {
        final List<Field> allFields = new ArrayList<>();
        Class<?> currentClass = aClass;
        while (currentClass != null) {
            final Field[] declaredFields = currentClass.getDeclaredFields();
            Collections.addAll(allFields, declaredFields);
            currentClass = currentClass.getSuperclass();
        }
        return allFields;
    }

That we use all around like this (example):

public BookJpa convertEntityForDB(Book book) {
    return copyProperties(book, BookJpa.builder().build()); //where needed, we also add ".additionalDBRelatedField(foo)" to Jpa builder before copying values from base class
}

That would be way cleaner in my opinion, if something like this would be possibile with @SuperBuilder:

public BookJpa convertEntityForDB(Book book) {
    return BookJpa.builder(book.toBuilder()).build()); //where needed, we could also add ".additionalDBRelatedField(foo)"  to Jpa builder _after_ copying values from base class, that feels "safer"
}

In the example, BookJpa stands simply for class that merely extends Book and adds some Jpa specific annotation to field/properties.

Adding an overload to the current builder() that accept a T.Builder where T is a parent class appears doable to me without too many generics headaches.

[toudidel]

What's the status of this issue?

[janrieke]

No progress in the meantime.
From my point of view, the path should be changing $fillValuesFrom as drafted above. From there on, users can implement this feature on their own by customizing their superbuilders.
I vote against a fully-automatic support for this feature in Lombok due to the described generics issues.

I could help reviewing PRs on this, but I don't have the time to implement it on my own at the moment.
And of course the final decision is on the Lombok maintainers, not on me.

[ahaverdings]

Isn't simply making the $fillValuesFromInstanceIntoBuilder method available in child builder not already a solution for this (make it protected)? With that method available in child builders you could do something like this:

public static abstract class ChildBuilder<C extends Child,
  B extends Child.ChildBuilder<C, B>> extends Parent.ParentBuilder<C, B> {


    public B fromParent(final Parent instance) {

        Parent.ParentBuilder.$fillValuesFromInstanceIntoBuilder(instance, this);

        return self();
    }

}


[rzwitserloot]

@ahaverdings That $ should be screaming at you: What? No! Don't!.

That it doesn't annoys me, but, them's the breaks. This is sufficiently complicated with nowhere near enough intent by contributors and maintainers to wade into the already extremely complicated Builder/SuperBuilder mechanism that I'm just going to close this; seems better than to keep an issue open for a decade.

[nacitar]

Found this ticket and needed precisely this. I had SuperBuilder of BaseOptions, and extended that to another type MoreOptions that are those and more. I wanted to be able to construct a builder of the derived type, MoreOptions, from an existing Options object. Then I could take the lesser options and build them out to include the additional options for the more expansive derived MoreOptions type.

I figured I could do something similar to what this ticket is asking to accomplish this. I came upon a few BeanUtils solutions like copyProperties and such but those weren't very Builder friendly either. The $fillValues stuff mentioned here caught my eye too, but indeed the $ screamed at me and I left that idea alone.

I'm not sure what the __best__ solution is, but if you end up on this ticket for similar reasons one _good_ solution is quite simple... JUST USE COMPOSITION. Make MoreOptions simply be a builder (not a SuperBuilder, and not inheriting from BasOptions) and give it a BaseOptions baseOptions member. Now people building MoreOptions objects will have more insight too because they can see when building it that these options are part of a different object which could perhaps be more applicable to what they're doing. Maybe this smaller object is _all_ they need?

Just seems like not being "clever" here is the way to go... at least it was for me. That said, if there is a solution to do the BaseOptions ==> MoreOptionsBuilder conversion with lombok at this point I'm still curious about it, but I think composition will still win out due to the semantics communicated.

This is the equivalent of my solution, but using the classes from the OP (simplified).

@Builder
class PetData {
    String name;
    String something;
}

@Builder
class DogData {
  PetData petData;
  int age;
}

Perfectly clear to me. You don't always want to hide the semantics of the base anyway.

This doesn't share a base type which could be an issue, though... but that's not so hard to fix if you want getters and are willing to make an interface for PetData, particularly if you use other lombok features to help:

public interface PetDataProvider {
  String getName();
  String getSomething();
}

@Getter
@Builder
class PetData implements PetDataProvider {
    String name;
    String something;
}

@Getter
@Builder
class DogData implements PetDataProvider {
  private interface ExcludeSettingsDelegate {
    // Because @Delegate doesn't check for collisions, we need to exclude this method so we can replace it
    String getSomething();
  }
  // the first type can be either PetDataProvider to just provide those getters, or PetData if you want extras from that type too
  @Delegate(types=PetDataProvider.class, excludes=ExcludeSettingsDelegate.class)
  PetData petData;
  int age;

  @Override
  public String getSomething() {
    // our own instead of petData.getSomething()
    return "Dog" + something;
  }
}

I included how to use @Delegate to help make this approach concise and to also exclude getters you want to define your own versions of in the derived class.

Hope this helps someone, especially considering this feature request doesn't appear to be something that's happening.

[janrieke]

For those who want to take the risk that it might break without notice with a new Lombok version, you can take a look at my answer at StackOverflow to a similar question.