Make PartialOptions immutable

- Switch to using OptionalInt/OptionalLong instead of @Nullable
   Long/Integers. I know IntelliJ complains, but it avoids the risk of
   implicit unboxing.

 - Instead of mutating PartialOptions, we now define a merge() function
   which returns the new options. This simplifies the logic in
   AddressRule a whole bunch.

src/main/java/dan200/computercraft/ComputerCraft.java

@@ -18,10 +18,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
-import java.util.Arrays;
+import java.util.*;
-import java.util.Collections;
-import java.util.EnumSet;
-import java.util.List;
 import java.util.concurrent.TimeUnit;
 
 @Mod( ComputerCraft.MOD_ID )
@@ -44,8 +41,8 @@ public final class ComputerCraft
     public static boolean httpEnabled = true;
     public static boolean httpWebsocketEnabled = true;
     public static List<AddressRule> httpRules = Collections.unmodifiableList( Arrays.asList(
-        AddressRule.parse( "$private", null, Action.DENY.toPartial() ),
+        AddressRule.parse( "$private", OptionalInt.empty(), Action.DENY.toPartial() ),
-        AddressRule.parse( "*", null, Action.ALLOW.toPartial() )
+        AddressRule.parse( "*", OptionalInt.empty(), Action.ALLOW.toPartial() )
     ) );
 
     public static int httpMaxRequests = 16;

src/main/java/dan200/computercraft/core/apis/http/options/Action.java

