Mapper should not throw an error when explicitly ignoring read-only properties

[ghost]

Ran the command ./gradlew assemble and the annotation processor threw these warnings. Andreas Gudian suggested that they should not do so.

The unmapped properties are generated via the entities' constructors and are marked with @Transient. They do not contain accessor methods.

Thanks for reading.

---

Edit by @agudian to clarify what problem we're actually trying to fix here:

Read-Only properties are already ignored automatically and don't show up as unmapped target properties. But explicitly ignoring a read-only property using @Mapping(target=.., ignore=true) leads to the error "property X has no write accessor" and supresses any warnings about further unmapped target properties.

[gunnarmorling]

Could you share the definitions of your mapped beans and your mapper definition?

[ghost]

Thanks for the quick reply.

Let me know if you need the entire definitions.

[gunnarmorling]

Hum, we don't have the definition of DeckForm, so it's hard to tell. Does DeckForm declare all these properties that are defined within Deck? If not (or if they have different names), the generator doesn't know how to populate the properties in the target bean and thus raises this warning.

[ghost]

public class DeckForm {

@NotNull
private Long id;
@NotNull
@Size(min = 5, max = 50)
private String name;
@NotNull
private String legend;
//Prevent empty String being sent into the database, creating null value (edit deck only).
@NotNull
@Size(min = 1)
private String deckType;
@NotNull
@Size(min = 30, max = 30)
private List<Reference> cards;
@NotNull
private Reference author;
@NotNull
@Size(max = 10000)
@SafeHtml(additionalTags = "iframe")
private String description;

public Long getId() {
    return id;
}

public void setId(Long id) {
    this.id = id;
}

public String getName() {
    return name;
}

public void setName(String name) {
    this.name = name;
}

public String getLegend() {
    return legend;
}

public void setLegend(String legend) {
    this.legend = legend;
}

public String getDeckType() {
    return deckType;
}

public void setDeckType(String deckType) {
    this.deckType = deckType;
}

public List<Reference> getCards() {
    return cards;
}

public void setCards(List<Reference> cards) {
    this.cards = cards;
}

public Reference getAuthor() {
    return author;
}

public void setAuthor(Reference author) {
    this.author = author;
}

public String getDescription() {
    return description;
}

public void setDescription(String description) {
    this.description = description;
}

}

DeckForm only declares some properties that are defined within the deck.

I made sure the ones that are in the Form object have the same names, but the warning still shows up.

[gunnarmorling]

DeckForm only declares some properties that are defined within the deck.

That explains it. When mapping from DeckForm to Deck there is no way to populate the additional properties in Deck. Usually such unmapped properties are an indicator that something isn't working as intended, hence the warning. If that's not the case, you either can ignore single target properties via @Mapping#ignore() or you change the "unmapped target report policy" for the entire mapper in @Mapper.

I'm going to close this issue as it "works as designed".

[agudian]

@arminnaderi, on Gitter, you mentioned that the reported unmapped properties don't have any accessor methods and that setting them to ignore didn't help and instead raised the error "Property "creatureCount" has no write accessor.".

Could you please add the full declaration of Deck?

[ghost]

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@NotNull
@Size(min = 5, max = 30)
private String name;

@NotNull
private Legend legend;

@NotNull
private DeckType deckType;

@OneToMany
@NotNull
@Size(min = 30, max = 30)
private List<Card> cards;

@OneToOne
@JoinColumn(name = "author_id")
@NotNull
private User author;

@NotNull
@Size(min = 100, max = 1000)
private String description;

private int views;
private int rating;

@OneToMany(mappedBy = "deck")
private List<DeckReview> reviews;

//Image created to serve as og preview.
@Lob
@JsonIgnore
private byte[] image;

private LocalDateTime dateUploaded;
private LocalDateTime lastUpdated;

//COMPUTED
@Transient
@JsonIgnore
private int cost;

@Transient
@JsonIgnore
private int supportCount;

@Transient
@JsonIgnore
private int creatureCount;
//Make use of TreeMap since the support cards need to be rendered on the page in sorted order

@Transient
@JsonIgnore
private Map<Card, Integer> supportMapCount = new TreeMap<>();
//Make use of TreeMap since the creature cards need to be rendered on the page in sorted order

@Transient
@JsonIgnore
private Map<Card, Integer> creatureMapCount = new TreeMap<>();

@Transient
@JsonIgnore
private Map<RewardType, Integer> rewardMapAmount = new HashMap<>();

public Deck() {
}

public Deck(String name, Legend legend, DeckType deckType, List<Card> cards, User publisher, String description) {
    this(0L, name, legend, deckType, cards, publisher, null, null, 0, 0, description);
}

public Deck(Long id, String name, Legend legend, DeckType deckType, List<Card> cards, User author, LocalDateTime dateUploaded,
            LocalDateTime lastUpdated, int rating, int views, String description) {
    this.id = id;
    this.name = name;
    this.legend = legend;
    this.deckType = deckType;
    //TODO: call setCards() without NPE
    this.cards = cards;
    this.author = author;
    this.dateUploaded = dateUploaded;
    this.lastUpdated = lastUpdated;
    this.rating = rating;
    this.views = views;
    this.description = description;
    this.reviews = new ArrayList<>();
}

public Deck(Long id, String name, Legend legend, DeckType deckType, User publisher, LocalDateTime dateUploaded, LocalDateTime lastUpdated,
            int rating, int views, String description) {
    this(id, name, legend, deckType, null, publisher, dateUploaded, lastUpdated, rating, views, description);
}

public boolean isOutdated() {
    long daysBetween = ChronoUnit.DAYS.between(dateUploaded, LocalDateTime.now());
    return daysBetween > 30;
}

@Override
public Long getId() {
    return id;
}

public void setId(Long id) {
    this.id = id;
}

public String getName() {
    return name;
}

public String getFormattedName() {
    if (name.length() > TRUNCATE_LENGTH) {
        return name.substring(0, TRUNCATE_LENGTH) + "...";
    }
    return name;
}

public void setName(String name) {
    this.name = name;
}

public Legend getLegend() {
    return legend;
}

public void setLegend(Legend legend) {
    this.legend = legend;
}

public DeckType getDeckType() {
    return deckType;
}

public void setDeckType(DeckType deckType) {
    this.deckType = deckType;
}

public List<Card> getCards() {
    return cards;
}

//TODO: if you set a List of cards which have null values, NPE will occur
public void setCards(List<Card> cards) {
    rewardMapAmount.put(RewardType.GOLD, 0);
    rewardMapAmount.put(RewardType.ATTACK, 0);
    rewardMapAmount.put(RewardType.HEALTH, 0);
    rewardMapAmount.put(RewardType.ARMOR, 0);
    rewardMapAmount.put(RewardType.WEAPON, 0);

    this.cards = cards;
    int total = 0;

    for (Card card: cards) {
        total += card.getRarity().getCraftingCost();
        if (card.getCardType() == CardType.CREATURE) {
            creatureCount += 1;
            int count = creatureMapCount.containsKey(card) ? 2 : 1;
            creatureMapCount.put(card, count);


        } else {
            supportCount += 1;
            int count = supportMapCount.containsKey(card) ? 2 : 1;
            supportMapCount.put(card, count);
        }

        //Cache first & second reward
        Reward firstReward = card.getFirstReward();
        Reward secondReward = card.getSecondReward();
        //Add increment the reward map's counts if this card drops rewards
        if (firstReward != null) {
            int currentAmount = rewardMapAmount.get(firstReward.getRewardType());
            rewardMapAmount.put(firstReward.getRewardType(), currentAmount + firstReward.getAmount());
        }
        if (secondReward != null) {
            rewardMapAmount.put(secondReward.getRewardType(), rewardMapAmount.get(secondReward.getRewardType()) + secondReward.getAmount());
        }
    }

    cost = total;
}

public User getAuthor() {
    return author;
}

public void setAuthor(User author) {
    this.author = author;
}

public LocalDateTime getLastUpdated() {
    return lastUpdated;
}

public void setLastUpdated(LocalDateTime lastUpdated) {
    this.lastUpdated = lastUpdated;
}

public LocalDateTime getDateUploaded() {
    return dateUploaded;
}

public void setDateUploaded(LocalDateTime dateUploaded) {
    this.dateUploaded = dateUploaded;
}

public String getFormattedDateUploaded() {
    return DateUtils.formatDateOffsetFromNow(dateUploaded);
}

public String getFormattedLastUpdated() {
    return DateUtils.formatDateOffsetFromNow(lastUpdated);
}

public int getRating() {
    return rating;
}

public void setRating(int rating) {
    this.rating = rating;
}

public int getViews() {
    return views;
}

public void setViews(int views) {
    this.views = views;
}

public int getCost() {
    return cost;
}

public void setCost(int cost) {
    this.cost = cost;
}

public String getDescription() {
    return description;
}

public void setDescription(String description) {
    this.description = description;
}

public byte[] getImage() {
    return image;
}

public void setImage(byte[] image) {
    this.image = image;
}

//COMPUTED getters and setters

public int getSupportCount() {
    return supportCount;
}

public int getCreatureCount() {
    return creatureCount;
}

public Map<Card, Integer> getSupportMapCount() {
    return supportMapCount;
}

public Map<Card, Integer> getCreatureMapCount() {
    return creatureMapCount;
}

public Map<RewardType, Integer> getRewardMapAmount() {
    return rewardMapAmount;
}

public void setRewardMapAmount(Map<RewardType, Integer> rewardMapAmount) {
    this.rewardMapAmount = rewardMapAmount;
}

public List<DeckReview> getReviews() {
    return reviews;
}

public void setReviews(List<DeckReview> reviews) {
    this.reviews = reviews;
}

[ghost]

@agudian Yes that is true, here is the implementation.

[filiphr]

@agudian, @gunnarmorling I think that the solution for #833 was wrong. If there is a readAccessor for a property we always throw an error, even when the property is explicitly ignored. Just have a look at the IgnorePropertyTest.shouldGiveWringingOnUnmapped. I think that the problem is that we first do the checks if there is a readAccessor or unknown properties and then we check for ignored.

I think that we shouldn't fail the compilation when a property is ignored. Maybe even implicitly ignore properties without a write accessor.

@arminnaderi Your question in gitter about why you couldn't find the implemented mapper is (I think) that the mappers were not generated for your mapping due to the outputted error.

[agudian]

Your question in gitter about why you couldn't find the implemented mapper is (I think) that the mappers were not generated for your mapping due to the outputted error.

I think we resolved that in the chat.

But anyway, I think we have a bug here that needs to be fixed. I'll reopen the issue.

[gunnarmorling]

Ok, but can we then first need to get a reproducer of the issue an it seems to be something different than indicated by the original description. Is it related to usage of `ignore()` for a non-existent target write accessor?

[filiphr]

Yes it is related to properties which are ignored and have no target write accessor. However, by looking at the issue again, I do not think that it is related to what I am saying (maybe I am even wrong and it works as designed).

The OP has a warning that some properties were not mapped which is correct, as his Deck has more properties than his DeckForm and thus we display the warning. If the unmappedTargetPolicy is set to IGNORE then the warnings will be gone or if he explicitly ignores those properties via @Mapping.

[agudian]

I'll create a reproducer...

[agudian]

I can reproduce that some read-only properties are listed in the "unmapped target properties" message (we'll fix that), but I can't reproduce the has no write accessor message.

@arminnaderi, could you please share the mapper with the @Mapping annotations that led to the error?

[ghost]

@agudian Sure, one moment, I have to check my VCS.

[ghost]

@Mapper(componentModel = "spring", uses = { ReferenceMapper.class })
public interface DeckMapper {
    DeckForm deckToDeckForm(Deck deck);

    @Mappings({
            @Mapping(target = "cost", ignore = true),
            @Mapping(target = "supportCount", ignore = true),
            @Mapping(target = "creatureCount", ignore = true),
            @Mapping(target = "supportMapCount", ignore = true),
            @Mapping(target = "creatureMapCount", ignore = true),
            @Mapping(target = "rewardMapAmount", ignore = true)

    })
    Deck deckFormToDeck(DeckForm deckForm);
}

[ghost]

I believe what happened is that I ignored the @Transient properties withDeck with ignore = true, and the

has no write accessor

message appeared which seemed strange to me at the time as the properties do not need to be accessed if they are ignored.

[agudian]

Okay, I'm seeing it now as well. Apperently, when only a subset of the unmapped properties is ignored, that error shows up... 😏

This is what you want to use in your application:

    @Mappings({
        @Mapping(target = "lastUpdated", ignore = true),
        @Mapping(target = "dateUploaded", ignore = true),
        @Mapping(target = "rating", ignore = true),
        @Mapping(target = "views", ignore = true),
        @Mapping(target = "cost", ignore = true),
        @Mapping(target = "image", ignore = true),
        @Mapping(target = "rewardMapAmount", ignore = true),
        @Mapping(target = "reviews", ignore = true),
        @Mapping(target = "supportMapCount", ignore = true),
        @Mapping(target = "creatureMapCount", ignore = true)
    })
    Deck toDeck(DeckForm form);

[ghost]

Sounds good, I'll try it out. Thanks!

[agudian]

I've edited the title and the description of the issue to clarify what we actually need to fix here.

Remark: the maps are reported as unmapped target properties, as they could be modified using .put(..) if no setter is available. Properties like outdated, creatureCount etc are not reported as unmapped target properties, because they can't be modified like maps or collections and have no setters.

[ghost]

Thanks for all of your support guys, it is truly amazing. You all seem highly dedicated to this project. A rare sight. And MapStruct is an amazing project.

[agudian]

Thanks for all of your support guys, it is truly amazing. You all seem highly dedicated to this project. A rare sight. And MapStruct is an amazing project.

Thanks for the nice feedback! We really appreciate that!

I've pushed a fix to our 1.1 branch and our master just now. So I'll close this issue.