Closed
Bug 404085
Opened 18 years ago
Closed 18 years ago
Consolidate provider methods into a default calendar implementation
Categories
(Calendar :: Internal Components, defect)
Calendar
Internal Components
Tracking
(Not tracked)
RESOLVED
FIXED
0.8
People
(Reporter: Fallen, Assigned: Fallen)
References
Details
(Whiteboard: [gdata-0.4])
Attachments
(1 file, 2 obsolete files)
123.30 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
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.
Attachment #289034 -
Flags: review?(daniel.boelzle)
Assignee | ||
Updated•18 years ago
|
Whiteboard: [gdata-cvs]
Target Milestone: --- → 0.8
Assignee | ||
Comment 1•18 years ago
|
||
Some small fixes to first patch
Attachment #289034 -
Attachment is obsolete: true
Attachment #289181 -
Flags: review?(daniel.boelzle)
Attachment #289034 -
Flags: review?(daniel.boelzle)
![]() |
||
Comment 2•18 years ago
|
||
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
Attachment #289181 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Comment 3•18 years ago
|
||
comments as discussed, will elaborate later.
Checked in on HEAD and MOZILLA_1_8_BRANCH
-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
![]() |
||
Comment 4•18 years ago
|
||
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'
Assignee | ||
Comment 5•18 years ago
|
||
(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.
Attachment #289181 -
Attachment is obsolete: true
Attachment #289526 -
Flags: review+
Assignee | ||
Comment 6•18 years ago
|
||
Missing file also checked in.
Assignee | ||
Updated•18 years ago
|
Whiteboard: [gdata-cvs] → [gdata-0.4]
You need to log in
before you can comment on or make changes to this bug.
Description
•