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

[Feature] Interfaces design flaw #328

Open
sitano opened this issue Jan 10, 2025 · 1 comment
Open

[Feature] Interfaces design flaw #328

sitano opened this issue Jan 10, 2025 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@sitano
Copy link
Contributor

sitano commented Jan 10, 2025

Currently, the interfaces (i.e. EventInterface) can't be implemented for any external object because they inherit ClientLike that has unpublished RedisClientInner.

Describe the solution you'd like

Either make interfaces not to inherit : ClientLike {} interface that can't be implemented externally for wrappers

pub trait EventInterface { ... }

or change ClientLike to be implementable outside of Fred

pub trait ClientLike: Clone + Send + Sync + Sized {
  #[doc(hidden)]
  fn client(&self) -> &Arc<RedisClient>;
}

and maybe minimize trait itself.

Additional context

I want to do basically this:

pub struct MyClient {
    inner: Arc<Inner>,
}

struct Inner {
    client: Arc<fred::clients::SubscriberClient>,
}

// OOOOOOPS: without this EventInterface would not do
impl ClientLike for MyClient {
  // OOOOOOPS: that's impossible cuz RedisClientInner is inner
  fn inner(&self) -> &RefCount<RedisClientInner> {
    &self.inner
  }
}

impl EventInterface for MyClient {}
@sitano sitano added the enhancement New feature or request label Jan 10, 2025
@aembke
Copy link
Owner

aembke commented Feb 15, 2025

Hi @sitano, thanks for bringing this up. I hadn't originally intended for folks to implement that trait, but I can see why it would be useful in cases like this. I'm working on a fairly substantial refactor to support this by breaking up the interfaces and semi-public types like ClientInner into separate crates, which should allow for this kind of usage and should hopefully speed up compilation times.

Unfortunately something changed with openssl last week and CI runs are now failing across the board for anything related to TLS (even ones from months ago that passed previously), so it may be a couple weeks before I have a chance to push this, but I may ping you for some early feedback on the new interfaces if that works for you.

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

No branches or pull requests

2 participants