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

Loader API option 1 #131

Merged
merged 1 commit into from
Nov 10, 2022
Merged

Conversation

ljnelson
Copy link
Contributor

@ljnelson ljnelson commented Oct 20, 2022

Don't dive into the code without reading this description and understanding it, or without reading and understanding the javadocs.

This pull request is one of three mutually exclusive options. It sketches a "loader" API for loading configuration-related objects in some vague unspecified manner. This pull request is named Option 1. You may also be interested in Option 2 (#132) and Option 3 (#133).

Sample usage:

Loader loader = loader.bootstrap();
MyConfigurationRelatedObject object = null;
try {
    object = loader.load(MyConfigurationRelatedObject.class);
} catch (NoSuchObjectException e) {
    // object is absent
}

In as many cases as I could, I asked for and incorporated some group opinions while working this sketch up. See for example #75, #109, #110, #119, #122 (particularly my comment and Roberto's response), #124, and #127, among others.

Option 1 (this PR) models the "loader API" that people always come up with first. In my opinion it has flaws but I've tried to do it justice.

In Option 1 (this PR), the Loader accepts only a type object (see, for example, David's comment in #124). As new cases and scenarios arise, the load method will need to be overloaded to accept any new parameters that we happen to discover.

In Option 1 (this PR), the Loader returns the loaded object directly. Following the group consensus arrived at in #119 (see Dmitry's comment in particular), whether or not the return value is determinate is deliberately left unspecified.

I personally prefer Option 3 (#133) which, IMHO, allows for more graceful API evolution over time.

Finally, and perhaps most importantly, I still think it is far too early for PRs and I would prefer to write documents first, but to date that has not met with success. Therefore I hope this (and the other options) will at least get the discussion going.

Signed-off-by: Laird Nelson ljnelson@gmail.com

Signed-off-by: Laird Nelson <ljnelson@gmail.com>
This was referenced Oct 20, 2022
@ljnelson
Copy link
Contributor Author

ljnelson commented Oct 20, 2022

Javadocs for Option 1 (this PR): jakarta.config-api-1.0.0-SNAPSHOT-javadoc.zip

* @exception NullPointerException if the supplied {@code type}
* was {@code null}
*/
public <T> T load(Class<T> type);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of the concerns I have with the approach this PR represents is that as we discover more "things" to add in the "addressing" section (i.e. not just type) we have to overload this method. In this PR as written you can see we need a minimum of two methods just to deal with the Class/ParameterizedType split. Now consider adding some qualifier like name or locale, incrementally over time. That's a lot of methods.

* <p><strong>Note:</strong> The rules governing how it is
* determined whether any given configuration-related object is
* "of the supplied {@code type}" are currently wholly
* undefined.</p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, not wholly undefined; see for example one opinion here: #124 (reply in thread)

*
* <p>Implementations of this method may or may not return a <a
* href="doc-files/terminology.html#determinate">determinate</a>
* value.</p>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another concern I have with the approach this PR represents is that it is impossible to tell whether a response is determinate or not. We may decide one thing today, and another thing tomorrow, and with this API we have no immediately obvious place to "put" information about the response. For that we would need Option 3 (#133).

* @exception ConfigException if bootstrapping failed because of a
* {@link Loader#load(Class)} problem
*/
public static Loader bootstrap(ClassLoader classLoader) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The implementation of this method is just a sketch. I think you get the general idea of how the Loader interface could bootstrap an implementation that uses itself to configure itself.

*
* @see #type()
*/
public abstract class TypeToken<T> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See also:

I'm sure there are others. But recall (#116) we don't want to depend on these specifications.

exception</a>. The opposite of an <em>absent</em> value is
a <a href="#present"><em>present</em></a> value.</dd>

<dt><a name="determinate">Determinate</a></dt>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

<h1>Terminology</h1>
<dl>

<dt><a name="absent">Absent</a></dt>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

and <a href="#present"><em>present</em></a>)
configuration-related object.</dd>

<dt><a name="present">Present</a></dt>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<configuration>
<docfilessubdirs>true</docfilessubdirs>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See src/main/javadoc/.../doc-files/terminology.html

*
* @exception NoSuchObjectException if the invocation was sound
* but the requested object was <a
* href="doc-files/terminology.html#absent">absent</a>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

…which by itself says nothing about whether it might be absent or present in the future. See #119.


/**
* A {@link ConfigException} thrown when a configuration-related
* object was not found.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically, that it was absent, following the terminology used throughout this PR.

/**
* A holder of a modeled {@link Type} that embodies <a
* href="http://gafter.blogspot.com/2006/12/super-type-tokens.html"
* target="_parent">Gafter's gadget</a>.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gafter's gadget (super type tokens): http://gafter.blogspot.com/2006/12/super-type-tokens.html

*/


private static final ParameterizedType mostSpecializedParameterizedSuperclass(Type type) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

e.g. my TypeToken subclass actually extends YourTypeToken subclass which extends HerTypeToken subclass which extends TypeToken; we want the actual type arguments from the most specialized of these (mine)

The opposite of a <em>present</em> value is
an <a href="#absent"><em>absent</em></a> value.</dd>

<dt><a name="selection">Selection</a></dt>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a given load request is known
as <a href="#selection"><em>selection</em></a>. Any given
configuration-related object may be more or less suitable than
another for any given load request.</dd>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To date we have punted on this, blithely assuming incorrectly that there will be exactly one possible configuration-related object that could possibly "fit" a given load request. The concept is sound even if we haven't addressed it yet.

@ljnelson ljnelson marked this pull request as ready for review November 10, 2022 15:22
@ljnelson ljnelson merged commit b519d15 into jakartaee:main Nov 10, 2022


/*
* Static methods.

Choose a reason for hiding this comment

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

👀

/*
* Copyright (c) 2022 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional

Choose a reason for hiding this comment

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

👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants