superclass methods not accessible in 1.2.1

[cmacdonald]

arraylist = autoclass("java.util.ArrayList")()
arraylist.iterator()
arraylist.stream()

This works for 1.2.0 but not for 1.2.1.

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-7-5e67e1c90388> in <module>()
----> 1 arraylist.stream()

AttributeError: 'java.util.ArrayList' object has no attribute 'stream'

Reproducible notebook at https://colab.research.google.com/drive/1F9u2jVQR5JFw_mk5Bq--VH1Ki91Xe5x3

stream() is defined in the super-interface as a default.

We also had problem accessing methods in interfaces that extended java.util.List.

[tshirtman]

I can reproduce it locally, let's see if it also happens in CI, i'll look for a fix when i have some time.

[cmacdonald]

CI failed, but I can't see how we broke it in 1.2.1. I can see that getDeclaredMethods() doesn't include methods that are default implementations. (They arent declared in the class or its supertypes).

There is some discussion in https://blog.jooq.org/2018/03/28/correct-reflective-access-to-interface-default-methods-in-java-8-9-10/ - I dont think this is relevant.

Is the solution also to walk all interfaces and their parent interfaces adding default methods? (We could add the abstract methods, but this seems superfluous as they must by implemented by the concrete object).

This is the most relevant stackoverflow thread I found: https://stackoverflow.com/questions/28400408/what-is-the-new-way-of-getting-all-methods-of-a-class-including-inherited-defau

it does seems that there could be some differences in different JVM implementations...

[tshirtman]

It does work if i cast to java.util.Collection first, but it shouldn't be necessary… :/

[tshirtman]

I also think it's because of using getDeclaredMethods instead of getMethods, i tried to walk all the interfaces but somehow it wasn't finding the Collection interface in getInterfaces()…

[tshirtman]

hm, this seems to work… not completely sure why.
https://github.com/kivy/pyjnius/pull/466/files#diff-06f2b31838f083623d82353f734d644a

edit: uh, save for a segfault… https://github.com/kivy/pyjnius/runs/348651345
tried running again in case it was a glitch, crashed again on ubuntu, python3.8, java 10…
edit2: very confused, all current combination worked when i excluded 3.8/java10/ubuntu together, then i enabled java 9 and 11, and got the same crash with 3.7/11/ubuntu… well at least i can install openjdk-11-jdk on my ubuntu and test with python3.7 easily… …and it works. grmbl.

[tshirtman]

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=100815&view=logs&j=696704cc-6fef-57a3-ea36-f27779b8cd5e&t=06421391-4b55-523d-a804-5e1a5bfc1908
incindentally it seems conda-forge build on linux also has some segfaults on 1.2.1, so the issue might have been present before my change.

[cmacdonald]

patch works for me on collab

[cmacdonald]

polite bump. Would be good to get this one merged. I added a comment to the diff suggesting a code comment.

[tshirtman]

Sorry for letting this linger, i just really have no idea what prevents the CI from passing, and i don't really like disabling part of the targets just to silent an incomprehensible error, i agree letting this bitrot is not any better but i'm not sure how to go forward from this.

[cmacdonald]

I had wondered if this was a concurrency issue as per #480, but I don't think pytest is concurrent by default.

I tried reproducing locally, but didn't succeed in reproducing #480. Here was my command - its a Debian-based image:

docker run -i continuumio/anaconda3 /bin/bash <<EOF

cat /etc/os-release
apt-get update
mkdir /usr/share/man/man1
apt-get -y install openjdk-11-jdk-headless gcc ant

conda create -y -n pyjnius python=3.7.5
conda activate pyjnius

git clone https://github.com/kivy/pyjnius.git
cd pyjnius/
python -m pip install -U setuptools cython
python setup.py bdist_wheel
pip install --timeout=120 .[dev,ci]
ant all
cd tests/
CLASSPATH="../build/test-classes:../build/classes" PYTHONPATH=/opt/conda/envs/pyjnius/lib/python3.7/site-packages/ pytest -v
cd ../
git checkout -b issue_465 origin/issue_465

python setup.py bdist_wheel
pip install --timeout=120 .[dev,ci]
ant all
cd tests/
CLASSPATH="../build/test-classes:../build/classes" PYTHONPATH=/opt/conda/envs/pyjnius/lib/python3.7/site-packages/ pytest -v

EOF

All tests passed on master and on the branch.

[asmirnov69]

I observe the same problem, superclass methods are not accessible in 1.2.1 but everything works in 1.2.0. This is quite nasty bug, don't you think it make sense to remove 1.2.1 release or somehow mark it as unusable ?

[cmacdonald]

Sorry for letting this linger, i just really have no idea what prevents the CI from passing, and i don't really like disabling part of the targets just to silent an incomprehensible error, i agree letting this bitrot is not any better but i'm not sure how to go forward from this.

Might I suggest re-running the CI tests? Can we try to narrows this down - is it a problem in this patch, or a problem with a previous version?

[cmacdonald]

To return to this:

I dont think mixing getDeclaredMethods() and getMethods() is the answer. getMethods() is sufficient.

My test cases for this are:

 def test_super_interface(self):
        LinkedList = autoclass('java.util.LinkedList')
        words = LinkedList()
        words.add('hello')
        words.add('world')
        q = cast('java.util.Queue', words)
        self.assertEqual(2, q.size())
        self.assertIsNotNone(q.iterator())

    def test_super_object(self):
        LinkedList = autoclass('java.util.LinkedList')
        words = LinkedList()
        words.hashCode()

    def test_super_interface_object(self):
        LinkedList = autoclass('java.util.LinkedList')
        words = LinkedList()
        q = cast('java.util.Queue', words)
        q.hashCode()

Various fail when we only use getDeclaredMethods().

My only issue with getMethods() is that test_inheritance.py fails. This is only marginally problematic - org.jnius.Child's static newInstance() overrides org.jnius.Parent newInstance() method. What happens is that getMethods() sees both newInstance() methods, so constructs a JavaMultipleMethod. This is wrong: Child.newInstance() should hide Parent.newInstance() - see https://www.java67.com/2012/08/can-we-override-static-method-in-java.html. I also verified this using Jshell:

jshell> org.jnius.Child.newInstance()
$3 ==> org.jnius.Child@506c589e

[cmacdonald]

Hi @tshirtman I see you committed getMethods(). I'd recommend the additional test cases above as well.
I wasnt sure how to solve the static method hiding that Child.newInstance() should hide Parent.newInstance(). I think you would have to reorder the iteration of classes & interfaces in autoclass() - walk the tree to get the classes, and then apply them in reverse order, i.e. starting at java.lang.Object.

[tshirtman]

hm, indeed adding this test shows that currently the object casted as a java.util.Queue autoclass doesn't have a size attribute, which is wrong.
I do agree with your analysis, reverse lookup, replacing parent methods with identical signature instead of making a JavaMultipleMethod, that seems like a good strategy.

[cmacdonald]

#501 passed all tests. Closing this issue. (Praise be).

[dlazesz]

Protected fields still missing!

Changing this line from public to protected fails the test.

[cmacdonald]

There is a discussion on #500 as to whether private/protected methods/fields should be exposed at all.

[tshirtman]

i think that's different, currently we are supposed to have everything, and with your fix, it seems to be true for methods, but apparently it's not (and possibly never was?) true for fields, so i think it's a bug to solve first, whether we then decide to allow filtering the methods/fields we want to see depending on their level of privacy. As this bug is closed, it makes sense to me to open a new one for that case.

[dlazesz]

it seems to be true for methods, but apparently it's not (and possibly never was?) true for fields

It worked for <1.2.1 so I would expect it to work in >1.2.1,<2.0 too even if it should not have worked for any version.