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

IonStructLite bugfixes #155

Merged
merged 4 commits into from Sep 17, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/software/amazon/ion/UnknownSymbolException.java
Expand Up @@ -29,10 +29,17 @@ public class UnknownSymbolException
private static final long serialVersionUID = 1L;

private final int mySid;
private final String myText;

public UnknownSymbolException(int sid)
{
mySid = sid;
myText = null;
}
public UnknownSymbolException(String message)
{
myText = message;
mySid = 0;
}

public int getSid()
Expand All @@ -43,6 +50,10 @@ public int getSid()
@Override
public String getMessage()
{
return "Unknown symbol text for $" + mySid;
if(myText == null) {
return "Unknown symbol text for $" + mySid;
} else {
return myText;
}
}
}
19 changes: 7 additions & 12 deletions src/software/amazon/ion/impl/lite/IonStructLite.java
Expand Up @@ -34,14 +34,14 @@
import software.amazon.ion.impl.PrivateCurriedValueFactory;
import software.amazon.ion.util.Equivalence;
import java.util.Set;
import software.amazon.ion.UnknownSymbolException;

final class IonStructLite
extends IonContainerLite
implements IonStruct
{
private static final int HASH_SIGNATURE =
IonType.STRUCT.toString().hashCode();

// TODO amzn/ion-java#41: add support for _isOrdered

IonStructLite(ContainerlessContext context, boolean isNull)
Expand All @@ -54,14 +54,13 @@ private IonStructLite(IonStructLite existing, IonContext context)
super(existing, context, true);
// field map can be shallow cloned due to it dealing with String and Integer
// values - both of which are immutable constructs and so safe to retain as references
this._field_map = null == _field_map
? null
: new HashMap<String, Integer>(existing._field_map);
this._field_map = null == existing._field_map ? null : new HashMap<String, Integer>(existing._field_map);
this._field_map_duplicate_count = existing._field_map_duplicate_count;
this.hasNullFieldName = existing.hasNullFieldName;
}

private Map<String, Integer> _field_map;

private boolean hasNullFieldName = false;

public int _field_map_duplicate_count;

Expand Down Expand Up @@ -402,9 +401,9 @@ public IonValue get(String fieldName)
IonValue field;

if (field_idx < 0) {
if(hasNullFieldName) throw new UnknownSymbolException("Unable to resolve fieldname due to null fields.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Null fields' is what's happening within the implementation, but from an Ion perspective isn't the issue. I don't know if this is any less wordy than the original, but here's my stab at it:

"Unable to determine whether the field exists because the struct contains field names with unknown text."

field = null;
}
else {
} else {
field = get_child(field_idx);
}

Expand Down Expand Up @@ -450,11 +449,6 @@ public boolean add(IonValue child)
{
// TODO validate in struct.setFieldName
String text = child.getFieldNameSymbol().getText();
if (text != null)
{
validateFieldName(text);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this was silly.

}

IonValueLite concrete = (IonValueLite) child;
_add(text, concrete);

Expand Down Expand Up @@ -483,6 +477,7 @@ protected void handle(IonValue newValue)
*/
private void _add(String fieldName, IonValueLite child)
{
hasNullFieldName |= fieldName == null;
int size = get_child_count();

// add this to the Container child collection
Expand Down
17 changes: 17 additions & 0 deletions test/software/amazon/ion/StructTest.java
Expand Up @@ -22,6 +22,8 @@
import java.util.Iterator;
import java.util.ListIterator;
import java.util.Random;

import com.sun.org.apache.bcel.internal.classfile.Unknown;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
Expand Down Expand Up @@ -738,6 +740,21 @@ public void testMakeNullRemovesChildsContainer()
val.getContainer());
}

@Test
public void testUnknownSymbolException()
{
String input = "$ion_symbol_table::{imports:[{name:\"foo\", version:1, max_id:1}]} { $10: \"Unknown symbol\"}";
IonDatagram dg = loader().load(input);
IonStruct val = (IonStruct) dg.get(0);
try {
val.get("z");
fail("Should've thrown UnknownSymbolException");
} catch (UnknownSymbolException e) { }
try {
val = system().clone(val);
} catch (UnknownSymbolException e) { }
}

@Test
public void testRemoveAfterClone()
{
Expand Down