@@ -6,13 +6,17 @@
 package dan200.computercraft.core.apis.http.options;
 
 import javax.annotation.Nonnull;
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 
 public enum Action
 {
     ALLOW,
     DENY;
 
-    private final PartialOptions partial = new PartialOptions( this, null, null, null, null );
+    private final PartialOptions partial = new PartialOptions(
+        this, OptionalLong.empty(), OptionalLong.empty(), OptionalInt.empty(), OptionalInt.empty()
+    );
 
     @Nonnull
     public PartialOptions toPartial()

src/main/java/dan200/computercraft/core/apis/http/options/AddressRule.java

@@ -16,6 +16,7 @@
 import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.util.OptionalInt;
 import java.util.regex.Pattern;
 
 /**
@@ -29,10 +30,10 @@ public final class AddressRule
     public static final int WEBSOCKET_MESSAGE = 128 * 1024;
 
     private final AddressPredicate predicate;
-    private final Integer port;
+    private final OptionalInt port;
     private final PartialOptions partial;
 
-    private AddressRule( @Nonnull AddressPredicate predicate, @Nullable Integer port, @Nonnull PartialOptions partial )
+    private AddressRule( @Nonnull AddressPredicate predicate, OptionalInt port, @Nonnull PartialOptions partial )
     {
         this.predicate = predicate;
         this.partial = partial;
@@ -40,7 +41,7 @@ private AddressRule( @Nonnull AddressPredicate predicate, @Nullable Integer port
     }
 
     @Nullable
-    public static AddressRule parse( String filter, @Nullable Integer port, @Nonnull PartialOptions partial )
+    public static AddressRule parse( String filter, OptionalInt port, @Nonnull PartialOptions partial )
     {
         int cidr = filter.indexOf( '/' );
         if( cidr >= 0 )
@@ -72,7 +73,7 @@ else if( filter.equalsIgnoreCase( "$private" ) )
      */
     private boolean matches( String domain, int port, InetAddress address, Inet4Address ipv4Address )
     {
-        if( this.port != null && this.port != port ) return false;
+        if( this.port.isPresent() && this.port.getAsInt() != port ) return false;
         return predicate.matches( domain )
             || predicate.matches( address )
             || (ipv4Address != null && predicate.matches( ipv4Address ));
@@ -80,8 +81,7 @@ private boolean matches( String domain, int port, InetAddress address, Inet4Addr
 
     public static Options apply( Iterable<? extends AddressRule> rules, String domain, InetSocketAddress socketAddress )
     {
-        PartialOptions options = null;
+        PartialOptions options = PartialOptions.DEFAULT;
-        boolean hasMany = false;
 
         int port = socketAddress.getPort();
         InetAddress address = socketAddress.getAddress();
@@ -91,24 +91,9 @@ public static Options apply( Iterable<? extends AddressRule> rules, String domai
         for( AddressRule rule : rules )
         {
             if( !rule.matches( domain, port, address, ipv4Address ) ) continue;
-
+            options = options.merge( rule.partial );
-            if( options == null )
-            {
-                options = rule.partial;
-            }
-            else
-            {
-
-                if( !hasMany )
-                {
-                    options = options.copy();
-                    hasMany = true;
-                }
-
-                options.merge( rule.partial );
-            }
         }
 
-        return (options == null ? PartialOptions.DEFAULT : options).toOptions();
+        return options.toOptions();
     }
 }

src/main/java/dan200/computercraft/core/apis/http/options/AddressRuleConfig.java

@@ -14,6 +14,8 @@
 import javax.annotation.Nullable;
 import java.util.Locale;
 import java.util.Optional;
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 import java.util.concurrent.ConcurrentHashMap;
 
 /**
@@ -48,7 +50,7 @@ public static UnmodifiableConfig makeRule( String host, Action action )
     public static boolean checkRule( UnmodifiableConfig builder )
     {
         String hostObj = get( builder, "host", String.class ).orElse( null );
-        Integer port = get( builder, "port", Number.class ).map( Number::intValue ).orElse( null );
+        OptionalInt port = unboxOptInt( get( builder, "port", Number.class ) );
         return hostObj != null && checkEnum( builder, "action", Action.class )
             && check( builder, "port", Number.class )
             && check( builder, "timeout", Number.class )
@@ -65,11 +67,11 @@ public static AddressRule parseRule( UnmodifiableConfig builder )
         if( hostObj == null ) return null;
 
         Action action = getEnum( builder, "action", Action.class ).orElse( null );
-        Integer port = get( builder, "port", Number.class ).map( Number::intValue ).orElse( null );
+        OptionalInt port = unboxOptInt( get( builder, "port", Number.class ) );
-        Integer timeout = get( builder, "timeout", Number.class ).map( Number::intValue ).orElse( null );
+        OptionalInt timeout = unboxOptInt( get( builder, "timeout", Number.class ) );
-        Long maxUpload = get( builder, "max_upload", Number.class ).map( Number::longValue ).orElse( null );
+        OptionalLong maxUpload = unboxOptLong( get( builder, "max_upload", Number.class ).map( Number::longValue ) );
-        Long maxDownload = get( builder, "max_download", Number.class ).map( Number::longValue ).orElse( null );
+        OptionalLong maxDownload = unboxOptLong( get( builder, "max_download", Number.class ).map( Number::longValue ) );
-        Integer websocketMessage = get( builder, "websocket_message", Number.class ).map( Number::intValue ).orElse( null );
+        OptionalInt websocketMessage = unboxOptInt( get( builder, "websocket_message", Number.class ).map( Number::intValue ) );
 
         PartialOptions options = new PartialOptions(
             action,
@@ -122,6 +124,16 @@ private static <T extends Enum<T>> Optional<T> getEnum( UnmodifiableConfig confi
         return get( config, field, String.class ).map( x -> parseEnum( klass, x ) );
     }
 
+    private static OptionalLong unboxOptLong( Optional<? extends Number> value )
+    {
+        return value.map( Number::intValue ).map( OptionalLong::of ).orElse( OptionalLong.empty() );
+    }
+
+    private static OptionalInt unboxOptInt( Optional<? extends Number> value )
+    {
+        return value.map( Number::intValue ).map( OptionalInt::of ).orElse( OptionalInt.empty() );
+    }
+
     @Nullable
     private static <T extends Enum<T>> T parseEnum( Class<T> klass, String x )
     {

src/main/java/dan200/computercraft/core/apis/http/options/PartialOptions.java

@@ -5,21 +5,25 @@
  */
 package dan200.computercraft.core.apis.http.options;
 
-import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 
 public final class PartialOptions
 {
-    static final PartialOptions DEFAULT = new PartialOptions( null, null, null, null, null );
+    public static final PartialOptions DEFAULT = new PartialOptions(
+        null, OptionalLong.empty(), OptionalLong.empty(), OptionalInt.empty(), OptionalInt.empty()
+    );
 
-    Action action;
+    private final @Nullable Action action;
-    Long maxUpload;
+    private final OptionalLong maxUpload;
-    Long maxDownload;
+    private final OptionalLong maxDownload;
-    Integer timeout;
+    private final OptionalInt timeout;
-    Integer websocketMessage;
+    private final OptionalInt websocketMessage;
 
-    Options options;
+    private @Nullable Options options;
 
-    PartialOptions( Action action, Long maxUpload, Long maxDownload, Integer timeout, Integer websocketMessage )
+    public PartialOptions( @Nullable Action action, OptionalLong maxUpload, OptionalLong maxDownload, OptionalInt timeout, OptionalInt websocketMessage )
     {
         this.action = action;
         this.maxUpload = maxUpload;
@@ -28,31 +32,36 @@ public final class PartialOptions
         this.websocketMessage = websocketMessage;
     }
 
-    @Nonnull
     Options toOptions()
     {
         if( options != null ) return options;
 
         return options = new Options(
             action == null ? Action.DENY : action,
-            maxUpload == null ? AddressRule.MAX_UPLOAD : maxUpload,
+            maxUpload.orElse( AddressRule.MAX_UPLOAD ),
-            maxDownload == null ? AddressRule.MAX_DOWNLOAD : maxDownload,
+            maxDownload.orElse( AddressRule.MAX_DOWNLOAD ),
-            timeout == null ? AddressRule.TIMEOUT : timeout,
+            timeout.orElse( AddressRule.TIMEOUT ),
-            websocketMessage == null ? AddressRule.WEBSOCKET_MESSAGE : websocketMessage
+            websocketMessage.orElse( AddressRule.WEBSOCKET_MESSAGE )
         );
     }
 
-    void merge( @Nonnull PartialOptions other )
+    /**
+     * Perform a left-biased union of two {@link PartialOptions}.
+     *
+     * @param other The other partial options to combine with.
+     * @return The merged options map.
+     */
+    PartialOptions merge( PartialOptions other )
     {
-        if( action == null && other.action != null ) action = other.action;
+        // Short circuit for DEFAULT. This has no effect on the outcome, but avoids an allocation.
-        if( maxUpload == null && other.maxUpload != null ) maxUpload = other.maxUpload;
+        if( this == DEFAULT ) return other;
-        if( maxDownload == null && other.maxDownload != null ) maxDownload = other.maxDownload;
-        if( timeout == null && other.timeout != null ) timeout = other.timeout;
-        if( websocketMessage == null && other.websocketMessage != null ) websocketMessage = other.websocketMessage;
-    }
 
-    PartialOptions copy()
+        return new PartialOptions(
-    {
+            action == null && other.action != null ? other.action : action,
-        return new PartialOptions( action, maxUpload, maxDownload, timeout, websocketMessage );
+            maxUpload.isPresent() ? maxUpload : other.maxUpload,
+            maxDownload.isPresent() ? maxDownload : other.maxDownload,
+            timeout.isPresent() ? timeout : other.timeout,
+            websocketMessage.isPresent() ? websocketMessage : other.websocketMessage
+        );
     }
 }

src/test/java/dan200/computercraft/core/apis/http/options/AddressRuleTest.java

@@ -12,6 +12,7 @@
 
 import java.net.InetSocketAddress;
 import java.util.Collections;
+import java.util.OptionalInt;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -21,8 +22,8 @@ public class AddressRuleTest
     public void matchesPort()
     {
         Iterable<AddressRule> rules = Collections.singletonList( AddressRule.parse(
-            "127.0.0.1", 8080,
+            "127.0.0.1", OptionalInt.of( 8080 ),
-            new PartialOptions( Action.ALLOW, null, null, null, null )
+            Action.ALLOW.toPartial()
         ) );
 
         assertEquals( apply( rules, "localhost", 8080 ).action, Action.ALLOW );