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

8301302: Platform preferences API #1014

Closed
wants to merge 62 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
2d28c85
Platform preferences implementation
mstr2 Jul 4, 2023
df03228
documentation
mstr2 Jul 18, 2023
ba26271
Removed platform-independent preference keys
mstr2 Jul 23, 2023
00e9eb7
doc changes
mstr2 Jul 23, 2023
dbad68d
Move Appearance enum to javafx.application
mstr2 Jul 23, 2023
e16491a
removed unused code
mstr2 Jul 24, 2023
2d1c7a9
Ensure that ColorProperty changes are atomic when observed
mstr2 Jul 24, 2023
0589078
doc changes
mstr2 Jul 24, 2023
f8a9c8a
Added test
mstr2 Jul 24, 2023
40e2f28
Removed application preferences implementation
mstr2 Sep 5, 2023
b100c6c
Addressed review comments
mstr2 Sep 5, 2023
4b9f75d
Revert Application changes
mstr2 Sep 5, 2023
27022f3
Added javadoc link to list of platform preferences
mstr2 Sep 5, 2023
02d6a2f
improve javadoc
mstr2 Sep 6, 2023
4172fa6
Update Eclipse .classpath file
mstr2 Sep 6, 2023
5081fda
Suppress empty change message in test application
mstr2 Sep 6, 2023
08a3692
Format arrays in test application
mstr2 Sep 6, 2023
2837e33
Merge branch 'master' into feature/platform-preferences
mstr2 Sep 6, 2023
9cfded0
Fire only a single invalidation event
mstr2 Sep 6, 2023
c8fd0e7
changes per review
mstr2 Sep 6, 2023
1eaea48
change test class name
mstr2 Sep 6, 2023
c736dee
Add signal handler for gtk-theme-name
mstr2 Sep 6, 2023
aae55f9
Remove javadocs
mstr2 Sep 6, 2023
cb81762
Handle key removals
mstr2 Sep 8, 2023
a893fa4
Merge branch 'master' into feature/platform-preferences
mstr2 Oct 29, 2023
8a5b213
changed parameter name
mstr2 Oct 29, 2023
7e826df
Javadoc change
mstr2 Oct 31, 2023
684f48b
formatting
mstr2 Oct 31, 2023
167d4e8
review changes
mstr2 Nov 1, 2023
ad74e39
changend bool comparison
mstr2 Nov 1, 2023
38177a8
review changes
mstr2 Nov 2, 2023
d012dec
removed extra newline
mstr2 Nov 3, 2023
9b607c6
Rename Appearance to ColorScheme
mstr2 Nov 10, 2023
6ec7e99
Doc changes
mstr2 Nov 17, 2023
3cc29e9
Add eager type checking for typed getters
mstr2 Nov 18, 2023
9a817d1
Remove Preferences.getPaint
mstr2 Nov 18, 2023
bb6071c
Replace HashMap with Map.ofEntries
mstr2 Nov 18, 2023
a3d0010
Support polymorphic values
mstr2 Nov 20, 2023
17b2b08
Use JLS 5.5.1 casting conversion rules
mstr2 Nov 24, 2023
9083d3e
flip S and T
mstr2 Nov 24, 2023
9adef99
address review comments
mstr2 Nov 24, 2023
57c8d0c
rename local variables
mstr2 Nov 24, 2023
bee1bae
typo
mstr2 Nov 24, 2023
89f1c8f
initialize field with NULL
mstr2 Nov 29, 2023
9cc3b4b
initialize field with NULL
mstr2 Nov 30, 2023
dbf24bf
check for pending exceptions in macOS PlatformSupport
mstr2 Dec 1, 2023
0572a36
check for pending exceptions in GTK PlatformSupport
mstr2 Dec 2, 2023
f291ea4
check for pending exceptions in Windows PlatformSupport
mstr2 Dec 2, 2023
a04a8c6
check for null return values of JNI methods
mstr2 Dec 4, 2023
0dff7a4
check return value of JNI functions
mstr2 Dec 4, 2023
6c37544
rename GLASS_CHECK_EXCEPTIONALLY_RETURN
mstr2 Dec 6, 2023
db393fc
null checking
mstr2 Dec 6, 2023
0759ddd
changed error condition check
mstr2 Dec 6, 2023
a2bd32d
consistently use NULL
mstr2 Dec 6, 2023
070eab5
swap message fields
mstr2 Dec 6, 2023
515395b
fixed bug in test application
mstr2 Dec 6, 2023
098ca17
fixed HighContrastScheme
mstr2 Dec 7, 2023
eac1d82
format arrays in test application, indicate added/removed prefs
mstr2 Dec 7, 2023
efdab27
renamed Windows.SPI.HighContrastOn to Windows.SPI.HighContrast
mstr2 Dec 7, 2023
20cccc5
query resource bundles for high-contrast schemes only on Windows
mstr2 Dec 7, 2023
6457503
javadoc
mstr2 Dec 7, 2023
3354782
removed unused import
mstr2 Dec 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Support polymorphic values
mstr2 committed Nov 20, 2023
commit a3d001039b5d0c2ec59cd6a2bc58e98d4c631cf5
Original file line number Diff line number Diff line change
@@ -789,6 +789,8 @@ public Map<String, String> getPlatformKeyMappings() {

/**
* Returns a mapping of platform-specific keys to the types of their values.
* Polymorphic types are supported by specifying the common base type; for example, a key can
* be mapped to {@code Paint.class} to support any type of paint.
* <p>
* Implementors must keep this map in sync with the mappings reported by the native Glass toolkit.
* If a native toolkit reports mappings for keys that are not contained in this map, the typed getters
Original file line number Diff line number Diff line change
@@ -147,7 +147,7 @@ public <T> Optional<T> getValue(String key, Class<T> type) {
return Optional.empty();
}

if (!type.isAssignableFrom(platformType)) {
if (!platformType.isAssignableFrom(type)) {
throw new IllegalArgumentException(
"Incompatible types: requested = " + type.getName() +
", actual = " + platformType.getName());
@@ -157,9 +157,15 @@ public <T> Optional<T> getValue(String key, Class<T> type) {
return Optional.empty();
}

@SuppressWarnings("unchecked")
T v = (T)value;
return Optional.of(v);
if (type.isInstance(value)) {
@SuppressWarnings("unchecked")
T v = (T)value;
return Optional.of(v);
}

throw new IllegalArgumentException(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this behavior be documented?
there is no mention of an exception in case of type mismatch in the base class.

also, do we want to have some kind of trivial conversion implemented such as int -> long -> double ?
or not?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is documented in the Platform.Preferences interface, which is implemented by this class:

        /**
         * Returns the value to which the specified key is mapped.
         *
         * @param <T> the type of the value
         * @param key the key
         * @param type the type of the value
         * @throws NullPointerException if {@code key} is null
         * @throws IllegalArgumentException if the key is not mapped to a {@code type} instance
         * @return the value to which the key is mapped, or {@code Optional.empty()}
         *         if no mapping exists for the specified key
         */
        <T> Optional<T> getValue(String key, Class<T> type);

I'm a bit hesitant regarding an automatic conversion. What would be the use case for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one [possible] use case is when different versions of the native platform have different types (int32/int64 ?).
or, if the property value is String with the semantics of an integer.

Of course, the app dev can always request an Object and parse that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add this, we'd also need to decide whether we only want to support lossless conversions, or if lossy conversions are acceptable as well. Since this will add a considerable amount of complexity to this PR, I suggest we don't to it at this time. If there's a need, we can always add it later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after some thought, I would say conversion is not needed - should that unlikely scenario happen, the app devs could use getValue(.., Object.class);

"Incompatible types: requested = " + type.getName() +
", actual = " + value.getClass().getName());
}

