Fix !1088 (API config.getApiValue() not returning null)

2026-02-04 07:19:50 -06:00
parent d0e14ac408
commit af0e5699e2
6 changed files with 109 additions and 12 deletions
@@ -62,7 +62,7 @@ public class ConfigHandler
 	 * <br> {@link String}
 	 * <br> 
 	 * <br> // Below, "T" should be a value from above
-	 * <br> // Note: This is not checked, so we trust that you are doing the right thing (TODO: Check it)
+	 * <br> // Note: This is not checked, so we trust that you are doing the right thing
 	 * <br> List<T>
 	 * <br> ArrayList<T>
 	 * <br> Map<String, T>
@@ -261,7 +261,6 @@ public class ConfigHandler
 			if (ConfigUIComment.class.isAssignableFrom(entry.getClass())
 				&& ((ConfigUIComment)entry).parentConfigPath != null)
 			{
-				// TODO this could potentially add the same item multiple times
 				entryPrefix = "distanthorizons.config." + ((ConfigUIComment)entry).parentConfigPath;
 			}
 			
@@ -46,6 +46,12 @@ public class DhApiConfigValue<coreType, apiType> implements IDhApiConfigValue<ap
 	private final IConverter<coreType, apiType> configConverter;
 	
 	
+	
+	//==============//
+ 	// constructors //
+	//==============//
+	//region
+	
 	/**
 	 * This constructor should only be called internally. <br>
 	 * There is no reason for API users to create this object. <br><br>
@@ -69,11 +75,29 @@ public class DhApiConfigValue<coreType, apiType> implements IDhApiConfigValue<ap
 		this.configConverter = newConverter;
 	}
 	
+	//endregion
 	
-	public apiType getValue() { return this.configConverter.convertToApiType(this.configBase.get()); }
-	public apiType getTrueValue() { return this.configConverter.convertToApiType(this.configBase.getTrueValue()); }
-	public apiType getApiValue() { return this.configConverter.convertToApiType(this.configBase.getApiValue()); }
 	
+	
+	//===========//
+ 	// overrides //
+	//===========//
+	//region
+	
+	@Override public apiType getValue() { return this.configConverter.convertToApiType(this.configBase.get()); }
+	@Override public apiType getTrueValue() { return this.configConverter.convertToApiType(this.configBase.getTrueValue()); }
+	@Override public apiType getApiValue() 
+	{
+		// if no API value is set, this should return null
+		if (this.configBase.getApiValue() == null)
+		{
+			return null;
+		}
+		
+		return this.configConverter.convertToApiType(this.configBase.getApiValue()); 
+	}
+	
+	@Override
 	public boolean setValue(apiType newValue)
 	{
 		if (this.configBase.getAllowApiOverride())
@@ -87,12 +111,12 @@ public class DhApiConfigValue<coreType, apiType> implements IDhApiConfigValue<ap
 		}
 	}
 	
+	@Override
 	public boolean clearValue()
 	{
 		if (this.configBase.getAllowApiOverride())
 		{
 			// no converter should be used here since null objects may need to be handled differently
-			// TODO the API should just have a bool to keep track of whether the API value is in use instead of using NULL
 			this.configBase.setApiValue(null);
 			return true;
 		}
@@ -102,13 +126,15 @@ public class DhApiConfigValue<coreType, apiType> implements IDhApiConfigValue<ap
 		}
 	}
 	
+	@Override
 	public boolean getCanBeOverrodeByApi() { return this.configBase.getAllowApiOverride(); }
 	
-	public apiType getDefaultValue() { return this.configConverter.convertToApiType(this.configBase.getDefaultValue()); }
-	public apiType getMaxValue() { return this.configConverter.convertToApiType(this.configBase.getMax()); }
-	public apiType getMinValue() { return this.configConverter.convertToApiType(this.configBase.getMin()); }
+	@Override public apiType getDefaultValue() { return this.configConverter.convertToApiType(this.configBase.getDefaultValue()); }
+	@Override public apiType getMaxValue() { return this.configConverter.convertToApiType(this.configBase.getMax()); }
+	@Override public apiType getMinValue() { return this.configConverter.convertToApiType(this.configBase.getMin()); }
 	
 	
+	@Override
 	public void addChangeListener(Consumer<apiType> onValueChangeFunc) 
 	{
 		this.configBase.addValueChangeListener((coreValue) -> 
@@ -118,4 +144,6 @@ public class DhApiConfigValue<coreType, apiType> implements IDhApiConfigValue<ap
 		}); 
 	}
 	
+	//endregion
+	
 }
@@ -360,7 +360,8 @@ public class ConfigFileHandler
 		{
 			LOGGER.error("File creation failed at ["+this.configPath+"], error: ["+e.getMessage()+"].", e);
 			
-			// TODO is there a reason this is lazily gotten?
+			// delayed MC getter since this object may be created before
+			// the singleton has been bound
 			IMinecraftClientWrapper mc = SingletonInjector.INSTANCE.get(IMinecraftClientWrapper.class);
 			mc.crashMinecraft("Loading file and resetting config file failed at path [" + this.configPath + "]. Please check the file is ok and you have the permissions", e);
 		}
@@ -43,7 +43,7 @@ public class ConfigEntry<T> extends AbstractConfigBase<T>
 	private final String comment;
 	private T min;
 	private T max;
-	private final ArrayList<IConfigListener> listenerList; // TODO make concurrent
+	private final ArrayList<IConfigListener> listenerList;
 	private final String chatCommandName;
 	
 	/**
@@ -18,7 +18,6 @@ public class DbConnectionClosedException extends SQLException
 	
 	public static boolean isClosedException(SQLException e) 
 	{
-		// TODO long term we should prevent using repos that are closed, but for now this is the easier solution
 		String message = e.getMessage().toLowerCase();
 		return message.contains("connection closed")
 				|| message.contains("pointer is closed")
@@ -0,0 +1,70 @@
+/*
+ *    This file is part of the Distant Horizons mod
+ *    licensed under the GNU LGPL v3 License.
+ *
+ *    Copyright (C) 2020 James Seibel
+ *
+ *    This program is free software: you can redistribute it and/or modify
+ *    it under the terms of the GNU Lesser General Public License as published by
+ *    the Free Software Foundation, version 3.
+ *
+ *    This program is distributed in the hope that it will be useful,
+ *    but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *    GNU Lesser General Public License for more details.
+ *
+ *    You should have received a copy of the GNU Lesser General Public License
+ *    along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package tests;
+
+import com.seibel.distanthorizons.api.enums.rendering.EDhApiRendererMode;
+import com.seibel.distanthorizons.core.config.api.DhApiConfigValue;
+import com.seibel.distanthorizons.core.config.api.converters.RenderModeEnabledConverter;
+import com.seibel.distanthorizons.core.config.types.ConfigEntry;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Quick test to confirm API config handling works correctly.
+ *
+ * @author James Seibel
+ * @version 2026-02-04
+ */
+public class DhApiConfigTest
+{
+	
+	@Test
+	public void ConfigTest()
+	{
+		ConfigEntry<EDhApiRendererMode> coreConfig = new ConfigEntry.Builder<EDhApiRendererMode>()
+			.set(EDhApiRendererMode.DEBUG)
+			.build();
+		
+		DhApiConfigValue<EDhApiRendererMode, Boolean> apiConfig = new DhApiConfigValue<>(coreConfig, new RenderModeEnabledConverter());
+		
+		// start with no API value
+		Assert.assertNull("API Value shouldn't be set yet", apiConfig.getApiValue());
+		Assert.assertEquals("underlying config should be 'DEBUG'", EDhApiRendererMode.DEBUG, coreConfig.get());
+		
+		
+		// set API value
+		apiConfig.setValue(true);
+		Assert.assertTrue("API Value should be 'true'", apiConfig.getApiValue());
+		Assert.assertEquals("underlying config should be 'DEFAULT'", EDhApiRendererMode.DEFAULT, coreConfig.get());  
+		
+		// set API value again
+		apiConfig.setValue(false);
+		Assert.assertFalse("API Value should be 'false'", apiConfig.getApiValue());
+		Assert.assertEquals("underlying config should be disabled", EDhApiRendererMode.DISABLED, coreConfig.get());
+		
+		// clear API value
+		apiConfig.clearValue();
+		Assert.assertNull("API Value should be null", apiConfig.getApiValue());
+		Assert.assertEquals("underlying config should be 'DEBUG'", EDhApiRendererMode.DEBUG, coreConfig.get());
+		
+		
+	}
+	
+}