Skip to content

Conversation

@opwvhk
Copy link
Contributor

@opwvhk opwvhk commented Mar 9, 2022

Automatically register LogicalTypeFactory classes using the Java 6
service loader upon startup. This works for any LogicalTypeFactory that
does not need constructor parameters.

Make sure you have checked all steps below.

Jira

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:
    org.apache.avro.TestLogicalType#testRegisterLogicalTypeFactoryByServiceLoader

Commits

  • My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain Javadoc that explain what it does

Automatically register LogicalTypeFactory classes using the Java 6
service loader upon startup. This works for any LogicalTypeFactory that
does not need constructor parameters.
@github-actions github-actions bot added the Java Pull Requests for Java binding label Mar 9, 2022
Comment on lines +60 to +64
static {
for (LogicalTypeFactory logicalTypeFactory : ServiceLoader.load(LogicalTypeFactory.class)) {
register(logicalTypeFactory);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual magic.

*/
package org.apache.avro;

public class DummyLogicalTypeFactory implements LogicalTypes.LogicalTypeFactory {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is part 1 of what makes it work: a public class with a public no-arg constructor (a default constructor in this case, but that's not important).

# See the License for the specific language governing permissions and
# limitations under the License.

org.apache.avro.DummyLogicalTypeFactory
Copy link
Contributor Author

@opwvhk opwvhk Mar 9, 2022

Choose a reason for hiding this comment

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

This is part 2 of what makes it work: a file in META-INF/services named after the binary name of the interface, containing names of classes that implement the interface (one per line); comments and empty lines are ignored.

Comment on lines +291 to +296
@Test
public void testRegisterLogicalTypeFactoryByServiceLoader() {
MatcherAssert.assertThat(LogicalTypes.getCustomRegisteredTypes(),
IsMapContaining.hasEntry(equalTo("service-example"), instanceOf(LogicalTypes.LogicalTypeFactory.class)));
}

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 actual test

Comment on lines +298 to +299
Assert.assertEquals("Should be equal (forward): " + message, o1, o2);
Assert.assertEquals("Should be equal (reverse): " + message, o2, o1);
Copy link
Contributor Author

@opwvhk opwvhk Mar 9, 2022

Choose a reason for hiding this comment

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

Boyscout rule (plus a few more)

@RyanSkraba RyanSkraba self-requested a review March 9, 2022 13:03
@RyanSkraba RyanSkraba merged commit ee4725c into apache:master Mar 18, 2022
RyanSkraba pushed a commit that referenced this pull request Mar 18, 2022
Automatically register LogicalTypeFactory classes using the Java 6
service loader upon startup. This works for any LogicalTypeFactory that
does not need constructor parameters.
@opwvhk opwvhk deleted the AVRO-3441-ServiceLoader-for-LogicalTypeFactory branch April 4, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java Pull Requests for Java binding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants