The attached patch (via subversive) fixes the problem described and also
implements RubyHash#values
class RubyHash
==============
- entrySet() implemented as per the original TODO
- values() implemented
- keySet() reimplemented in same style as values() without dependency on inner
class ConversionSet()
class TestBSF
=============
- Tests added/extended to test the new functionality
Other Details
=============
- Inner class RubyHash.ConversionSet removed (It was partially an unnecessary
reimpementation of java.util.AbstractCollection methods)
- Four new private static inner classes to wrap the Java to JRuby type
conversion. (All four classes are relatively simple wrapping classes.)
As the four inner classes are static, they could easily be made top level
classes, but I can't see need for the reuse anywhere else. Three of the inner
classes could also be made non-static inner classes which would simplify the
code a little. I decided that private static inner classes was the most
readable/maintainable option.
Aside
=====
There is an implicit assumption in the original RubyHash that there should be a
transparent conversion between JRuby and Java types, when using the
java.util.Map inteface to a RubyHash. This patch continues with this implicit
assumption by correcting the interface where it was not fully implemented.
Index: src/org/jruby/RubyHash.java
===================================================================
--- src/org/jruby/RubyHash.java (revision 2120)
+++ src/org/jruby/RubyHash.java (working copy)
@@ -20,6 +20,7 @@
* Copyright (C) 2004 Stefan Matthias Aust <[EMAIL PROTECTED]>
* Copyright (C) 2005 Charles O Nutter <[EMAIL PROTECTED]>
* Copyright (C) 2006 Ola Bini <[EMAIL PROTECTED]>
+ * Copyright (C) 2006 Tim Azzopardi <[EMAIL PROTECTED]>
*
* Alternatively, the contents of this file may be used under the terms of
* either of the GNU General Public License Version 2 or later (the "GPL"),
@@ -36,7 +37,8 @@
package org.jruby;
import java.io.IOException;
-import java.lang.reflect.Array;
+import java.util.AbstractCollection;
+import java.util.AbstractSet;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
@@ -45,7 +47,6 @@
import java.util.Set;
import org.jruby.javasupport.JavaUtil;
-import org.jruby.javasupport.util.ConversionIterator;
import org.jruby.runtime.Arity;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.callback.Callback;
@@ -582,13 +583,9 @@
}
}
- public Set keySet() {
- return new ConversionSet(valueMap.keySet());
- }
public Set entrySet() {
- // TODO: Set.Entry must be wrapped appropriately...?
- return new ConversionSet(valueMap.entrySet());
+ return new ConversionMapEntrySet(getRuntime(),
valueMap.entrySet());
}
public int size() {
@@ -595,121 +592,171 @@
return valueMap.size();
}
- public Collection values() {
- // TODO Auto-generated method stub
- return null;
- }
-
public void clear() {
valueMap.clear();
}
-
- class ConversionSet implements Set {
- private Set set;
- public ConversionSet(Set set) {
- this.set = set;
- }
+ public Collection values() {
+ return new AbstractCollection() {
+ public Iterator iterator() {
+ return new
IteratorAdapter(entrySet().iterator()) {
+ public Object next() {
+ return ((Map.Entry)
super.next()).getValue();
+ }
+ };
+ }
- public int size() {
- return set.size();
- }
+ public int size() {
+ return RubyHash.this.size();
+ }
- public boolean isEmpty() {
- return set.isEmpty();
- }
-
- public boolean contains(Object element) {
- return
set.contains(JavaUtil.convertJavaToRuby(getRuntime(), element));
- }
-
- public Iterator iterator() {
- return new ConversionIterator(set.iterator());
- }
+ public boolean contains(Object v) {
+ return RubyHash.this.containsValue(v);
+ }
+ };
+ }
+
+ public Set keySet() {
+ return new AbstractSet() {
+ public Iterator iterator() {
+ return new
IteratorAdapter(entrySet().iterator()) {
+ public Object next() {
+ return ((Map.Entry)
super.next()).getKey();
+ }
+ };
+ }
- public Object[] toArray() {
- Object[] array = new Object[size()];
- Iterator iter = iterator();
-
- for (int i = 0; iter.hasNext(); i++) {
- array[i] = iter.next();
+ public int size() {
+ return RubyHash.this.size();
}
+ };
+ }
- return array;
+ /**
+ * Convenience adaptor for delegating to an Iterator.
+ *
+ */
+ private static class IteratorAdapter implements Iterator {
+ private Iterator iterator;
+
+ public IteratorAdapter(Iterator iterator) {
+ this.iterator = iterator;
}
+ public boolean hasNext() {
+ return iterator.hasNext();
+ }
+ public Object next() {
+ return iterator.next();
+ }
+ public void remove() {
+ iterator.remove();
+ }
+ }
+
+
+ /**
+ * Wraps a Set of Map.Entry (See #entrySet) such that JRuby types are
mapped to Java types and vice verce.
+ *
+ */
+ private static class ConversionMapEntrySet extends AbstractSet {
+ protected Set mapEntrySet;
+ protected IRuby runtime;
- public Object[] toArray(final Object[] arg) {
- Object[] array = arg;
- int length = size();
-
- if(array.length < length) {
- Class type = array.getClass().getComponentType();
- array = (Object[]) Array.newInstance(type, length);
+ public ConversionMapEntrySet(IRuby runtime, Set mapEntrySet) {
+ this.mapEntrySet = mapEntrySet;
+ this.runtime = runtime;
+ }
+ public Iterator iterator() {
+ return new ConversionMapEntryIterator(runtime,
mapEntrySet.iterator());
+ }
+ public boolean contains(Object o) {
+ if (!(o instanceof Map.Entry)) {
+ return false;
}
-
- Iterator iter = iterator();
- for (int i = 0; iter.hasNext(); i++) {
- array[i] = iter.next();
+ return mapEntrySet.contains(getRubifiedMapEntry((Map.Entry) o));
+ }
+
+ public boolean remove(Object o) {
+ if (!(o instanceof Map.Entry)) {
+ return false;
}
-
- return array;
+ return mapEntrySet.remove(getRubifiedMapEntry((Map.Entry) o));
}
-
- public boolean add(Object element) {
- return set.add(JavaUtil.convertJavaToRuby(getRuntime(),
element));
- }
-
- public boolean remove(Object element) {
- return
set.remove(JavaUtil.convertJavaToRuby(getRuntime(), element));
+ public int size() {
+ return mapEntrySet.size();
}
-
- public boolean containsAll(Collection c) {
- for (Iterator iter = c.iterator(); iter.hasNext();) {
- if (!contains(iter.next())) {
- return false;
+ public void clear() {
+ mapEntrySet.clear();
+ }
+ private Entry getRubifiedMapEntry(final Map.Entry mapEntry) {
+ return new Map.Entry(){
+ public Object getKey() {
+ return
JavaUtil.convertJavaToRuby(runtime, mapEntry.getKey());
}
- }
-
- return true;
+ public Object getValue() {
+ return
JavaUtil.convertJavaToRuby(runtime, mapEntry.getValue());
+ }
+ public Object setValue(Object arg0) {
+ // This should never get called in this
context, but if it did...
+ throw new
UnsupportedOperationException("unexpected call in this context");
+ }
+ };
}
+ }
+
+ /**
+ * Wraps a RubyHash#entrySet#iterator such that the Map.Entry returned by
next() will have its key and value
+ * mapped from JRuby types to Java types where applicable.
+ */
+ private static class ConversionMapEntryIterator implements Iterator {
+ private Iterator iterator;
+ private IRuby runtime;
- public boolean addAll(Collection c) {
- for (Iterator iter = c.iterator(); iter.hasNext(); ) {
- add(iter.next());
- }
+ public ConversionMapEntryIterator(IRuby runtime, Iterator iterator) {
+ this.iterator = iterator;
+ this.runtime = runtime;
+ }
- return !c.isEmpty();
- }
-
- public boolean retainAll(Collection c) {
- boolean listChanged = false;
-
- for (Iterator iter = iterator(); iter.hasNext();) {
- Object element = iter.next();
- if (!c.contains(element)) {
- remove(element);
- listChanged = true;
- }
- }
+ public boolean hasNext() {
+ return iterator.hasNext();
+ }
- return listChanged;
- }
-
- public boolean removeAll(Collection c) {
- boolean changed = false;
-
- for (Iterator iter = c.iterator(); iter.hasNext();) {
- if (remove(iter.next())) {
- changed = true;
- }
- }
+ public Object next() {
+ return new ConversionMapEntry(runtime, ((Map.Entry)
iterator.next()));
+ }
- return changed;
- }
+ public void remove() {
+ iterator.remove();
+ }
+ }
+
+
+ /**
+ * Wraps a Map.Entry from RubyHash#entrySet#iterator#next such that the
the key and value
+ * are mapped from/to JRuby/Java types where applicable.
+ */
+ private static class ConversionMapEntry implements Map.Entry {
+ private Entry entry;
+ private IRuby runtime;
- public void clear() {
- set.clear();
- }
-
- }
+ public ConversionMapEntry(IRuby runtime, Map.Entry entry) {
+ this.entry = entry;
+ this.runtime = runtime;
+ }
+
+ public Object getKey() {
+ IRubyObject rubyObject = (IRubyObject) entry.getKey();
+ return JavaUtil.convertRubyToJava(rubyObject, Object.class);
+ }
+
+ public Object getValue() {
+ IRubyObject rubyObject = (IRubyObject) entry.getValue();
+ return JavaUtil.convertRubyToJava(rubyObject, Object.class);
+ }
+
+ public Object setValue(Object value) {
+ return entry.setValue(JavaUtil.convertJavaToRuby(runtime, value));
+ }
+ }
+
}
Index: test/org/jruby/javasupport/test/TestBSF.java
===================================================================
--- test/org/jruby/javasupport/test/TestBSF.java (revision 2115)
+++ test/org/jruby/javasupport/test/TestBSF.java (working copy)
@@ -33,6 +33,7 @@
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;
+import java.util.ArrayList;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -37,6 +38,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.bsf.BSFException;
import org.apache.bsf.BSFManager;
@@ -64,6 +66,8 @@
public void tearDown() {
manager = null;
}
+
+
public void testList() {
try {
@@ -163,6 +167,24 @@
}
}
+ /**
+ * Tests the use of RubyHash when used from java.
+ * Tests:
+ * RubyHash#keySet()
+ * RubyHash#get()
+ * RubyHash#keySet()#iterator()#hasNext()
+ * RubyHash#keySet()#iterator()#next()
+ * RubyHash#keySet()#remove()
+ * RubyHash#keySet()#contains()
+ * RubyHash#keySet()#containsAll()
+ * RubyHash#values()
+ * RubyHash#values()#iterator()
+ * RubyHash#values()#iterator()#hasNext()
+ * RubyHash#values()#iterator()#next()
+ * RubyHash#values()#contains()
+ * RubyHash#values()#containsAll()
+ * RubyHash#values()#remove()
+ */
public void testMap() {
try {
SimpleInterface si = (SimpleInterface)
manager.eval("ruby", "(java)", 1, 1, "SimpleInterfaceImpl.new");
@@ -168,17 +190,143 @@
SimpleInterface si = (SimpleInterface)
manager.eval("ruby", "(java)", 1, 1, "SimpleInterfaceImpl.new");
Map map = si.getMap();
- for (Iterator e = map.keySet().iterator(); e.hasNext();
) {
- Object key = e.next();
+ List values = new ArrayList();
+ List keys = new ArrayList();
+
+
+ Iterator valuesIterator = map.values().iterator();
+ assertTrue(valuesIterator.hasNext());
+
+ // Iterate over the RubyHash keySet, simultaneously
iterating over the values()
+ for (Iterator keySetIterator = map.keySet().iterator();
keySetIterator.hasNext(); ) {
+ Object key = keySetIterator.next();
+
+ // Get the value from the map via the key
Object value = map.get(key);
+
assertTrue(key.getClass() == String.class);
assertTrue(value.getClass() == Long.class);
+
+ // Get the value from the map via the values
iterator
+ Object valueViaValuesIterator = valuesIterator.next();
+
+ // Check the 2 values obtained via different means
+
assertTrue(value.equals(valueViaValuesIterator));
+
+ // Note that WE CAN'T say the following,
because of the on-the-fly conversion of Fixnum to Long
+ // assertTrue(value == valueViaValuesIterator);
+
+ assertTrue(map.keySet().contains(key));
+ assertTrue(map.values().contains(value));
}
+ assertFalse(valuesIterator.hasNext());
+
+ assertTrue(map.keySet().containsAll(keys));
+ assertTrue(map.values().containsAll(values));
+
+ assertTrue(map.keySet().contains("A"));
+ assertTrue(map.values().contains(new Long(1)));
+ assertFalse(map.keySet().remove("-"));
+ assertTrue(map.keySet().remove("A"));
+ assertFalse(map.keySet().contains("A"));
+ assertFalse(map.values().contains(new Long(1)));
+
+ assertTrue(map.keySet().contains("B"));
+ assertTrue(map.values().contains(new Long(2)));
+ assertFalse(map.values().remove("-"));
+ assertTrue(map.values().remove(new Long(2)));
+ assertFalse(map.values().contains(new Long(2)));
+ assertFalse(map.keySet().contains("B"));
+
+
} catch (BSFException e) {
- fail("Problem evaluating List Test: " + e);
+ fail("Problem evaluating Map Test: " + e);
}
}
+ /**
+ * Tests the use of RubyHash when used from java.
+ * Tests:
+ * RubyHash#entrySet()
+ * RubyHash#entrySet()#iterator()#hasNext()
+ * RubyHash#entrySet()#iterator()#next()
+ * RubyHash#entrySet()#iterator()#next()#setValue()
+ */
+ public void testMapEntrySetIterator() {
+
+ class TestMapValue { private int i; private String s; TestMapValue() {i
= 1; s="2";} public String toString(){ return s + i; } }
+
+ try {
+ SimpleInterface si = (SimpleInterface) manager.eval("ruby",
"(java)", 1, 1, "SimpleInterfaceImpl.new");
+ Map map = si.getMap();
+ int iteration = 1;
+ for (Iterator e = map.entrySet().iterator(); e.hasNext();) {
+ Object o = e.next();
+ assertNotNull(o);
+ Map.Entry entry = (Map.Entry) o;
+ assertTrue(entry.getKey().getClass() == String.class);
+ assertTrue(entry.getValue().getClass() == Long.class);
+ if (iteration++ == 1) {
+ assertEquals("A", entry.getKey());
+ assertEquals(new Long(1L), entry.getValue());
+ // Set a value in the RubyHash
+ entry.setValue(new Long(3));
+ } else {
+ assertEquals("B", entry.getKey());
+ assertEquals(new Long(2L), entry.getValue());
+ // Set a value in the RubyHash
+ entry.setValue(new TestMapValue());
+ }
+ }
+ // Check the entry.setValue values come back out ok
+
+ iteration = 1;
+ for (Iterator e = map.entrySet().iterator(); e.hasNext();) {
+ Object o = e.next();
+ assertNotNull(o);
+ Map.Entry entry = (Map.Entry) o;
+ assertTrue(entry.getKey().getClass() == String.class);
+ if (iteration++ == 1) {
+ assertTrue(entry.getValue().getClass() == Long.class);
+ assertEquals("A", entry.getKey());
+ assertEquals(new Long(3L), entry.getValue());
+ } else {
+ assertTrue(entry.getValue().getClass() ==
TestMapValue.class);
+ assertEquals("B", entry.getKey());
+ assertEquals("21", entry.getValue().toString());
+ }
+ }
+ } catch (BSFException e) {
+ fail("Problem evaluating testMapEntrySetIterator Test: " + e);
+ }
+ }
+
+ /**
+ * Tests the use of RubyHash when used from java.
+ * Tests:
+ * RubyHash#entrySet()#contains()
+ * RubyHash#entrySet()#remove()
+ */
+ public void testMapEntrySetContainsAndRemove() {
+ try {
+ SimpleInterface si = (SimpleInterface) manager.eval("ruby",
"(java)", 1, 1, "SimpleInterfaceImpl.new");
+ Map map = si.getMap();
+ Set entrySet = map.entrySet();
+ Iterator e = entrySet.iterator();
+ Object next1 = e.next();
+ Object next2 = e.next();
+ assertFalse(e.hasNext());
+ assertTrue(entrySet.contains(next1));
+ assertTrue(entrySet.contains(next2));
+ entrySet.remove(next1);
+ assertFalse(entrySet.contains(next1));
+ entrySet.remove(next2);
+ assertFalse(entrySet.contains(next2));
+ } catch (BSFException e) {
+ fail("Problem evaluating testMapEntrySetContainsAndRemove Test: "
+ e);
+ }
+ }
+
public void testModifyMap() {
try {
SimpleInterface si = (SimpleInterface)
manager.eval("ruby", "(java)", 1, 1, "MODIFY_MAP = SimpleInterfaceImpl.new");
\ No newline at end of file
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jruby-devel mailing list
Jruby-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jruby-devel