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

Thoughts on changing visibility of EventRepository-related classes to public #554

Open
pcuriel opened this issue Apr 15, 2024 · 3 comments
Assignees
Labels
in: event publication registry Event publication registry meta: waiting for feedback Waiting for feedback of the original reporter

Comments

@pcuriel
Copy link

pcuriel commented Apr 15, 2024

Currently, different implementations of EventPublicationRepository and their related classes (e.g., JdbcEventPublicationRepository, JdbcEventPublicationRepository and its JpaEventPublication, MongoDbEventPublicationRepository and its MongoDbEventPublicationRepository, etc.) have package-protected visibility.

This makes it impossible to reuse any of these classes, and even for simple customizations the only alternative left is copying & pasting all the classes. Wouldn't it make more sense to change visibility to public, to ease extensibility?

@odrotbohm
Copy link
Member

It could. That said, the current design considers the repository implementations an implementation detail of the EventPublicationRegistry and doesn't expect anyone to extend it. That, of course, doesn't have to stay that way, but I'd much rather open things up gradually as soon as we understand the actual needs for extension. Having everything open by default would mean we're unable to change a thing about them anymore, as there might be someone already compiling against the types. The repository interfaces have changed quite a bit since their inception.

Can you elaborate what the reuse would look like and why that would be the technology-specific interfaces and not the repository interface itself?

@odrotbohm odrotbohm added in: event publication registry Event publication registry meta: waiting for feedback Waiting for feedback of the original reporter labels Apr 16, 2024
@odrotbohm odrotbohm self-assigned this Apr 16, 2024
@pcuriel
Copy link
Author

pcuriel commented Apr 16, 2024

For instance, a case where I think reusing existing implementation can be helpful is customizing the DB schema.

class CustomMongoDbEventPublicationRepository extends MongoDbEventPublicationRepository {

	/*
	 * Note: in this case, these two methods should be changed from static to instance too, apart from the visibility change.
	 */

	protected MongoDbEventPublication domainToDocument(TargetEventPublication publication) {

		return new CustomMongoDbEventPublication( //
				publication.getIdentifier(), //
				publication.getPublicationDate(), //
				publication.getTargetIdentifier().getValue(), //
				publication.getEvent(),
				// ... additional fields that apply
				);
	}

	protected TargetEventPublication documentToDomain(MongoDbEventPublication document) {
		return new CustomMongoDbEventPublicationAdapter((CustomMongoDbEventPublication) document);
	}

}

For this particular use case, could even be better if the repository implementation is defined like this:

public abstract class AbstractMongoDbEventPublicationRepository<DOCUMENT> {
  ...
}

class CustomMongoDbEventPublicationRepository extends AbstractMongoDbEventPublicationRepository<MongoDbEventPublication> {
}

In fact, maybe having a public Abstract Repository implementation and a package-private concrete one could be a solution for this problem you mention:

Having everything open by default would mean we're unable to change a thing about them anymore, as there might be someone already compiling against the types.

@zikozee
Copy link

zikozee commented May 8, 2024

@odrotbohm
Another Scenario is managing the EventPublication as an Entity in jpa module

right now I am forced to create a duplicate entity for EventPublication to mirror the already created table
and then manage the repository by exposing what I need.

I guess if the incomplete Events as is exposed List/Stream in the incompleteEventPublications interface would solve this
image

the same way completedEventsPublications interface has a findAll()
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: event publication registry Event publication registry meta: waiting for feedback Waiting for feedback of the original reporter
Projects
None yet
Development

No branches or pull requests

3 participants