Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]Order in collection is not preserved while using useImmutableCollections option #153

Open
pawellabaj opened this issue Jun 8, 2023 · 8 comments

Comments

@pawellabaj
Copy link
Contributor

pawellabaj commented Jun 8, 2023

When a record has a component of the type that is the Set implementation keeping the order, the order is lost when using useImmutableCollections = true option.

Following test shows the problem:

import io.soabase.recordbuilder.core.RecordBuilder;

import java.util.Set;

@RecordBuilder
@RecordBuilder.Options(useImmutableCollections = true)
record OrderedSetRecord(Set<String> orderedSet) {}
import org.junit.jupiter.api.Test;

import java.util.LinkedHashSet;

import static org.assertj.core.api.Assertions.assertThat;

public class TestOrderedSetsBuilder {

    @Test
    void shouldKeepOrderInSetIfProvided() {
        // given
        var orderedSet = new LinkedHashSet<String>();
        orderedSet.add("C");
        orderedSet.add("B");
        orderedSet.add("A");

        // when
        var record = OrderedSetRecordBuilder.builder().orderedSet(orderedSet).build();

        // then
        assertThat(record.orderedSet()).containsExactly("C", "B", "A");
    }
}

Order is lost in java.util.Set#copyOf method used in generated __set method.

One of possible solution is te generate __set method as follow:

private static <T> Set<T> __set(Collection<? extends T> o) {
        return (o != null) ?  Collections.unmodifiableSet((Set<T>) o) : Set.of();
    }
pawellabaj added a commit to pawellabaj/record-builder that referenced this issue Jun 8, 2023
@freelon
Copy link
Contributor

freelon commented Jun 13, 2023

unmodifiableSet unfortunately only creates a view on the still modifiable original set. Meaning that from a reference to the underlying set the new "unmodifiable" set could change. This is ofc not desirable.

@pawellabaj
Copy link
Contributor Author

In most cases, reference to the original collection is not being kept and collection from the record is used.

Another possible solution would be to treat java.util.LinkedHashSet or com.google.common.collect.ImmutableSet in a special way.

Randgalt pushed a commit that referenced this issue Jul 3, 2023
@pawellabaj
Copy link
Contributor Author

pawellabaj commented Jul 4, 2023

Introducing the useUnmodifiableCollections option works for me. However, it didn't solve the issue reported here.
Possibly checking for specialized interfaces like SortedSet, etc. (see #151 ) is the solution.

@Randgalt
Copy link
Owner

Randgalt commented Jul 5, 2023

@pawellabaj could you provide a PR?

@pawellabaj
Copy link
Contributor Author

@Randgalt , I'm thinking of it.

Do you consider introducing Guava as a dependency to your project?

@Randgalt
Copy link
Owner

Randgalt commented Jul 5, 2023

Do you consider introducing Guava as a dependency to your project

No - we shouldn't do that. We could, however, do FQPN string comparisons if needed.

@pawellabaj
Copy link
Contributor Author

@Randgalt have you thought about introducing a plugins/extensions mechanism?
Extensions with the usage of any library could be implemented.

@Randgalt
Copy link
Owner

At this point I'd like to limit any new customizations. This library ran into a lot of problems with some of the recently added customizations.

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

No branches or pull requests

3 participants