Skip to content

Commit

Permalink
fix: incorrect copying behavior when copying view containers, optimiz…
Browse files Browse the repository at this point in the history
…ation to prevent graph traversal when copying already-immutable containers (or immutable views)
  • Loading branch information
Steanky committed Aug 12, 2024
1 parent 8b02dee commit ea21edb
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ plugins {
}

group 'com.github.steanky'
version '0.26.0'
version '0.26.1'

publishing {
publications {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ final class ConfigContainers {
* @return an immutable copy of the original
*/
static @NotNull ConfigContainer immutableCopy(@NotNull ConfigContainer original) {
if (original instanceof Immutable) {
return original;
}

return Graph.process(original, (ConfigElement node) -> {
ConfigContainer configContainer = node.asContainer();
Collection<ConfigEntry> entryCollection = configContainer.entryCollection();
Expand Down Expand Up @@ -111,11 +115,16 @@ public void accept(String s, ConfigElement element, boolean circular) {
* @return an immutable view of the provided container
*/
static @NotNull ConfigContainer immutableView(@NotNull ConfigContainer container) {
if (container instanceof ImmutableView) {
return container;
}

return Graph.process(container, (ConfigElement node) -> {
ConfigContainer configContainer = node.asContainer();
Collection<ConfigEntry> entryCollection = configContainer.entryCollection();

if (configContainer instanceof Immutable) {
// all Immutable are also ImmutableView, so we don't explore those either!
if (configContainer instanceof ImmutableView) {
return Graph.node(Iterators.iterator(), Graph.output(configContainer, Graph.emptyAccumulator()));
}

Expand All @@ -130,7 +139,7 @@ public void accept(String s, ConfigElement element, boolean circular) {
}, ConfigElement::isContainer, Function.identity(), Graph.Options.TRACK_REFERENCES).asContainer();
}

private static class ConfigNodeView extends AbstractConfigNode implements Immutable {
private static class ConfigNodeView extends AbstractConfigNode implements ImmutableView {
private final ConfigNode underlying;

private ConfigNodeView(ConfigNode underlying) {
Expand Down Expand Up @@ -191,7 +200,7 @@ public Set<Entry<String, ConfigElement>> entrySet() {
}
}

static final class ConfigListView extends AbstractConfigList implements Immutable, RandomAccess {
static final class ConfigListView extends AbstractConfigList implements RandomAccess, ImmutableView {
private final ConfigList underlying;

private ConfigListView(ConfigList underlying) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*
* @see ConfigList#defaulting(ConfigList, ConfigList)
*/
class DefaultingConfigList extends AbstractConfigList implements Immutable, RandomAccess {
class DefaultingConfigList extends AbstractConfigList implements RandomAccess {
private final ConfigList base;
private final ConfigList defaults;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*
* @see ConfigNode#defaulting(ConfigNode, ConfigNode)
*/
class DefaultingConfigNode extends AbstractConfigNode implements Immutable {
class DefaultingConfigNode extends AbstractConfigNode {
private final ConfigNode base;
private final ConfigNode defaults;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
/**
* Marker interface indicating a {@link ConfigContainer} implementation is immutable; i.e. its mutating methods, like
* {@link ConfigList#add(Object)} or {@link ConfigNode#put(Object, Object)}, will throw an exception if called, and
* cannot change the container itself. However, the contents may still change if there is an underlying container
* that is mutable.
* cannot change the container itself. Similarly, the reported contents of the container may <i>not</i> change over its
* lifetime.
* <p>
* Additionally, implementations must ensure that any sub-containers are also immutable.
* Implementations must ensure that any sub-containers are also immutable in this way.
* @see ImmutableView
*/
public interface Immutable {
public interface Immutable extends ImmutableView {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.github.steanky.ethylene.core.collection;

/**
* Marker interface used to indicate that the {@link ConfigContainer} implementation does not support direct
* modification. However, it <i>may</i> be modified if a backing container is changed.
* <p>
* Implementations must ensure that any sub-containers are also immutable in this way.
* @see Immutable
*/
public interface ImmutableView {
}

0 comments on commit ea21edb

Please sign in to comment.