JFXtras / jfxtras

A supporting library for JavaFX, containing helper classes, extended layouts, controls and other interesting widgets.
http://jfxtras.org
Other
599 stars 123 forks source link

NullPointerException when parsing VCalendar #116

Open Olafxso opened 5 years ago

Olafxso commented 5 years ago

Hi,

I tried to read a calendar but get an error:

java.lang.NullPointerException
    at jfxtras.icalendarfx.VParentBase.checkChild(VParentBase.java:370)
    at jfxtras.icalendarfx.VParentBase.addChildInternal(VParentBase.java:397)
    at jfxtras.icalendarfx.VParentBase.parseContent(VParentBase.java:305)
    at jfxtras.icalendarfx.VParentBase.parseContent(VParentBase.java:283)
    at jfxtras.icalendarfx.VCalendar.parse(VCalendar.java:956)
    at TestVCalendar.main(TestVCalendar.java:18)

. Here is my test case:

public class TestVCalendar
{
  public static void main( String[] args )
  {
    try (InputStream fin = new URL(
        "https://api.socialschools.eu/api/v1/icalfeed/?schoolId=1406&roleTypeId=1&userId=8452ab23-41bc-4fe7-bdb2-1a232ebe16ea&hash=B23ERfs2lNttZ17UG78QmPY2mPLFKCVVn6cGYwSEGs" )
            .openStream())
    {
      InputStreamReader reader = new InputStreamReader( fin, StandardCharsets.UTF_8 );
      System.out.println( VCalendar.parse( reader ) );
    }
    catch ( Exception e )
    {
      e.printStackTrace();
    }
  }
}

I also make a workaround in VParentBase.checkChild. The variable getter is null in this case. I have Move the part below if ( !isChildAllowed )to an else block. Now it is ok for me.

Here is the complete method:

   protected boolean checkChild( List<Message> messages, String content, String elementName, VChild newChild )
  {
    int initialMessageSize = messages.size();
    if ( newChild == null )
    {
      Message message = new Message( this,
          "Ignored invalid element:" + content,
          MessageEffect.MESSAGE_ONLY );
      messages.add( message );
    }
    Method getter = getGetter( newChild );
    boolean isChildAllowed = getter != null;
    if ( !isChildAllowed )
    {
      Message message = new Message( this,
          elementName + " not allowed in " + name(),
          MessageEffect.THROW_EXCEPTION );
      messages.add( message );
    }
    else // Moved to an else block, because getter could be null here
    {
      final boolean isChildAlreadyPresent;
      Object currentParameter = null;
      try
      {
        currentParameter = getter.invoke( this );
      }
      catch ( IllegalAccessException | IllegalArgumentException | InvocationTargetException e )
      {
        e.printStackTrace();
      }
      if ( currentParameter instanceof Collection )
      {
        isChildAlreadyPresent = ( (Collection<?>)currentParameter ).contains( newChild ); // TODO contains is expensive - try to find a way to avoid
      }
      else
      {
        isChildAlreadyPresent = currentParameter != null;
      }
      if ( isChildAlreadyPresent )
      {
        Message message = new Message( this,
            newChild.getClass().getSimpleName() + " can only occur once in a calendar component.  Ignoring instances beyond first.",
            MessageEffect.MESSAGE_ONLY );
        messages.add( message );
      }
    }
    return messages.size() == initialMessageSize;
  }

What do you think of this solution?

daviddbal commented 5 years ago

Yes, I think your code change is smart. Would you like to commit it and make a PR?