404085 - Consolidate provider methods into a default calendar implementation

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Create Default Calendar Implementation

We have a lot of stuff that is done the same in each providers. Using js inheritance, we can consolidate all that into a default calendar implementation. This patch takes care.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Create Default Calendar Implementation

Some small fixes to first patch

Comment 2

[Daniel Boelzle [:dbo]]

Comment on attachment 289181 [details] [diff] [review]
Create Default Calendar Implementation

>+++ calendar/providers/default/calDefaultCalendar.js	2007-11-18 01:12:23.471211200 +0100
I'd prefer calCalendarBase or calProviderBase (or similar) expressing that it is meant for inheritance only. Add an ASSERT(false, "just inherit, call!") or throw in calCalendarBase, too.

>+    QueryInterface: function cDC_QueryInterface() {
>+        if (!aIID.equals(Components.interfaces.nsISupports) &&
>+            !aIID.equals(Components.interfaces.calICalendar)) {
>+            throw Components.results.NS_ERROR_NO_INTERFACE;
>+        }
use ensureIID for brevity.

>+const kStorageServiceContractID = "@mozilla.org/storage/service;1";
>+const kStorageServiceIID = Components.interfaces.mozIStorageService;
>+
>+const kCalICalendar = Components.interfaces.calICalendar;
>+
>+const kCalAttendeeContractID = "@mozilla.org/calendar/attendee;1";
>+const kCalIAttendee = Components.interfaces.calIAttendee;
>+var CalAttendee;
>+
>+const kCalRecurrenceInfoContractID = "@mozilla.org/calendar/recurrence-info;1";
>+const kCalIRecurrenceInfo = Components.interfaces.calIRecurrenceInfo;
>+var CalRecurrenceInfo;
>+
>+const kCalRecurrenceRuleContractID = "@mozilla.org/calendar/recurrence-rule;1";
>+const kCalIRecurrenceRule = Components.interfaces.calIRecurrenceRule;
>+var CalRecurrenceRule;
>+
>+const kCalRecurrenceDateSetContractID = "@mozilla.org/calendar/recurrence-date-set;1";
>+const kCalIRecurrenceDateSet = Components.interfaces.calIRecurrenceDateSet;
>+var CalRecurrenceDateSet;
>+
>+const kCalRecurrenceDateContractID = "@mozilla.org/calendar/recurrence-date;1";
>+const kCalIRecurrenceDate = Components.interfaces.calIRecurrenceDate;
>+var CalRecurrenceDate;
>+
>+const kMozStorageStatementWrapperContractID = "@mozilla.org/storage/statement-wrapper;1";
>+const kMozStorageStatementWrapperIID = Components.interfaces.mozIStorageStatementWrapper;
>+var MozStorageStatementWrapper;
>+
>+if (!kMozStorageStatementWrapperIID) {
>+    dump("*** mozStorage not available, calendar/storage provider will not function\n");
>+}
>+
>+function initCalStorageCalendarComponent() {
>+    CalAttendee = new Components.Constructor(kCalAttendeeContractID, kCalIAttendee);
>+    CalRecurrenceInfo = new Components.Constructor(kCalRecurrenceInfoContractID, kCalIRecurrenceInfo);
>+    CalRecurrenceRule = new Components.Constructor(kCalRecurrenceRuleContractID, kCalIRecurrenceRule);
>+    CalRecurrenceDateSet = new Components.Constructor(kCalRecurrenceDateSetContractID, kCalIRecurrenceDateSet);
>+    CalRecurrenceDate = new Components.Constructor(kCalRecurrenceDateContractID, kCalIRecurrenceDate);
>+    MozStorageStatementWrapper = new Components.Constructor(kMozStorageStatementWrapperContractID, kMozStorageStatementWrapperIID);
>+}
why did you move the above code from calStorageCalendar?

>+            try {
>+                var fileurl = iosvc.newFileURI(f);
>+                loader.loadSubScript(fileurl.spec, null);
>+            } catch (e) {
>+                dump("Error while loading " + fileurl.spec + "\n");
>+                throw e;
>+            }
please repair all the module code to use Components.util.reportError instead of dump.

>+++ calendar/providers/caldav/calDavCalendar.js	17 Nov 2007 23:51:29 -0000
> function calDavCalendar() {
>-    this.wrappedJSObject = this;
>-    this.mObservers = new calListenerBag(Components.interfaces.calIObserver);
>     this.unmappedProperties = [];
>     this.mPendingStartupRequests = [];
>     this.mUriParams = null;
>     this.mEtagCache = [];
>     this.mDisabled = false;
>+    this.initDefaultCalendar();
> }
call base ctors before anything selse.

>     QueryInterface: function (aIID) {
>         if (!aIID.equals(Components.interfaces.nsISupports) &&
>             !aIID.equals(Components.interfaces.calICalendarProvider) &&
>             !aIID.equals(Components.interfaces.calICalendar) &&
>             !aIID.equals(Components.interfaces.nsIInterfaceRequestor)) {
>             throw Components.results.NS_ERROR_NO_INTERFACE;
>         }
I'd prefer you revise and shorten QueryInterface implementations, too, e.g. adding

// yet untested!
function queryInterface(that, aList, aIID) {
    function checkIID(iid) {
        return iid.equals(aIID);
    }
    if (aList.some(checkIID)) {
        return that;
    }
    var base = that.__proto__.__proto__;
    if (base && base.QueryInterface) {
        return base.QueryInterface.call(that, aIID);
    }
    throw Components.results.NS_ERROR_NO_INTERFACE;
}

to calUtils.js

>         webSvc.report(calendarDirResource, queryDoc, true, reportListener,
>                       this, null);
>-        return;    
>+        return;
why leave this return statement?

> function calMemoryCalendar() {
>-    this.wrappedJSObject = this;
>     this.calendarToReturn = this,
>     this.initMemoryCalendar();
>+    this.initDefaultCalendar();
> }
base ctor before anything else

r=dbo with that fixed/explained/resolved

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

comments as discussed, will elaborate later.

Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED

Comment 4

[Robert Kaiser]

This broke all calendar-related builds on trunk:
gmake[8]: *** No rule to make target `calStorageCalendarModule.js'.  Stop.
gmake[8]: Leaving directory `/mnt/mozilla/build/seamonkey-dbg/calendar/providers/storage'

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Attachment: Patch as checked in

(In reply to comment #2)
> why did you move the above code from calStorageCalendar?
Moved back and code changed as discussed

> I'd prefer you revise and shorten QueryInterface implementations, too, e.g.
> adding [...]
Added doQueryInterface to calUtils.js

> >         webSvc.report(calendarDirResource, queryDoc, true, reportListener,
> >                       this, null);
> >-        return;    
> >+        return;
> why leave this return statement?
This was just a result of my autoclean script, I didn't really touch this segment of the code, and since some local problems made creating the second patch pretty annoying I left this in for now.


Everything else taken care of.

Comment 6

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Missing file also checked in.