Skip to content

Conversation

@mxsm
Copy link
Member

@mxsm mxsm commented Jun 17, 2023

Fixes #4129

Motivation

Explain the content here.
Explain why you want to make the changes and what problem you're trying to solve.

Modifications

  • Enhance the functionality of EventMeshExtensionFactory

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@mxsm mxsm force-pushed the eventmesh-4129 branch from 64f2b6d to f016dc6 Compare June 18, 2023 01:50
@mxsm mxsm force-pushed the eventmesh-4129 branch from f016dc6 to c7190ac Compare June 25, 2023 13:33
@codecov
Copy link

codecov bot commented Jun 26, 2023

Codecov Report

Merging #4131 (5073553) into master (991cbda) will increase coverage by 0.02%.
The diff coverage is 62.50%.

❗ Current head 5073553 differs from pull request most recent head c7190ac. Consider uploading reports for the commit c7190ac to get more accurate results

@@             Coverage Diff              @@
##             master    #4131      +/-   ##
============================================
+ Coverage     16.93%   16.95%   +0.02%     
- Complexity     1410     1411       +1     
============================================
  Files           588      588              
  Lines         25739    25751      +12     
  Branches       2387     2390       +3     
============================================
+ Hits           4358     4366       +8     
- Misses        20946    20948       +2     
- Partials        435      437       +2     
Impacted Files Coverage Δ
...pache/eventmesh/spi/EventMeshExtensionFactory.java 56.66% <62.50%> (-1.67%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xwm1992
Copy link
Contributor

xwm1992 commented Jun 27, 2023

What is the intent and purpose of this pr? @mxsm

@mxsm
Copy link
Member Author

mxsm commented Jun 27, 2023

What is the intent and purpose of this pr? @mxsm

@xwm1992
I have the following two interfaces in the same module

@EventMeshSPI(eventMeshExtensionType = EventMeshExtensionType.JDBC_DATABASE_DIALECT)
public interface DatabaseDialectFactory {

    /**
     * Creates a database dialect based on the provided source configuration.
     *
     * @param config the source configuration to create a database dialect for
     * @return the created database dialect
     */
    DatabaseDialect createDatabaseDialect(SourceConfig config);

}

@EventMeshSPI(eventMeshExtensionType = EventMeshExtensionType.JDBC_CDC_ENGINE)
public interface CdcEngineFactory {

    /**
     * Determines whether the provided JDBC URL is compatible with the CDC engine
     *
     * @param url jdbc url, e.g. mysql: jdbc:mysql://localhost:3306/
     * @return
     */
    boolean acceptJdbcProtocol(String url);

    /**
     * Creates a new CDC engine
     *
     * @return cdc engine
     */
    CdcEngine createCdcEngine(SourceConfig config, DatabaseDialect databaseDialect);

}

image

mysql=org.apache.eventmesh.source.connector.jdbc.dialect.cdc.mysql.MysqlCdcEngineFactory

mysql=org.apache.eventmesh.source.connector.jdbc.dialect.mysql.MysqlDatabaseDialectFactory

When I use utility class loading

@UtilityClass
public class JdbcAllFactoryLoader {

    /**
     * Returns a CdcEngineFactory for the given database name.
     * <p>Throws NullPointerException if databaseName is null.</p>
     * <p>Throws IllegalArgumentException if CdcEngineFactory is not supported for the given database name.</p>
     *
     * @param databaseName Name of the database for which CdcEngineFactory is required.
     * @return CdcEngineFactory for the given database name.
     */
    public static CdcEngineFactory getCdcEngineFactory(String databaseName) {
        checkNotNull(databaseName, "database name can not be null");
        CdcEngineFactory engineFactory = EventMeshExtensionFactory.getExtension(CdcEngineFactory.class, databaseName);
        return checkNotNull(engineFactory, "CdcEngineFactory: " + databaseName + " is not supported");
    }

    /**
     * Returns a DatabaseDialectFactory based on the specified database name.
     *
     * @param databaseName the name of the database
     * @return the DatabaseDialectFactory for the specified database name
     * @throws NullPointerException     if the database name is null
     * @throws IllegalArgumentException if the specified database name is not supported
     */
    public static DatabaseDialectFactory getDatabaseDialectFactory(String databaseName) {
        Objects.requireNonNull(databaseName, "database name can not be null");
        DatabaseDialectFactory databaseDialectFactory = EventMeshExtensionFactory.getExtension(DatabaseDialectFactory.class, databaseName);
        Objects.requireNonNull(databaseDialectFactory, "DatabaseDialectFactory: " + databaseName + " is not supported");
        return databaseDialectFactory;
    }
}

Because the key in the file is mysql. Will throw

2023-06-17 21:53:26,101 INFO  [main] MetaInfExtensionClassLoader(MetaInfExtensionClassLoader.java:86) - load extension class success, extensionType: interface org.apache.eventmesh.source.connector.jdbc.dialect.DatabaseDialectFactory, extensionClass: class org.apache.eventmesh.source.connector.jdbc.dialect.mysql.MysqlDatabaseDialectFactory
2023-06-17 21:53:26,106 INFO  [main] EventMeshExtensionFactory(EventMeshExtensionFactory.java:92) - initialize extension instance success, extensionType: interface org.apache.eventmesh.source.connector.jdbc.dialect.DatabaseDialectFactory, extensionInstanceName: mysql
2023-06-17 21:53:26,616 INFO  [main] MysqlDatabaseDialect(MysqlDatabaseDialect.java:87) - Mysql Connection initialize Success,JDBC driver metadata:JdbcDriverMetaData(JdbcMajorVersion=4, jdbcMinorVersion=2, jdbcDriverName=MySQL Connector/J, databaseProductName=MySQL, databaseProductVersion=8.0.33)
Exception in thread "main" java.lang.ClassCastException: org.apache.eventmesh.source.connector.jdbc.dialect.mysql.MysqlDatabaseDialectFactory cannot be cast to org.apache.eventmesh.source.connector.jdbc.dialect.cdc.CdcEngineFactory
	at org.apache.eventmesh.source.connector.jdbc.JdbcAllFactoryLoader.getCdcEngineFactory(JdbcAllFactoryLoader.java:46)
	at org.apache.eventmesh.source.connector.jdbc.connector.JdbcSourceConnector.init(JdbcSourceConnector.java:91)
	at org.apache.eventmesh.openconnect.SourceWorker.<init>(SourceWorker.java:59)
	at org.apache.eventmesh.openconnect.Application.run(Application.java:55)
	at org.apache.eventmesh.source.connector.jdbc.JdbcSourceWorker.main(JdbcSourceWorker.java:26)

image
The first time you get the DatabaseDialectFactory instance. When the second call JdbcAllFactoryLoader getDatabaseDialectFactory method, because the databaseName will again get to cache CdcEngineFactory instance.

So this problem is solved by this PR

@mxsm
Copy link
Member Author

mxsm commented Aug 29, 2023

ping @xwm1992 @pandaapo @Pil0tXia

@mxsm mxsm merged commit 643f059 into apache:master Aug 29, 2023
@mxsm mxsm deleted the eventmesh-4129 branch August 29, 2023 07:06
Comment on lines +94 to +95
private static <T> T getSingletonExtension(Class<T> extensionClass, EventMeshSPI spi, String extensionInstanceName) {
return (T) EXTENSION_INSTANCE_CACHE.computeIfAbsent(new Extension(spi, extensionInstanceName), name -> {
Copy link
Member

Choose a reason for hiding this comment

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

@xwm1992 The main purpose of this PR is to change the cache key from String to an Extension class specified with its SPI annotation, in order to support two extensions loading in one module.

Copy link
Member

Choose a reason for hiding this comment

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

This review is a comment.

xuhongjia pushed a commit to Deckers-Ohana/eventmesh that referenced this pull request Mar 13, 2025
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.

[Enhancement] Enhance the functionality of EventMeshExtensionFactory

5 participants