Skip to content

Conversation

@jungm
Copy link
Member

@jungm jungm commented Jan 6, 2026

#47 changed the behavior of ObjectRecipe to copy all properties into a TreeMap, this obviously caused the original order to be lost and a case like this (happens regularly in TomEE) is broken because the last property properties is processed too early:

    @Test
    public void testPropertyOrderNotAlteredCaseInsensitive() {
        ObjectRecipe recipe = new ObjectRecipe(CatchAll.class);
        recipe.allow(Option.CASE_INSENSITIVE_PROPERTIES);

        recipe.setProperty("123", "a");
        recipe.setProperty("zzz", 1);
        recipe.setProperty("aaa", 2);
        recipe.setProperty("properties", new UnsetPropertiesRecipe());

        CatchAll catchAll = (CatchAll) recipe.create();

        assertEquals(3, catchAll.properties.size());
    }

@rzo1 rzo1 requested a review from rmannibucau January 6, 2026 14:23
/**
* LinkedHashMap that normalizes keys using a function to for example make a case-insensitive LinkedHashMap
*/
public class NormalizedLinkedHashMap<K,V> extends LinkedHashMap<K, V> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from my XP this kind of extension is always a promise of later issue cause you miss to extend some new API for future PR

we only need iterator() and get() (merge of contains+get/remove in the code), what about using a new interface PropertiesAccessor { Iterator<Property> iterator(), Property poll(String name); } or alike and back it with the treemap+properties (iterator)?

sounds more future proof

else looks good to me

@fpapon fpapon changed the title XBEAN-453 revision - don't alter property order in case-insensitive mode XBEAN-353 revision - don't alter property order in case-insensitive mode Jan 7, 2026
@jungm jungm requested a review from rmannibucau January 8, 2026 07:16
@rzo1 rzo1 merged commit 16ce6c0 into apache:trunk Jan 8, 2026
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

Successfully merging this pull request may close these issues.

3 participants