@Override
Original file line number Diff line number Diff line change
@@ -26,6 +26,8 @@
package test.com.sun.javafx.application.preferences;

import com.sun.javafx.application.preferences.PlatformPreferences;
import javafx.scene.paint.CycleMethod;
import javafx.scene.paint.LinearGradient;
import javafx.scene.paint.Paint;
import test.javafx.collections.MockMapObserver;
import javafx.beans.InvalidationListener;
@@ -85,6 +87,21 @@ void testWellKnownKeyReturnsEmptyValueWhenMappingNotPresent() {
assertEquals(Optional.empty(), prefs.getColor("test.aColor"));
assertEquals(Optional.empty(), prefs.getValue("test.aColor", Color.class));
assertEquals(Optional.empty(), prefs.getValue("test.aPaint", Paint.class));
assertEquals(Optional.empty(), prefs.getValue("test.aPaint", Color.class));
}

@Test
void testPolymorphicValues() {
prefs.update(Map.of("test.aPaint", Color.RED));
assertEquals(Color.RED, prefs.getColor("test.aPaint").orElseThrow());
assertEquals(Color.RED, prefs.getValue("test.aPaint", Paint.class).orElseThrow());
assertThrows(IllegalArgumentException.class, () -> prefs.getValue("test.aPaint", LinearGradient.class));

var gradient = new LinearGradient(0, 0, 1, 1, true, CycleMethod.NO_CYCLE);
prefs.update(Map.of("test.aPaint", gradient));
assertEquals(gradient, prefs.getValue("test.aPaint", Paint.class).orElseThrow());
assertEquals(gradient, prefs.getValue("test.aPaint", LinearGradient.class).orElseThrow());
assertThrows(IllegalArgumentException.class, () -> prefs.getColor("test.aPaint").orElseThrow());
}

@Test