diff --git a/mailnews/addrbook/public/nsIAbAddressCollector.idl b/mailnews/addrbook/public/nsIAbAddressCollector.idl index 7674c73f0c..87887edffe 100644 --- a/mailnews/addrbook/public/nsIAbAddressCollector.idl +++ b/mailnews/addrbook/public/nsIAbAddressCollector.idl @@ -46,7 +46,7 @@ interface nsIAbCard; * It will save and update the supplied addresses into the address book * specified by the "mail.collect_addressbook" pref. */ -[scriptable, uuid(db4eb202-5dec-48d5-b853-b46d8d5bb055)] +[scriptable, uuid(069d3fba-37d4-4158-b401-a8efaeea0b66)] interface nsIAbAddressCollector : nsISupports { /** * Collects email addresses into the address book. @@ -89,9 +89,4 @@ interface nsIAbAddressCollector : nsISupports { in boolean aCreateCard, in unsigned long aSendFormat, [optional] in boolean aSkipCheckExisting); - - /** - * Get the first card with this name / value in the collected ab. - */ - nsIAbCard getCardFromAttribute(in ACString aName, in ACString aValue); }; diff --git a/mailnews/addrbook/src/nsAbAddressCollector.cpp b/mailnews/addrbook/src/nsAbAddressCollector.cpp index 65b60a1485..a2faf5cdcf 100644 --- a/mailnews/addrbook/src/nsAbAddressCollector.cpp +++ b/mailnews/addrbook/src/nsAbAddressCollector.cpp @@ -51,7 +51,7 @@ #include "prmem.h" #include "nsServiceManagerUtils.h" #include "nsComponentManagerUtils.h" -#include "nsIAbMDBDirectory.h" +#include "nsIAbManager.h" NS_IMPL_ISUPPORTS2(nsAbAddressCollector, nsIAbAddressCollector, nsIObserver) @@ -65,22 +65,50 @@ nsAbAddressCollector::~nsAbAddressCollector() { nsresult rv; nsCOMPtr pPrefBranchInt(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv)); - if(NS_SUCCEEDED(rv)) + if (NS_SUCCEEDED(rv)) pPrefBranchInt->RemoveObserver(PREF_MAIL_COLLECT_ADDRESSBOOK, this); } -NS_IMETHODIMP nsAbAddressCollector::GetCardFromAttribute(const nsACString &aName, const nsACString &aValue, nsIAbCard **aCard) +nsIAbCard* +nsAbAddressCollector::GetCardFromProperty(const char *aName, + const nsACString &aValue, + nsIAbDirectory **aDirectory) { - NS_ENSURE_ARG_POINTER(aCard); - if (m_database) - // Please DO NOT change the 3rd param of GetCardFromAttribute() call to - // PR_TRUE (i.e. case insensitive) without reading bugs #128535 and #121478. - return m_database->GetCardFromAttribute(m_directory.get(), - PromiseFlatCString(aName).get(), - aValue, PR_FALSE /* retain case */, - aCard); + nsresult rv; + nsCOMPtr abManager(do_GetService(NS_ABMANAGER_CONTRACTID, &rv)); + NS_ENSURE_SUCCESS(rv, nsnull); - return NS_ERROR_FAILURE; + nsCOMPtr enumerator; + rv = abManager->GetDirectories(getter_AddRefs(enumerator)); + NS_ENSURE_SUCCESS(rv, nsnull); + + PRBool hasMore; + nsCOMPtr supports; + nsCOMPtr directory; + nsIAbCard *result = nsnull; + while (NS_SUCCEEDED(enumerator->HasMoreElements(&hasMore)) && hasMore) + { + rv = enumerator->GetNext(getter_AddRefs(supports)); + NS_ENSURE_SUCCESS(rv, nsnull); + + directory = do_QueryInterface(supports, &rv); + if (NS_FAILED(rv)) + continue; + + // Some implementations may return NS_ERROR_NOT_IMPLEMENTED here, + // so just catch the value and continue. + if (NS_FAILED(directory->GetCardFromProperty(aName, aValue, PR_TRUE, + &result))) + continue; + + if (result) + { + if (aDirectory) + directory.swap(*aDirectory); + return result; + } + } + return nsnull; } NS_IMETHODIMP @@ -88,6 +116,10 @@ nsAbAddressCollector::CollectAddress(const nsACString &aAddresses, PRBool aCreateCard, PRUint32 aSendFormat) { + // If we've not got a valid directory, no point in going any further + if (!mDirectory) + return NS_OK; + // note that we're now setting the whole recipient list, // not just the pretty name of the first recipient. PRUint32 numAddresses; @@ -142,20 +174,25 @@ nsAbAddressCollector::CollectSingleAddress(const nsACString &aEmail, PRUint32 aSendFormat, PRBool aSkipCheckExisting) { + if (!mDirectory) + return NS_OK; + nsresult rv; nsCOMPtr card; PRBool emailAddressIn2ndEmailColumn = PR_FALSE; + nsCOMPtr originDirectory; + if (!aSkipCheckExisting) { - rv = GetCardFromAttribute(NS_LITERAL_CSTRING(kPriEmailProperty), - aEmail, getter_AddRefs(card)); + card = GetCardFromProperty(kPriEmailProperty, aEmail, + getter_AddRefs(originDirectory)); // We've not found a card, but is this address actually in the additional // email column? if (!card) { - rv = GetCardFromAttribute(NS_LITERAL_CSTRING(k2ndEmailProperty), - aEmail, getter_AddRefs(card)); + card = GetCardFromProperty(k2ndEmailProperty, aEmail, + getter_AddRefs(originDirectory)); if (card) emailAddressIn2ndEmailColumn = PR_TRUE; } @@ -164,7 +201,7 @@ nsAbAddressCollector::CollectSingleAddress(const nsACString &aEmail, if (!card && (aCreateCard || aSkipCheckExisting)) { card = do_CreateInstance(NS_ABCARDPROPERTY_CONTRACTID, &rv); - if (NS_SUCCEEDED(rv) && card && m_directory) + if (NS_SUCCEEDED(rv) && card) { // Set up the fields for the new card. SetNamesForCard(card, aDisplayName); @@ -175,13 +212,22 @@ nsAbAddressCollector::CollectSingleAddress(const nsACString &aEmail, card->SetPropertyAsUint32(kPreferMailFormatProperty, aSendFormat); nsCOMPtr addedCard; - rv = m_directory->AddCard(card, getter_AddRefs(addedCard)); + rv = mDirectory->AddCard(card, getter_AddRefs(addedCard)); NS_ASSERTION(NS_SUCCEEDED(rv), "failed to add card"); } } } - else if (card && !emailAddressIn2ndEmailColumn) + else if (card && !emailAddressIn2ndEmailColumn && originDirectory) { + // It could be that the origin directory is read-only, so don't try and + // write to it if it is. + PRBool readOnly; + rv = originDirectory->GetReadOnly(&readOnly); + NS_ENSURE_SUCCESS(rv, rv); + + if (readOnly) + return NS_OK; + // address is already in the AB, so update the names PRBool modifiedCard = PR_FALSE; @@ -205,8 +251,8 @@ nsAbAddressCollector::CollectSingleAddress(const nsACString &aEmail, modifiedCard = PR_TRUE; } - if (modifiedCard && m_directory) - m_directory->ModifyCard(card); + if (modifiedCard) + originDirectory->ModifyCard(card); } return NS_OK; @@ -277,69 +323,71 @@ nsAbAddressCollector::SplitFullName(const nsCString &aFullName, nsCString &aFirs } // Observes the collected address book pref in case it changes. -NS_IMETHODIMP nsAbAddressCollector::Observe(nsISupports *aSubject, const char *aTopic, const PRUnichar *aData) +NS_IMETHODIMP +nsAbAddressCollector::Observe(nsISupports *aSubject, const char *aTopic, + const PRUnichar *aData) { - nsCOMPtr pPrefBranchInt = do_QueryInterface(aSubject); - if (!pPrefBranchInt) { - NS_ASSERTION(pPrefBranchInt, "failed to get prefs"); + nsCOMPtr prefBranch = do_QueryInterface(aSubject); + if (!prefBranch) { + NS_ASSERTION(prefBranch, "failed to get prefs"); return NS_OK; } - nsresult rv; - nsCString prefVal; - pPrefBranchInt->GetCharPref(PREF_MAIL_COLLECT_ADDRESSBOOK, - getter_Copies(prefVal)); - rv = SetAbURI(prefVal); - NS_ASSERTION(NS_SUCCEEDED(rv),"failed to change collected ab"); + SetUpAbFromPrefs(prefBranch); return NS_OK; } // Initialises the collector with the required items. -nsresult nsAbAddressCollector::Init(void) +nsresult +nsAbAddressCollector::Init(void) { nsresult rv; - nsCOMPtr pPrefBranchInt(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv)); + nsCOMPtr prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID, + &rv)); NS_ENSURE_SUCCESS(rv, rv); - rv = pPrefBranchInt->AddObserver(PREF_MAIL_COLLECT_ADDRESSBOOK, this, PR_FALSE); + rv = prefBranch->AddObserver(PREF_MAIL_COLLECT_ADDRESSBOOK, this, PR_FALSE); NS_ENSURE_SUCCESS(rv, rv); - nsCString prefVal; - pPrefBranchInt->GetCharPref(PREF_MAIL_COLLECT_ADDRESSBOOK, - getter_Copies(prefVal)); - return SetAbURI(prefVal); + SetUpAbFromPrefs(prefBranch); + return NS_OK; } // Performs the necessary changes to set up the collector for the specified // collected address book. -nsresult nsAbAddressCollector::SetAbURI(nsCString &aURI) +void +nsAbAddressCollector::SetUpAbFromPrefs(nsIPrefBranch *aPrefBranch) { - if (aURI.IsEmpty()) - aURI.AssignLiteral(kPersonalAddressbookUri); + nsCString abURI; + aPrefBranch->GetCharPref(PREF_MAIL_COLLECT_ADDRESSBOOK, + getter_Copies(abURI)); - if (aURI == m_abURI) - return NS_OK; + if (abURI.IsEmpty()) + abURI.AssignLiteral(kPersonalAddressbookUri); - m_database = nsnull; - m_directory = nsnull; - m_abURI = aURI; + if (abURI == mABURI) + return; + + mDirectory = nsnull; + mABURI = abURI; nsresult rv; - nsCOMPtr rdfService = do_GetService("@mozilla.org/rdf/rdf-service;1", &rv); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr abManager(do_GetService(NS_ABMANAGER_CONTRACTID, &rv)); + NS_ENSURE_SUCCESS(rv, ); - nsCOMPtr resource; - rv = rdfService->GetResource(m_abURI, getter_AddRefs(resource)); - NS_ENSURE_SUCCESS(rv, rv); + rv = abManager->GetDirectory(mABURI, getter_AddRefs(mDirectory)); + NS_ENSURE_SUCCESS(rv, ); - m_directory = do_QueryInterface(resource, &rv); - NS_ENSURE_SUCCESS(rv, rv); + PRBool readOnly; + rv = mDirectory->GetReadOnly(&readOnly); + NS_ENSURE_SUCCESS(rv, ); - nsCOMPtr mdbDir(do_QueryInterface(m_directory, &rv)); - NS_ENSURE_SUCCESS(rv, rv); - - rv = mdbDir->GetDatabase(getter_AddRefs(m_database)); - NS_ENSURE_SUCCESS(rv, rv); - - return rv; + // If the directory is read-only, we can't write to it, so just blank it out + // here, and warn because we shouldn't hit this (UI is wrong). + if (readOnly) + { + NS_ERROR("Address Collection book preferences is set to a read-only book. " + "Address collection will not take place."); + mDirectory = nsnull; + } } diff --git a/mailnews/addrbook/src/nsAbAddressCollector.h b/mailnews/addrbook/src/nsAbAddressCollector.h index a07de41a61..413ad4be8f 100644 --- a/mailnews/addrbook/src/nsAbAddressCollector.h +++ b/mailnews/addrbook/src/nsAbAddressCollector.h @@ -41,11 +41,12 @@ #include "nsIAbAddressCollector.h" #include "nsCOMPtr.h" -#include "nsIAddrDatabase.h" #include "nsIAbDirectory.h" #include "nsIAbCard.h" #include "nsIObserver.h" +class nsIPrefBranch; + class nsAbAddressCollector : public nsIAbAddressCollector, public nsIObserver { @@ -60,14 +61,16 @@ public: nsresult Init(); private: + nsIAbCard* GetCardFromProperty(const char *aName, const nsACString &aValue, + nsIAbDirectory **aDirectory); + void AutoCollectScreenName(nsIAbCard *aCard, const nsACString &aEmail); PRBool SetNamesForCard(nsIAbCard *aSenderCard, const nsACString &aFullName); void SplitFullName(const nsCString &aFullName, nsCString &aFirstName, nsCString &aLastName); - nsresult SetAbURI(nsCString &aURI); - nsCOMPtr m_database; - nsCOMPtr m_directory; - nsCString m_abURI; + void SetUpAbFromPrefs(nsIPrefBranch *aPrefBranch); + nsCOMPtr mDirectory; + nsCString mABURI; }; #endif // _nsAbAddressCollector_H_ diff --git a/mailnews/addrbook/test/unit/data/collect.mab b/mailnews/addrbook/test/unit/data/collect.mab new file mode 100644 index 0000000000..a40af0fbe9 --- /dev/null +++ b/mailnews/addrbook/test/unit/data/collect.mab @@ -0,0 +1 @@ +// < <(a=c)> // (f=iso-8859-1) (B8=LastModifiedDate)(B9=RecordKey)(BA=AddrCharSet)(BB=LastRecordKey) (BC=ns:addrbk:db:table:kind:pab)(BD=ListName)(BE=ListNickName) (BF=ListDescription)(C0=ListTotalAddresses)(C1=LowercaseListName) (C2=ns:addrbk:db:table:kind:deleted) (80=ns:addrbk:db:row:scope:card:all) (81=ns:addrbk:db:row:scope:list:all) (82=ns:addrbk:db:row:scope:data:all)(83=FirstName)(84=LastName) (85=PhoneticFirstName)(86=PhoneticLastName)(87=DisplayName) (88=NickName)(89=PrimaryEmail)(8A=LowercasePrimaryEmail) (8B=SecondEmail)(8C=PreferMailFormat)(8D=PopularityIndex) (8E=AllowRemoteContent)(8F=WorkPhone)(90=HomePhone)(91=FaxNumber) (92=PagerNumber)(93=CellularNumber)(94=WorkPhoneType)(95=HomePhoneType) (96=FaxNumberType)(97=PagerNumberType)(98=CellularNumberType) (99=HomeAddress)(9A=HomeAddress2)(9B=HomeCity)(9C=HomeState) (9D=HomeZipCode)(9E=HomeCountry)(9F=WorkAddress)(A0=WorkAddress2) (A1=WorkCity)(A2=WorkState)(A3=WorkZipCode)(A4=WorkCountry) (A5=JobTitle)(A6=Department)(A7=Company)(A8=_AimScreenName) (A9=AnniversaryYear)(AA=AnniversaryMonth)(AB=AnniversaryDay) (AC=SpouseName)(AD=FamilyName)(AE=WebPage1)(AF=WebPage2)(B0=BirthYear) (B1=BirthMonth)(B2=BirthDay)(B3=Custom1)(B4=Custom2)(B5=Custom3) (B6=Custom4)(B7=Notes)> <(80=0)> {1:^80 {(k^BC:c)(s=9)} [1:^82(^BB=0)]} @$${1{@ < <(a=c)> // (f=iso-8859-1) (C3=DbRowID)> <(86=1)(81=)(82=Other Book)(83=Book)(84=other@book.invalid)(85=Other)> {-1:^80 {(k^BC:c)(s=9)} [1:^82(^BB=1)] [-1(^B0=)(^B7=)(^B6=)(^B8=0)(^88=)(^AE=)(^A6=)(^87^82)(^B3=)(^A8=) (^84^83)(^9F=)(^9E=)(^9C=)(^90=)(^8F=)(^89^84)(^85=)(^A1=)(^99=) (^8E=0)(^8B=)(^8D=0)(^B5=)(^91=)(^B1=)(^A4=)(^9D=)(^9B=)(^92=)(^93=) (^8C=0)(^83^85)(^A2=)(^B2=)(^9A=)(^B4=)(^A3=)(^86=)(^A0=)(^C3=1) (^A5=)(^A7=)(^AF=)(^8A^84)(^B9=1)]} @$$}1}@ \ No newline at end of file diff --git a/mailnews/addrbook/test/unit/test_collection.js b/mailnews/addrbook/test/unit/test_collection.js index 5514b6f9b7..6ba132437a 100644 --- a/mailnews/addrbook/test/unit/test_collection.js +++ b/mailnews/addrbook/test/unit/test_collection.js @@ -2,8 +2,10 @@ /* * Test suite for the Address Collector Service. * - * XXX Todo: May still need to add test for - * nsIAbAddressCollector::getCardFromAttribute if its kept. + * This tests the main collection functions for adding new cards and modifying + * existing ones. + * + * Tests against cards in different ABs are done in test_collection_2.js. */ const nsIAbPMF = Components.interfaces.nsIAbPreferMailFormat; diff --git a/mailnews/addrbook/test/unit/test_collection_2.js b/mailnews/addrbook/test/unit/test_collection_2.js new file mode 100644 index 0000000000..2e55d198a9 --- /dev/null +++ b/mailnews/addrbook/test/unit/test_collection_2.js @@ -0,0 +1,57 @@ +/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* + * Test suite for the Address Collector Service part 2. + * + * This test checks that we don't collect addresses when they already exist + * in other address books. + */ + +const nsIAbPMF = Components.interfaces.nsIAbPreferMailFormat; + +function run_test() +{ + // Test - Get the address collecter + + var prefService = Components.classes["@mozilla.org/preferences-service;1"] + .getService(Components.interfaces.nsIPrefBranch); + + var abManager = Components.classes["@mozilla.org/abmanager;1"] + .getService(Components.interfaces.nsIAbManager); + + // Get the actual collecter + var addressCollect = + Components.classes["@mozilla.org/addressbook/services/addressCollector;1"] + .getService(Components.interfaces.nsIAbAddressCollector); + + // Set the new pref afterwards to ensure we change correctly + prefService.setCharPref("mail.collect_addressbook", kCABData.URI); + + // For this test use an address book that isn't the one we're collecting + // to. + var testAB = do_get_file("../mailnews/addrbook/test/unit/data/collect.mab"); + + testAB.copyTo(gProfileDir, kPABData.fileName); + + // XXX Getting all directories ensures we create all ABs because the + // address collecter can't currently create ABs itself (bug 314448). + var temp = abManager.directories; + + addressCollect.collectAddress("Other Book ", true, + nsIAbPMF.unknown); + + var PAB = abManager.getDirectory(kPABData.URI); + + var childCards = PAB.childCards; + + do_check_true(childCards.hasMoreElements()); + + var card = childCards.getNext().QueryInterface(Components.interfaces.nsIAbCard); + + do_check_eq(card.displayName, "Other Book"); + do_check_eq(card.primaryEmail, "other@book.invalid"); + + // Check the CAB has no cards. + var CAB = abManager.getDirectory(kCABData.URI); + + do_check_false(CAB.childCards.hasMoreElements()); +};