microsoftgraph / msgraph-sdk-android

Microsoft Graph SDK for Android! https://graph.microsoft.io
Other
51 stars 43 forks source link

RecurrenceRange values are not being serialized correctly #10

Closed peternied closed 8 years ago

peternied commented 8 years ago

RecurrenceRange's Edm.Date values for StartDate and EndDate is not properly being serialized. Instead of following the full yyyy-mm-ddThh:mm[:ss[.fffffff]] pattern, this needs to be changed to only return the yyyy-mm-dd part of the pattern in the serialized response.

<ComplexType Name="recurrenceRange">
   <Property Name="type" Type="microsoft.graph.recurrenceRangeType" />
   <Property Name="startDate" Type="Edm.Date" />
   <Property Name="endDate" Type="Edm.Date" />
   <Property Name="recurrenceTimeZone" Type="Edm.String" Unicode="false" />
   <Property Name="numberOfOccurrences" Type="Edm.Int32" Nullable="false" />
</ComplexType>
iambmelt commented 8 years ago

@peternied Happy to take a look at this - will dive into the date parsing and get back to you [soon, I hope!]

iambmelt commented 8 years ago

@peternied Does this unjam you? Call it simple, but it seems safe-enough to substring off whatever data we don't want. Eh?

import java.util.*;
import java.text.*;

public class DateParse {

    // yyyy-mm-ddThh:mm[:ss[.fffffff]]
    private static final String sampleDate = "2016-04-19T11:00:00.0000000";

    // our desired format
    private static final String targetFormat = "yyyy-MM-dd";

    public static void main(String[] args) throws Exception {
        // trim off what we don't need
        String simp = sampleDate.substring(0, sampleDate.indexOf("T"));

        // make a formatter for what we want
        SimpleDateFormat sdf = new SimpleDateFormat(targetFormat);

        // parse it into our Date object
        final Date date = sdf.parse(simp);

        // get a calendar instance
        final Calendar calendar = Calendar.getInstance();

        // set it with the parsed date
        calendar.setTime(date);

        // log stuff out
        String msg = "Starting with " + sampleDate
                     + "\nTrimmed to " + simp
                     + "\nUsing format " + targetFormat
                     + "\n\n->Date:[" + date.toString() + "]"
                     + "\n\n->Calendar:[" + calendar.toString() + "]";
        System.out.println(msg);
    }
}

sample output

Starting with 2016-04-19T11:00:00.0000000
Trimmed to 2016-04-19
Using format yyyy-MM-dd

->Date:[Tue Apr 19 00:00:00 PDT 2016]

->Calendar:[java.util.GregorianCalendar[time=1461049200000,areFieldsSet=true,areAllFieldsSet=true,lenient=true,zone=sun.util.calendar.ZoneInfo[id="America/Los_Angeles",offset=-28800000,dstSavings=3600000,useDaylight=true,transitions=185,lastRule=java.util.SimpleTimeZone[id=America/Los_Angeles,offset=-28800000,dstSavings=3600000,useDaylight=true,startYear=0,startMode=3,startMonth=2,startDay=8,startDayOfWeek=1,startTime=7200000,startTimeMode=0,endMode=3,endMonth=10,endDay=1,endDayOfWeek=1,endTime=7200000,endTimeMode=0]],firstDayOfWeek=1,minimalDaysInFirstWeek=1,ERA=1,YEAR=2016,MONTH=3,WEEK_OF_YEAR=17,WEEK_OF_MONTH=4,DAY_OF_MONTH=19,DAY_OF_YEAR=110,DAY_OF_WEEK=3,DAY_OF_WEEK_IN_MONTH=3,AM_PM=0,HOUR=0,HOUR_OF_DAY=0,MINUTE=0,SECOND=0,MILLISECOND=0,ZONE_OFFSET=-28800000,DST_OFFSET=3600000]]
peternied commented 8 years ago

Yea, using the format string would work well. Can you update the way Edm.Date is handled within the Android/TypeHelperAndroid.cs and downstream usages so that the type is properly mapped to produce this serialized output?

iambmelt commented 8 years ago

@peternied -- Can do. Is there a precedent we have for dealing with "special cases" when serializing fields for specific types? Some way you've handled this kind of thing before?

I see RecurrenceRange extends BaseRecurrenceRange -- should all subclasses of BaseRecurrenceRange get this special treatment? Or only RecurrenceRange objects?

Am I right in thinking this change takes the form of modifying the relationship between RecurrenceRange, GsonFactory, and CalendarSerializer?

peternied commented 8 years ago

I think we should either 1) co-opt a type for these 'Date' values or 2) create our own Date object within the core SDK that we can use to represent this other kind of functionality.

Instead of treating the Reocurrance range object in a special manor, we should just handle that specific type different and leave the current CalendarSerializer alone. We night need to update the GsonFactory for this new type by adding a new serializer/deserializer, or we could make the new type automatically do the right thing.

iambmelt commented 8 years ago

Link to WIP

Still to do: tests, updating templates to reflect changes in BaseRecurrenceRange.java

Questions for @peternied

iambmelt commented 8 years ago

Tests added

iambmelt commented 8 years ago

@peternied cleaned up date formatting

peternied commented 8 years ago

@iambmelt could open a PR?