-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
FlexibleConfig #2757
Merged
Merged
FlexibleConfig #2757
Changes from all commits
Commits
Show all changes
57 commits
Select commit
Hold shift + click to select a range
8fe159f
Add FlexibleConfig, Setting, SettingValue
eviltak 7165875
Add SettingValueValidator (with an impl) and tests
eviltak b4bc197
RangedNumberValueValidator can remove bounds
eviltak e1fb7c0
Make FlexibleConfig.get return generic Setting<T>
eviltak 831892b
Add tests for Setting
eviltak 772413a
Fix FlexibleConfig.get, Add FlexibleConfig tests
eviltak d3001c2
Make FlexibleConfig.Key and Setting use SimpleUri
eviltak 5805d70
Rename SettingValueValidator.isValid to validate
eviltak fbb5594
Remove Setting setters except setValue
eviltak 308e914
Use SimpleUri as the key type for FlexibleConfig
eviltak 1c157ee
Log warnings when Setting can't be removed
eviltak 1588ce7
Add ctor to Setting, rename to validator
eviltak 3b6db40
Lazy create and delete Setting subscriber list
eviltak 5493584
Fix Setting.hasSubscribers
eviltak 05b3acd
Rename Setting.name to humanReadableName
eviltak 9d7bdef
Rename RangedNumberValueValidator to RangedNumberValidator
eviltak 9664a7f
Make RangedNumberValidator immutable
eviltak 08eef0f
Rename RangedNumberValidator.low, high to min, max
eviltak 60ee10f
Add separate minInclusive and maxInclusive
eviltak df0325b
Split Setting into interface and implementation
eviltak d599fea
Split FlexibleConfig into interface and implementation
eviltak 3d580d6
Rename FlexibleConfig.has to contains
eviltak d1eca9d
Test RangedNumberValidator min and maxInclusive
eviltak 1003fd1
Rename FlexibleConfigTest subclasses
eviltak a5b547e
Log warnings in FlexibleConfig.add
eviltak dde4600
Log warning when invalid value is passed to Setting
eviltak 3796563
Add issueWarnings and validateWithWarnings
eviltak b1d6496
Document RangedValueValidator
eviltak 7d21437
Document Setting
eviltak 16770ee
Document SettingImpl
eviltak e0a0eae
Document FlexibleConfig, Make add non-generic
eviltak 41fe079
Document FlexibleConfigImpl
eviltak 66115e1
Update SettingImpl documentation
eviltak 90f871a
Make SettingImpl.subscribers a Set
eviltak 3b6946c
Fix Setting subscriber tests
eviltak a44e2c8
Update Setting documentation
eviltak f8055f9
Add resetValue method to Setting
eviltak 76f069e
Refactor SettingValueValidator methods
eviltak aa2db9a
Check if validator is null in SettingImpl.setValue
eviltak ec49a6a
Make SettingValueValidator an abstract class
eviltak 3a29fb1
Update SettingValueValidator Javadoc
eviltak e1537ef
Make RangedNumberValidator.issueWarnings protected
eviltak 45207e6
Update FlexibleConfig Javadoc
eviltak f63cfa2
Update FlexibleConfig tests
eviltak 6f003fa
Log warnings in constructor if SettingImpl default value is invalid
eviltak a004aa1
Make Setting.subscribe and unsubscribe return operation status
eviltak 0ed410d
Make SettingImpl.id a final field
eviltak 701316f
Add warningFormatString to SettingImpl
eviltak 328e754
Update Setting documentation
eviltak c92f14f
Refactor Setting tests to improve readability
eviltak 86b2237
Convert SettingValueValidator to interface
eviltak 5038857
Merge branch 'develop' into flexible-config
eviltak 57cf7ab
FlexibleConfigTest formatting improvements
eviltak f9b1959
Update RangedNumberValidatorTest
eviltak 02132ca
Add newline at end of RangedValueValidatorTest.java
eviltak 165e97d
Rename EPSILON to MAX_ALLOWED_ERROR
eviltak 3090f48
# Executed last requested changes.
emanuele3d File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
145 changes: 145 additions & 0 deletions
145
engine-tests/src/test/java/org/terasology/config/flexible/FlexibleConfigTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,145 @@ | ||
/* | ||
* Copyright 2016 MovingBlocks | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.terasology.config.flexible; | ||
|
||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.junit.experimental.runners.Enclosed; | ||
import org.junit.runner.RunWith; | ||
import org.terasology.engine.SimpleUri; | ||
|
||
import static org.junit.Assert.*; | ||
|
||
@RunWith(Enclosed.class) | ||
public class FlexibleConfigTest { | ||
private static final SimpleUri KEY_NON_EXISTENT = new SimpleUri("engine-tests:TestSettingX"); | ||
|
||
public static class Get { | ||
private FlexibleConfig config; | ||
|
||
@Before | ||
public void setUp() throws Exception { | ||
config = new FlexibleConfigImpl(); | ||
} | ||
|
||
@Test | ||
public void testGet() throws Exception { | ||
SimpleUri id1 = new SimpleUri("engine-tests:TestSetting1"); | ||
SimpleUri id2 = new SimpleUri("engine-tests:TestSetting2"); | ||
|
||
Setting<Integer> expectedSetting1 = new MockSetting<>(id1); | ||
config.add(expectedSetting1); | ||
|
||
Setting<Double> expectedSetting2 = new MockSetting<>(id2); | ||
config.add(expectedSetting2); | ||
|
||
Setting<Integer> retrievedSetting1 = config.get(id1); | ||
Setting<Double> retrievedSetting2 = config.get(id2); | ||
|
||
// We need the references to be equal | ||
assertEquals(expectedSetting1, retrievedSetting1); | ||
assertEquals(expectedSetting2, retrievedSetting2); | ||
} | ||
} | ||
|
||
public static class Contains { | ||
private FlexibleConfig config; | ||
|
||
@Before | ||
public void setUp() throws Exception { | ||
config = new FlexibleConfigImpl(); | ||
} | ||
|
||
@Test | ||
public void testContains() throws Exception { | ||
SimpleUri id = new SimpleUri("engine-tests:TestSetting"); | ||
config.add(new MockSetting<Integer>(id)); | ||
|
||
assertTrue(config.contains(id)); | ||
} | ||
|
||
@Test | ||
public void testNotContains() throws Exception { | ||
SimpleUri id = new SimpleUri("engine-tests:TestSetting"); | ||
config.add(new MockSetting<Integer>(id)); | ||
|
||
assertFalse(config.contains(KEY_NON_EXISTENT)); | ||
} | ||
} | ||
|
||
public static class Add { | ||
private FlexibleConfig config; | ||
|
||
@Before | ||
public void setUp() throws Exception { | ||
config = new FlexibleConfigImpl(); | ||
} | ||
|
||
@Test | ||
public void testAdd() throws Exception { | ||
SimpleUri id = new SimpleUri("engine-tests:TestSetting"); | ||
|
||
assertTrue(config.add(new MockSetting(id))); | ||
} | ||
|
||
@Test | ||
public void testAddExisting() throws Exception { | ||
SimpleUri id = new SimpleUri("engine-tests:TestSetting"); | ||
|
||
assertTrue(config.add(new MockSetting<Integer>(id))); | ||
assertFalse(config.add(new MockSetting<Double>(id))); | ||
} | ||
} | ||
|
||
public static class Remove { | ||
private FlexibleConfig config; | ||
|
||
@Before | ||
public void setUp() throws Exception { | ||
config = new FlexibleConfigImpl(); | ||
} | ||
|
||
@Test | ||
public void testRemove() throws Exception { | ||
SimpleUri id1 = new SimpleUri("engine-tests:TestSetting1"); | ||
SimpleUri id2 = new SimpleUri("engine-tests:TestSetting2"); | ||
|
||
config.add(new MockSetting(id1)); | ||
config.add(new MockSetting(id2)); | ||
|
||
assertTrue(config.remove(id1)); | ||
assertTrue(config.remove(id2)); | ||
} | ||
|
||
@Test | ||
public void testNonexistentRemove() throws Exception { | ||
SimpleUri id = new SimpleUri("engine-tests:TestSetting"); | ||
config.add(new MockSetting(id)); | ||
|
||
assertFalse(config.remove(KEY_NON_EXISTENT)); | ||
} | ||
|
||
@Test | ||
public void testSubscribedRemove() throws Exception { | ||
SimpleUri id = new SimpleUri("engine-tests:TestSetting"); | ||
Setting setting = new MockSetting(id); | ||
config.add(setting); | ||
setting.subscribe(propertyChangeEvent -> {}); | ||
|
||
assertFalse(config.remove(id)); | ||
} | ||
} | ||
} |
82 changes: 82 additions & 0 deletions
82
engine-tests/src/test/java/org/terasology/config/flexible/MockSetting.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
/* | ||
* Copyright 2016 MovingBlocks | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.terasology.config.flexible; | ||
|
||
import org.terasology.config.flexible.validators.SettingValueValidator; | ||
import org.terasology.engine.SimpleUri; | ||
|
||
import java.beans.PropertyChangeListener; | ||
|
||
class MockSetting<T> implements Setting<T> { | ||
private final SimpleUri id; | ||
private boolean isSubscribedTo = false; | ||
|
||
MockSetting(SimpleUri id) { | ||
this.id = id; | ||
} | ||
|
||
@Override | ||
public boolean subscribe(PropertyChangeListener changeListener) { | ||
isSubscribedTo = true; | ||
return true; | ||
} | ||
|
||
@Override | ||
public boolean unsubscribe(PropertyChangeListener changeListener) { | ||
isSubscribedTo = false; | ||
return false; | ||
} | ||
|
||
@Override | ||
public SimpleUri getId() { | ||
return id; | ||
} | ||
|
||
@Override | ||
public SettingValueValidator<T> getValidator() { | ||
return null; | ||
} | ||
|
||
@Override | ||
public T getDefaultValue() { | ||
return null; | ||
} | ||
|
||
@Override | ||
public T getValue() { | ||
return null; | ||
} | ||
|
||
@Override | ||
public boolean setValue(T newValue) { | ||
return false; | ||
} | ||
|
||
@Override | ||
public String getHumanReadableName() { | ||
return null; | ||
} | ||
|
||
@Override | ||
public String getDescription() { | ||
return null; | ||
} | ||
|
||
@Override | ||
public boolean hasSubscribers() { | ||
return isSubscribedTo; | ||
} | ||
} |
144 changes: 144 additions & 0 deletions
144
engine-tests/src/test/java/org/terasology/config/flexible/SettingTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
/* | ||
* Copyright 2016 MovingBlocks | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.terasology.config.flexible; | ||
|
||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.junit.experimental.runners.Enclosed; | ||
import org.junit.runner.RunWith; | ||
import org.terasology.config.flexible.validators.RangedNumberValidator; | ||
import org.terasology.engine.SimpleUri; | ||
import org.terasology.utilities.random.FastRandom; | ||
import org.terasology.utilities.random.Random; | ||
|
||
import java.beans.PropertyChangeListener; | ||
|
||
import static org.junit.Assert.*; | ||
|
||
@RunWith(Enclosed.class) | ||
public class SettingTest { | ||
private static final SimpleUri SETTING_ID = new SimpleUri("engine-tests:TestSetting"); | ||
|
||
public static class SetValue { | ||
private Setting<Integer> setting; | ||
|
||
private int eventResult; | ||
|
||
@Before | ||
public void setUp() { | ||
setting = new SettingImpl<>(SETTING_ID, | ||
50, new RangedNumberValidator<>(0, 100, false, false)); | ||
|
||
eventResult = -1; | ||
|
||
setting.subscribe(propertyChangeEvent -> eventResult = (int) propertyChangeEvent.getNewValue()); | ||
} | ||
|
||
@Test | ||
public void testSetsValue() { | ||
assertTrue(setting.setValue(25)); | ||
|
||
assertEquals(25, eventResult); | ||
} | ||
|
||
@Test | ||
public void testDoesNotSetValue() { | ||
assertFalse(setting.setValue(101)); | ||
|
||
assertEquals(-1, eventResult); | ||
} | ||
} | ||
|
||
public static class Subscribers { | ||
private Setting<Integer> setting; | ||
|
||
private PropertyChangeListener listener; | ||
|
||
private int eventCallCount; | ||
|
||
@Before | ||
public void setUp() { | ||
setting = new SettingImpl<>(SETTING_ID, | ||
50, new RangedNumberValidator<>(0, 100, false, false)); | ||
|
||
eventCallCount = 0; | ||
|
||
listener = propertyChangeEvent -> eventCallCount++; | ||
} | ||
|
||
@Test | ||
public void testHasSubscribers() { | ||
setting.subscribe(listener); | ||
|
||
assertTrue(setting.hasSubscribers()); | ||
|
||
setting.unsubscribe(listener); | ||
|
||
assertFalse(setting.hasSubscribers()); | ||
} | ||
|
||
@Test | ||
public void testSetEventCall() { | ||
setting.subscribe(listener); | ||
|
||
Random random = new FastRandom(); | ||
|
||
final int MAX_SET_VALUE_COUNT = 50; | ||
int expectedEventCallCount = 0; | ||
|
||
for (int i = 0; i < MAX_SET_VALUE_COUNT; i++) { | ||
int randomInt = random.nextInt(-50, 150); | ||
expectedEventCallCount += setting.setValue(randomInt) ? 1 : 0; | ||
} | ||
|
||
assertEquals(expectedEventCallCount, eventCallCount); | ||
} | ||
|
||
@Test | ||
public void testSubscribe() { | ||
final int SUBSCRIBER_COUNT = 10; | ||
|
||
for (int i = 0; i < SUBSCRIBER_COUNT; i++) { | ||
setting.subscribe(propertyChangeEvent -> eventCallCount++); | ||
} | ||
|
||
setting.setValue(30); | ||
|
||
assertEquals(SUBSCRIBER_COUNT, eventCallCount); | ||
} | ||
|
||
@Test | ||
public void testUnsubscribe() { | ||
int subscriberCount = 10; | ||
|
||
PropertyChangeListener[] listeners = new PropertyChangeListener[subscriberCount]; | ||
|
||
for (int i = 0; i < subscriberCount; i++) { | ||
listeners[i] = propertyChangeEvent -> eventCallCount++; | ||
setting.subscribe(listeners[i]); | ||
} | ||
|
||
for (int i = 0; i < new FastRandom().nextInt(subscriberCount / 2); i++) { | ||
setting.unsubscribe(listeners[i]); | ||
subscriberCount--; | ||
} | ||
|
||
setting.setValue(30); | ||
|
||
assertEquals(subscriberCount, eventCallCount); | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe this to me in plain english please? I don't see any flaw in it but I'm not agile with this newish syntax yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lambdas are equivalent to new anonymous object (with a single method) creation statements, but are much cleaner.
propertyChangeEvent -> eventResult = (int) propertyChangeEvent.getNewValue()
evaluates toThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood, thank you.