Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 28, 2025

Load wrappers on separate class loader

Analysis completed:

  • Explored repository structure and identified wrapper plugins (mongo, redis, postgres, gremlin)
  • Analyzed current plugin loading mechanism in ArcadeDBServer.registerPlugins()
  • Identified that plugins are currently loaded using Class.forName() with main class loader
  • Located relevant plugin classes:
    • MongoDBProtocolPlugin
    • RedisProtocolPlugin
    • PostgresProtocolPlugin
    • GremlinServerPlugin

Implementation completed:

  • Create a dedicated ClassLoader utility class for wrapper plugins
  • Modify ArcadeDBServer.registerPlugins() to identify wrapper plugins and load them with dedicated class loader
  • Ensure proper resource management and cleanup for dedicated class loaders
  • Add unit tests for the WrapperPluginClassLoader functionality
  • Test the implementation with validation logic
  • Create comprehensive documentation
  • Refactor tests to use AssertJ assertions instead of JUnit assertions

Implementation details:

  • Created WrapperPluginClassLoader that extends URLClassLoader
  • Modified ArcadeDBServer.registerPlugins() to detect wrapper plugins by class name patterns
  • Added proper cleanup in ArcadeDBServer.stop() to close dedicated class loaders
  • Implemented singleton pattern for class loaders per plugin type
  • Converted all test assertions from JUnit (assertTrue, assertEquals, assertNull, etc.) to AssertJ (assertThat(...).isTrue(), assertThat(...).isEqualTo(...), assertThat(...).isNull(), etc.)
  • Updated imports to use AssertJ static imports

Key Features:

  • Automatic Detection: Wrapper plugins are automatically detected by class name patterns
  • Isolation: Each wrapper plugin type gets its own dedicated class loader
  • Resource Management: Proper cleanup of class loaders during server shutdown
  • Backward Compatibility: Regular plugins continue using main class loader
  • Logging: Clear logging when wrapper plugins are loaded with dedicated class loaders
  • Modern Test Framework: Tests now use AssertJ for better readability and fluent API

Testing completed:

  • Unit tests for WrapperPluginClassLoader logic using AssertJ assertions
  • Integration tests for plugin detection and isolation using AssertJ assertions
  • Validation script confirms correct plugin identification
  • All wrapper plugins properly detected: MongoDB, Redis, PostgreSQL, Gremlin
  • Non-wrapper plugins correctly excluded from dedicated class loader usage
Original prompt

This section details on the original issue you should resolve

<issue_title>Load wrappers on separate class Loader</issue_title>
<issue_description>## Expected behavior
Load wrappers (mongo, redis, postgres, gremlin) on dedicated class loader

Actual behavior

wrappers are loaded on the main class loader

depends on #156 </issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #157

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Load wrappers on separate class Loader Load wrapper plugins on dedicated class loaders for better isolation Sep 28, 2025
Copilot AI requested a review from robfrank September 28, 2025 22:09
@codacy-production
Copy link

codacy-production bot commented Sep 28, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.01% 90.70%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (f7a27da) 77229 49479 64.07%
Head commit (b0c0683) 77271 (+42) 49496 (+17) 64.06% (-0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#2578) 43 39 90.70%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

@robfrank
Copy link
Collaborator

@copilot refactor the tests to use assertj assertions instead of junit ones

Copy link
Contributor Author

Copilot AI commented Sep 29, 2025

@copilot refactor the tests to use assertj assertions instead of junit ones

Refactored both test classes to use AssertJ assertions instead of JUnit ones. Changes include:

  • Replaced JUnit static imports with AssertJ imports (import static org.assertj.core.api.Assertions.assertThat)
  • Converted assertTrue/assertFalse to assertThat(...).isTrue()/assertThat(...).isFalse()
  • Converted assertEquals to assertThat(...).isEqualTo(...)
  • Converted assertNull to assertThat(...).isNull()
  • Converted assertNotNull to assertThat(...).isNotNull()
  • Converted assertSame/assertNotSame to assertThat(...).isSameAs(...)/assertThat(...).isNotSameAs(...)
  • Converted assertDoesNotThrow to assertThatCode(...).doesNotThrowAnyException()

Commit: bb6b3cb

@robfrank robfrank force-pushed the copilot/fix-f2bc99e0-032f-4a2c-9477-1922d21b4238 branch from bb6b3cb to 135ba71 Compare November 15, 2025 14:28
@mergify
Copy link
Contributor

mergify bot commented Nov 15, 2025

🧪 CI Insights

Here's what we observed from your CI run for b0c0683.

🟢 All jobs passed!

But CI Insights is watching 👀

Copilot AI and others added 5 commits December 22, 2025 16:51
Co-authored-by: robfrank <413587+robfrank@users.noreply.github.com>
Co-authored-by: robfrank <413587+robfrank@users.noreply.github.com>
Co-authored-by: robfrank <413587+robfrank@users.noreply.github.com>
@robfrank robfrank force-pushed the copilot/fix-f2bc99e0-032f-4a2c-9477-1922d21b4238 branch from 135ba71 to b0c0683 Compare December 22, 2025 15:51
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.

Load wrappers on separate class Loader

2 participants