search

hypfvieh / dbus-java

Improved version of java DBus library provided by freedesktop.org (https://dbus.freedesktop.org/doc/dbus-java/)
https://hypfvieh.github.io/dbus-java/
MIT License
185 stars 73 forks source link

Adds a `DBusIgnore` annotation #164

Closed brett-smith closed 2 years ago

brett-smith commented 2 years ago

Adds a DBusIgnore annotation that allows ignoring methods, preventing them being exported. They will neither appear in introspection data, nor can they be called. This is useful when you are exporting a class directly without an interface, but don't want to expose all methods (perhaps they are used internally). It could also be useful on an interface, where both sides are Java and share the same class definition.

This patch will also specifically ignore DBusInterface.getObjectPath(), which is otherwise included when you export a class directly.

hypfvieh commented 2 years ago

Would you please provide a unit test case for this feature?

brett-smith commented 2 years ago

Apologies for the delay in getting this done, but it's been a bit of wild ride!

Please see the commit message for https://github.com/hypfvieh/dbus-java/pull/164/commits/456e00b18d24afd9a6c712a9d2ce7cd6a31b81fa

hypfvieh commented 2 years ago

Thanks for the PR - really a lot of changes. I have fixed some formatting stuff and will merge now.

brett-smith commented 2 years ago

Yeh sorry about the formatting again, I noticed after I had committed some of it wasn't spaces. I've got Eclipse setup to format with spaces now, hopefully will prevent this happening again.

hypfvieh commented 2 years ago

@brett-smith Something is wrong with these changes. After merging, the 'testArrays' method will fail. mvn clean test-compile surefire:test@jnr-tests -Dtest=TestAll#testArrays

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.465 s <<< FAILURE! - in org.freedesktop.dbus.test.TestAll [ERROR] org.freedesktop.dbus.test.TestAll.testArrays Time elapsed: 0.289 s <<< FAILURE! org.opentest4j.AssertionFailedError: Didn't get the correct this ==> expected: <org.freedesktop.dbus.test.helper.SampleClass@2be48336> but was: <:1.293:/TestAll:null> at org.freedesktop.dbus.test.TestAll.testArrays(TestAll.java:654)

Can you please take a look at this?

Edit: I noticed that the log output is different than before. After the patch the introspection-data gets printed to console, before the patch there is no inspection data printed.

hypfvieh commented 2 years ago

@brett-smith Any news on this? If this doesn't get fixed, I will remove the changes introduced with this PR.

brett-smith commented 1 year ago

I'm sorry I totally missed this reply. I am just about to use dbus-java in a new project and came to see what's changed and stumbled on this.

I see@DBusIgnore is still in the source tree, and the unit tests are now passing. So I guess it's all now OK anyway.

FWIW, my original commit message did mention testArrays(). It was something to do with the shared connection. 

I have fixed all of the fall-out from this *except* for `testArrays()` where the assertion as to whether the remote `this` is the same as local one now fails, and I can't see why. Switching back to `withShared(true)` for both connections fixes this, but this does not seem to be a fair test or what is intended. I would appreciate some guidance one this one.

To progress from this, I think I need clarification as to what should be happening with the shared connections. For tests to be close to what is happening in real usage, they should not be sharing connections for the local and remote side of the tests.