Merge pull request #3754 from jruby/ji-package-refactor

[ji] package (internal) refactoring
jruby · Mar 29, 2016 · ae94c39 · ae94c39
2 parents 81fe82d + 4036c08
commit ae94c39
Showing 12 changed files with 545 additions and 120 deletions.
diff --git a/core/src/main/java/org/jruby/java/util/BlankSlateWrapper.java b/core/src/main/java/org/jruby/java/util/BlankSlateWrapper.java
@@ -19,6 +19,8 @@
  * Because this is in the hierarchy, it does mean any methods that are not Java
  * packages or otherwise defined on the JavaPackageModuleTemplate module will
  * be inaccessible.
+ *
+ * @deprecated no longer used - probably needs revamp if needed  to be re-usable
  */
 public class BlankSlateWrapper extends IncludedModuleWrapper {
     public BlankSlateWrapper(Ruby runtime, RubyClass superClass, RubyModule delegate) {

diff --git a/core/src/main/java/org/jruby/javasupport/Java.java b/core/src/main/java/org/jruby/javasupport/Java.java
@@ -52,7 +52,6 @@
 
 import org.jcodings.Encoding;
 
-import org.jruby.MetaClass;
 import org.jruby.Ruby;
 import org.jruby.RubyArray;
 import org.jruby.RubyBasicObject;
@@ -115,10 +114,9 @@ public class Java implements Library {
 
     @Override
     public void load(Ruby runtime, boolean wrap) {
-        createJavaModule(runtime);
+        final RubyModule Java = createJavaModule(runtime);
 
-        RubyModule jpmt = runtime.defineModule("JavaPackageModuleTemplate");
-        jpmt.getSingletonClass().setSuperClass(new BlankSlateWrapper(runtime, jpmt.getMetaClass().getSuperClass(), runtime.getKernel()));
+        JavaPackage.createJavaPackageClass(runtime, Java);
 
         // load Ruby parts of the 'java' library
         runtime.getLoadService().load("jruby/java.rb", false);
@@ -289,40 +287,6 @@ private static void addNameClassMappings(final Ruby runtime, final Map<String, J
         nameClassMap.put("Void", JavaClass.get(runtime, Void.class));
     }
 
-    private static class JavaPackageClassProvider implements ClassProvider {
-
-        static final JavaPackageClassProvider INSTANCE = new JavaPackageClassProvider();
-
-        public RubyClass defineClassUnder(RubyModule pkg, String name, RubyClass superClazz) {
-            // shouldn't happen, but if a superclass is specified, it's not ours
-            if ( superClazz != null ) return null;
-
-            String packageName = getPackageName(pkg);
-            // again, shouldn't happen. TODO: might want to throw exception instead.
-            if ( packageName == null ) return null;
-
-            final Ruby runtime = pkg.getRuntime();
-            JavaClass javaClass = JavaClass.forNameVerbose(runtime, packageName + name);
-            return (RubyClass) get_proxy_class(runtime.getJavaSupport().getJavaUtilitiesModule(), javaClass);
-        }
-
-        public RubyModule defineModuleUnder(RubyModule pkg, String name) {
-            String packageName = getPackageName(pkg);
-            // again, shouldn't happen. TODO: might want to throw exception instead.
-            if ( packageName == null ) return null;
-
-            final Ruby runtime = pkg.getRuntime();
-            JavaClass javaClass = JavaClass.forNameVerbose(runtime, packageName + name);
-            return get_interface_module(runtime, javaClass);
-        }
-
-        private static String getPackageName(final RubyModule pkg) {
-            final IRubyObject package_name = pkg.getInstanceVariables().getInstanceVariable("@package_name");
-            return package_name == null ? null : package_name.asJavaString();
-        }
-
-    }
-
     public static IRubyObject create_proxy_class(
             IRubyObject self,
             IRubyObject name,
@@ -700,7 +664,8 @@ private static void addToJavaPackageModule(RubyModule proxyClass) {
             className = parentModule == null ? fullName : fullName.substring(endPackage + 1);
         }
 
-        if ( parentModule != null && IdUtil.isConstant(className) ) {
+        if ( parentModule != null && // TODO a Java Ruby class should not validate (as well)
+            ( IdUtil.isConstant(className) || parentModule instanceof JavaPackage ) ) {
             if (parentModule.getConstantAt(className) == null) {
                 parentModule.setConstant(className, proxyClass);
             }
@@ -742,23 +707,16 @@ private static RubyModule getJavaPackageModule(final Ruby runtime, final String
     private static RubyModule createPackageModule(final Ruby runtime,
         final RubyModule parentModule, final String name, final String packageString) {
 
-        final RubyModule packageModule = (RubyModule) runtime.getJavaSupport().getPackageModuleTemplate().dup();
-
-        final String package_name = packageString.length() > 0 ? packageString + '.' : packageString;
-        packageModule.setInstanceVariable( "@package_name", runtime.newString(package_name) );
-
-        // this is where we'll get connected when classes are opened using
-        // package module syntax.
-        packageModule.addClassProvider( JavaPackageClassProvider.INSTANCE );
+        final RubyModule packageModule = JavaPackage.newPackage(runtime, packageString, parentModule);
 
         synchronized (parentModule) { // guard initializing in multiple threads
             final IRubyObject packageAlreadySet = parentModule.fetchConstant(name);
             if ( packageAlreadySet != null ) {
                 return (RubyModule) packageAlreadySet;
             }
             parentModule.setConstant(name.intern(), packageModule);
-            MetaClass metaClass = (MetaClass) packageModule.getMetaClass();
-            metaClass.setAttached(packageModule);
+            //MetaClass metaClass = (MetaClass) packageModule.getMetaClass();
+            //metaClass.setAttached(packageModule);
         }
         return packageModule;
     }
@@ -790,19 +748,15 @@ public static IRubyObject get_package_module_dot_format(final IRubyObject self,
         return module == null ? runtime.getNil() : module;
     }
 
-    private static RubyModule getProxyOrPackageUnderPackage(final ThreadContext context,
+    static RubyModule getProxyOrPackageUnderPackage(final ThreadContext context,
         final RubyModule parentPackage, final String name, final boolean cacheMethod) {
         final Ruby runtime = context.runtime;
 
         if ( name.length() == 0 ) {
             throw runtime.newArgumentError("empty class or package name");
         }
 
-        IRubyObject package_name = parentPackage.getInstanceVariable("@package_name");
-        if ( package_name == null ) throw runtime.newArgumentError("invalid package module");
-
-        final String parentPackageName = package_name.asJavaString();
-        final String fullName = parentPackageName + name;
+        final String fullName = JavaPackage.buildPackageName(parentPackage, name).toString();
 
         final RubyModule result;
 
@@ -1001,10 +955,7 @@ private static class JavaAccessor extends org.jruby.internal.runtime.methods.Jav
         @Override
         public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
             if ( args.length != 0 ) {
-                IRubyObject packageName = parentPackage.callMethod("package_name");
-                throw context.runtime.newArgumentError(
-                    "Java package `" + packageName + "' does not have a method `" + name + "'"
-                );
+                throw JavaPackage.packageMethodArgumentMismatch(context.runtime, parentPackage, name, args.length);
             }
             return call(context, self, clazz, name);
         }
@@ -1492,17 +1443,6 @@ private static int interfacesHashCode(Class[] a) {
         return result;
     }
 
-    @Deprecated
-    private static void addToJavaPackageModule(RubyModule proxyClass, JavaClass javaClass) {
-        addToJavaPackageModule(proxyClass);
-    }
-
-    @Deprecated
-    private static RubyClass createProxyClass(final Ruby runtime,
-                                              final RubyClass baseType, final JavaClass javaClass, boolean invokeInherited) {
-        return createProxyClass(runtime, RubyClass.newClass(runtime, baseType), javaClass, invokeInherited);
-    }
-
     /**
      * @param iface
      * @return the sole un-implemented method for a functional-style interface or null

diff --git a/core/src/main/java/org/jruby/javasupport/JavaPackage.java b/core/src/main/java/org/jruby/javasupport/JavaPackage.java
diff --git a/core/src/main/java/org/jruby/javasupport/JavaSupport.java b/core/src/main/java/org/jruby/javasupport/JavaSupport.java
@@ -83,8 +83,11 @@ public abstract class JavaSupport {
 
     public abstract RubyClass getJavaClassClass();
 
+    public abstract RubyClass getJavaPackageClass() ;
+
     public abstract RubyModule getJavaInterfaceTemplate();
 
+    @Deprecated
     public abstract RubyModule getPackageModuleTemplate();
 
     public abstract RubyClass getJavaProxyClass();

diff --git a/core/src/main/java/org/jruby/javasupport/JavaSupportImpl.java b/core/src/main/java/org/jruby/javasupport/JavaSupportImpl.java
@@ -85,6 +85,7 @@ private static final class UnfinishedProxy extends ReentrantLock {
     private RubyClass javaObjectClass;
     private JavaClass objectJavaClass;
     private RubyClass javaClassClass;
+    private RubyClass javaPackageClass;
     private RubyClass javaArrayClass;
     private RubyClass javaProxyClass;
     private RubyClass arrayJavaProxyCreatorClass;
@@ -275,12 +276,19 @@ public RubyClass getJavaClassClass() {
         return javaClassClass = getJavaModule().getClass("JavaClass");
     }
 
+    public RubyClass getJavaPackageClass() {
+        RubyClass clazz;
+        if ((clazz = javaPackageClass) != null) return clazz;
+        return javaPackageClass = getJavaModule().getClass("JavaPackage");
+    }
+
     public RubyModule getJavaInterfaceTemplate() {
         RubyModule module;
         if ((module = javaInterfaceTemplate) != null) return module;
         return javaInterfaceTemplate = runtime.getModule("JavaInterfaceTemplate");
     }
 
+    @Deprecated
     public RubyModule getPackageModuleTemplate() {
         RubyModule module;
         if ((module = packageModuleTemplate) != null) return module;

diff --git a/core/src/main/ruby/jruby/java.rb b/core/src/main/ruby/jruby/java.rb
@@ -34,7 +34,6 @@
 ###### END LICENSE BLOCK ######
 
 # These are loads so they don't pollute LOADED_FEATURES
-load 'jruby/java/java_package_module_template.rb'
 load 'jruby/java/java_utilities.rb'
 
 load 'jruby/java/core_ext.rb'

diff --git a/core/src/main/ruby/jruby/java/java_module.rb b/core/src/main/ruby/jruby/java/java_module.rb
diff --git a/core/src/main/ruby/jruby/java/java_package_module_template.rb b/core/src/main/ruby/jruby/java/java_package_module_template.rb
diff --git a/spec/java_integration/packages/access_spec.rb b/spec/java_integration/packages/access_spec.rb
@@ -23,17 +23,17 @@
 
   it "can be imported using 'include_package \"package.module\"'" do
     m = Module.new { include_package 'java.lang' }
-    expect(m::System).to respond_to 'getProperty'
+    expect(m::System).to respond_to :getProperty
   end
 
   it "can be imported using 'import package.module" do
     m = Module.new { import java.lang }
-    expect(m::System).to respond_to 'getProperty'
+    expect(m::System).to respond_to 'currentTimeMillis'
   end
 
   it "can be imported using 'import \"package.module\"'" do
     m = Module.new { import 'java.lang' }
-    expect(m::System).to respond_to 'getProperty'
+    m::System.currentTimeMillis
   end
 
   it "supports const_get" do
@@ -42,27 +42,70 @@
   end
 
   it "supports const_get with inherit argument" do
-    expect(java.util.const_get("Arrays", false)).to respond_to :asList
+    expect( java.util.const_get(:Arrays, false) ).to respond_to :asList
   end
 
   it "can be accessed using Java module and CamelCase" do
     expect(Java::JavaLang).to eq(java.lang)
     expect(Java::ComBlahV8Something).to eq(com.blah.v8.something)
     expect(Java::X_Y_).to eq(Java::x_.y_)
   end
+
+  it 'sub-packages work with const_get' do
+    java.const_get(:util)
+    pkg = java::util.const_get(:zip)
+    expect( pkg ).to be_a Module
+    expect( pkg.is_a?(Class) ).to be false
+    expect( pkg ).to equal Java::JavaUtilZip
+
+    klass = java::util.const_get(:StringTokenizer)
+    expect( klass ).to be_a Class
+    expect( klass.name ).to eql 'Java::JavaUtil::StringTokenizer'
+
+    pkg = Java::JavaxSecurityAuth.const_get(:callback, true)
+    expect( pkg ).to eql Java::javax::security::auth::callback
+  end
+
+  it 'does not inherit constants' do
+    #expect( Java::JavaLang::TOP_LEVEL_BINDING ).to raise_error(NameError)
+    begin
+      Java::JavaLang::TOP_LEVEL_BINDING
+    rescue NameError
+    else; fail 'error not raised' end
+    #expect( Java::java.util::Object ).to raise_error(NameError)
+    begin
+      Java::java.util::Object
+    rescue NameError
+    else; fail 'error not raised' end
+  end
+
+  it 'fails with argument passed to package method' do
+    begin
+      javax.script(1)
+    rescue ArgumentError => e
+      expect( e.message ).to eql "Java package 'javax' does not have a method `script' with 1 argument"
+    else; fail 'error not raised' end
+  end
+
 end
 
 # for DefaultPackageClass
 $CLASSPATH << File.dirname(__FILE__) + "/../../../target/test-classes"
 
 describe "class in default package" do
+
   it "can be opened using Java::Foo syntax" do
-    expect(Java::DefaultPackageClass.new.foo).to eq("foo")
+    expect( Java::DefaultPackageClass.new.foo ).to eql "foo"
     class Java::DefaultPackageClass
       def bar; 'bar'; end
     end
-    expect(Java::DefaultPackageClass.new.bar).to eq("bar")
-    expect(Java::DefaultPackageClass.new.foo).to eq("foo")
+    expect( Java::DefaultPackageClass.new.bar ).to eql "bar"
+    expect( Java::DefaultPackageClass.new.foo ).to eql "foo"
+
+    class Java::java::util::StringTokenizer
+      def xxx; 'xxx' end
+    end
+    expect( java::util::StringTokenizer.new('').xxx ).to eql 'xxx'
   end
 
   it "does not failover to a package if there are classloading errors" do

diff --git a/spec/java_integration/packages/name_spec.rb b/spec/java_integration/packages/name_spec.rb
@@ -7,19 +7,34 @@
 
   it 'have name' do
     expect( Java::JavaLang::Integer.name ).to eql 'Java::JavaLang::Integer'
-    #expect( Java::JavaLang.name ).to eql 'Java::JavaLang'
-    #expect( Java::Java.name ).to eql 'Java::Java'
-    #expect( Java::Javax.name ).to eql 'Java::Javax'
+    expect( Java::JavaLang.name ).to eql 'Java::JavaLang'
+    expect( Java::Java.name ).to eql 'Java::Java'
+    expect( Java::Javax.name ).to eql 'Java::Javax'
     expect( Java.name ).to eql 'Java'
-    #expect( Java::java.util.name ).to eql 'Java::JavaUtil'
-    #expect( org.xml.name ).to eql 'Java:OrgXml'
-    #expect( org.xml.sax.name ).to eql 'Java:OrgXmlSax'
+    expect( Java::java.util.name ).to eql 'Java::JavaUtil'
+    expect( org.xml.name ).to eql 'Java::OrgXml'
+    expect( org.xml.sax.name ).to eql 'Java::OrgXmlSax'
     #expect( Java::Default.name ).to eql ''
     # TODO avoid Default package in favor of Java :
     #expect( Java::DefaultPackageClass.name ).to eql 'Java::DefaultPackageClass'
     expect( Java::DefaultPackageClass.name ).to eql 'Java::Default::DefaultPackageClass'
   end
 
+  it 'handles Kernel methods' do
+    expect( Java::JavaLang::Integer.to_s ).to eql 'Java::JavaLang::Integer'
+    expect( Java::JavaLang.to_s ).to eql 'Java::JavaLang'
+    expect( Java::Java.inspect ).to eql 'Java::Java'
+    expect( Java::Javax.to_s ).to eql 'Java::Javax'
+    expect( Java.inspect ).to eql 'Java'
+    expect( Java::java.util.to_s ).to eql 'Java::JavaUtil'
+    expect( org.xml.object_id ).to be_a Fixnum
+    expect( org.xml.sax.singleton_class ).to be_a Class
+    expect( org.xml == org.xml.sax ).to be false
+    expect( org.xml.eql? Java::org::xml ).to be true
+    expect( Java::OrgXmlSax.equal?org.xml.sax ).to be true
+    #expect( Java::OrgXmlSax === org.xml.sax ).to be true
+  end
+
   it 'have package name' do
     expect( Java::JavaLang::Integer.respond_to? :package_name ).to be false
     expect( Java::JavaLang.respond_to? :package_name ).to be true

diff --git a/test/jruby/test_higher_javasupport.rb b/test/jruby/test_higher_javasupport.rb
@@ -446,7 +446,13 @@ def test_void
       Java::void[1].new
       fail "expected to raise"
     rescue ArgumentError => e
-      assert_equal "Java package `void' does not have a method `[]'", e.message
+      assert_equal "Java package 'void' does not have a method `[]' with 1 argument", e.message
+    end
+    assert Java::Void == Java::void
+    assert Java::void.equal? Java::Void
+    quiet do
+      p Java
+      p Java::void
     end
   end
 
@@ -844,8 +850,11 @@ def test_that_misspelt_fq_class_names_dont_stop_future_fq_class_names_with_same_
   end
 
   def test_that_subpackages_havent_leaked_into_other_packages
-    assert_equal(false, Java::java.respond_to?(:zip))
-    assert_equal(false, Java::com.respond_to?(:util))
+    assert ! Java::java.respond_to?(:zip)
+    assert ! Java::com.respond_to?(:util)
+
+    assert Java::java.respond_to_missing?(:zip)
+    assert Java::comx.respond_to_missing?(:foo)
   end
 
   def test_that_sub_packages_called_java_javax_com_org_arent_short_circuited
@@ -870,6 +879,52 @@ def test_uppercase_package_name_and_lowercase_class_name # and upper-case method
     # Java::OrgJrubyJavasupportTestApp::lowerClass
   end if ALLOW_UPPERCASE_PACKAGE_NAMES
 
+  def test_package_class
+    assert org.jruby.class.is_a?(Class)
+    assert_equal org.jruby.class, Java::JavaPackage
+    assert_equal Java::OrgJrubyJavasupport.class, Java::JavaPackage
+
+    assert org.jruby.singleton_class.is_a?(Class)
+    assert_not_equal org.jruby.singleton_class, org.jruby.class
+    assert_not_equal org.jruby.singleton_class, org.jruby.javasupport.singleton_class
+  end
+
+  def test_package_name_colliding_with_name_method
+    assert_equal 'Java::OrgJrubyJavasupport', org.jruby.javasupport.name
+    assert_equal true, org.jruby.javasupport.respond_to?(:name)
+    assert org.jruby.javasupport.test.is_a?(Java::JavaPackage)
+
+    assert_equal 'Java::OrgJrubyJavasupportTest', org.jruby.javasupport.test.name
+    # we can use :: to access the name package :
+    assert Java::OrgJrubyJavasupportTestName.is_a?(Java::JavaPackage)
+    assert Java::OrgJrubyJavasupportTestName::Sample
+  end
+
+  def test_package_object_id
+    assert org.jruby.object_id.is_a?(Fixnum)
+    assert Java::java::lang.object_id.is_a?(Fixnum)
+  end
+
+  def test_package_singleton_method_hooks
+    assert org.respond_to?(:singleton_method_added, true)
+    assert java.lang.respond_to?(:singleton_method_removed, true)
+
+    assert_nil org.__send__(:singleton_method_added, :sym)
+    assert_nil java.lang.__send__(:singleton_method_removed, :sym)
+  end
+
+  def test_package_does_not_respond_to_hidden_methods
+    assert Kernel.respond_to?(:test)
+    assert ! org.respond_to?(:test)
+    assert ! java.lang.respond_to?(:test, true)
+  end
+
+  def test_package_does_respond_to_missing
+    assert org.respond_to_missing?(:test)
+    assert java.lang.respond_to_missing?(:test)
+    assert java.lang.respond_to_missing?(:test, true)
+  end
+
   @@include_proc = Proc.new do
     Thread.stop
     java_import "java.lang.System"
@@ -1197,16 +1252,21 @@ class FooXBarBarBar < Java::JavaLang::Runnable
 
   # JRUBY-781
   def test_that_classes_beginning_with_small_letter_can_be_referenced
-    assert_equal Module, org.jruby.test.smallLetterClazz.class
+    assert_equal Java::JavaPackage, org.jruby.test.smallLetterClazz.class
+    assert org.jruby.test.smallLetterClazz.is_a?(Module)
+    assert ! org.jruby.test.smallLetterClazz.is_a?(Class)
+
     assert_equal Class, org.jruby.test.smallLetterClass.class
+    assert ! org.jruby.test.smallLetterClass.is_a?(Java::JavaPackage)
   end
 
   # JRUBY-1076
   def test_package_module_aliased_methods
     assert java.lang.respond_to?(:__constants__)
-    assert java.lang.respond_to?(:__methods__)
+    assert java.lang.respond_to?(:__methods__, true)
 
-    java.lang.String # ensure java.lang.String has been loaded
+    java.lang.String # ensure java.lang.
+    # String has been loaded
     assert java.lang.__constants__.include?(:String)
   end
 

diff --git a/test/org/jruby/javasupport/test/name/Sample.java b/test/org/jruby/javasupport/test/name/Sample.java
@@ -0,0 +1,3 @@
+package org.jruby.javasupport.test.name;
+
+public class Sample { /* */ }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,3 @@
		package org.jruby.javasupport.test.name;

		public class Sample { /* */ }