HaikuDepot: Refactor notification and screenshots

Changes to the package model to factor-out
screenshots, to adjust the notifications and to
adjust depots so that packages cannot be added
more than once under the same name.

Change-Id: I2f9219a1cbc0f42631bddff00852c447fd66c929
Reviewed-on: https://review.haiku-os.org/c/haiku/+/8563
Tested-by: Commit checker robot <no-reply+buildbot@haiku-os.org>
Reviewed-by: Jérôme Duval <jerome.duval@gmail.com>

src/apps/haikudepot/Jamfile

@@ -183,6 +183,7 @@ local applicationSources =
 	PackageInfo.cpp
 	PackageLocalInfo.cpp
 	PackageLocalizedText.cpp
+	PackageScreenshotInfo.cpp
 	PublisherInfo.cpp
 	ScreenshotInfo.cpp
 	UserInfo.cpp

src/apps/haikudepot/model/PackageInfoListener.h

@@ -18,12 +18,10 @@ enum {
 	PKG_CHANGED_LOCAL_INFO					= 1 << 3,
 		// ^ Covers state, download and size.
 	PKG_CHANGED_ICON						= 1 << 4,
-	PKG_CHANGED_CHANGELOG					= 1 << 5,
+		// ^ Handled slightly differently.
-	PKG_CHANGED_CLASSIFICATION				= 1 << 6,
+	PKG_CHANGED_CLASSIFICATION				= 1 << 5,
 		// ^ This covers categories, prominence and is native desktop
-	PKG_CHANGED_DEPOT						= 1 << 8,
+	PKG_CHANGED_VERSION_CREATE_TIMESTAMP	= 1 << 6
-	PKG_CHANGED_VERSION						= 1 << 9,
-	PKG_CHANGED_VERSION_CREATE_TIMESTAMP	= 1 << 10
 	// ...
 };
 

src/apps/haikudepot/packagemodel/DepotInfo.cpp

@@ -123,7 +123,10 @@ DepotInfo::AddPackage(PackageInfoRef& package)
 			fPackages.end(),
 			package,
 			&_IsPackageBefore);
-	fPackages.insert(itInsertionPt, package);
+	if (fPackages.end() != itInsertionPt && (*itInsertionPt)->Name() == package->Name())
+		*itInsertionPt = package;
+	else
+		fPackages.insert(itInsertionPt, package);
 }
 
 

src/apps/haikudepot/packagemodel/PackageInfo.cpp

@@ -24,17 +24,20 @@ PackageInfo::PackageInfo()
 	:
 	fName(),
 	fVersion(),
+	fVersionCreateTimestamp(0),
 	fPublisher(),
-	fLocalizedText(),
-	fPackageClassificationInfo(),
-	fScreenshotInfos(),
-	fUserRatingInfo(),
-	fLocalInfo(),
 	fArchitecture(),
 	fDepotName(""),
+
+	fLocalizedText(),
+	fClassificationInfo(),
+	fScreenshotInfo(),
+	fUserRatingInfo(),
+	fLocalInfo(),
+
+	fListeners(),
 	fIsCollatingChanges(false),
-	fCollatedChanges(0),
+	fCollatedChanges(0)
-	fVersionCreateTimestamp(0)
 {
 }
 
@@ -43,15 +46,18 @@ PackageInfo::PackageInfo(const BPackageInfo& info)
 	:
 	fName(info.Name()),
 	fVersion(info.Version()),
+	fVersionCreateTimestamp(0),
 	fPublisher(),
-	fPackageClassificationInfo(),
-	fScreenshotInfos(),
-	fUserRatingInfo(),
 	fArchitecture(info.ArchitectureName()),
 	fDepotName(""),
+
+	fClassificationInfo(),
+	fScreenshotInfo(),
+	fUserRatingInfo(),
+
+	fListeners(),
 	fIsCollatingChanges(false),
-	fCollatedChanges(0),
+	fCollatedChanges(0)
-	fVersionCreateTimestamp(0)
 {
 	BString publisherURL;
 	if (info.URLList().CountStrings() > 0)
@@ -89,17 +95,20 @@ PackageInfo::PackageInfo(const PackageInfo& other)
 	:
 	fName(other.fName),
 	fVersion(other.fVersion),
+	fVersionCreateTimestamp(other.fVersionCreateTimestamp),
 	fPublisher(other.fPublisher),
-	fLocalizedText(other.fLocalizedText),
-	fPackageClassificationInfo(other.fPackageClassificationInfo),
-	fScreenshotInfos(other.fScreenshotInfos),
-	fUserRatingInfo(other.fUserRatingInfo),
-	fLocalInfo(other.fLocalInfo),
 	fArchitecture(other.fArchitecture),
 	fDepotName(other.fDepotName),
+
+	fLocalizedText(other.fLocalizedText),
+	fClassificationInfo(other.fClassificationInfo),
+	fScreenshotInfo(other.fScreenshotInfo),
+	fUserRatingInfo(other.fUserRatingInfo),
+	fLocalInfo(other.fLocalInfo),
+
+	fListeners(),
 	fIsCollatingChanges(false),
-	fCollatedChanges(0),
+	fCollatedChanges(0)
-	fVersionCreateTimestamp(other.fVersionCreateTimestamp)
 {
 }
 
@@ -109,14 +118,14 @@ PackageInfo::operator=(const PackageInfo& other)
 {
 	fName = other.fName;
 	fVersion = other.fVersion;
+	fVersionCreateTimestamp = other.fVersionCreateTimestamp;
 	fPublisher = other.fPublisher;
 	fLocalizedText = other.fLocalizedText;
-	fPackageClassificationInfo = other.fPackageClassificationInfo;
+	fClassificationInfo = other.fClassificationInfo;
-	fScreenshotInfos = other.fScreenshotInfos;
+	fScreenshotInfo = other.fScreenshotInfo;
 	fUserRatingInfo = other.fUserRatingInfo;
 	fArchitecture = other.fArchitecture;
 	fDepotName = other.fDepotName;
-	fVersionCreateTimestamp = other.fVersionCreateTimestamp;
 	fLocalInfo = other.fLocalInfo;
 
 	return *this;
@@ -131,8 +140,8 @@ PackageInfo::operator==(const PackageInfo& other) const
 		&& fVersion == other.fVersion
 		&& fPublisher == other.fPublisher
 		&& fLocalizedText == other.fLocalizedText
-		&& fPackageClassificationInfo == other.fPackageClassificationInfo
+		&& fClassificationInfo == other.fClassificationInfo
-		&& fScreenshotInfos == other.fScreenshotInfos
+		&& fScreenshotInfo == other.fScreenshotInfo
 		&& fUserRatingInfo == fUserRatingInfo
 		&& fArchitecture == other.fArchitecture
 		&& fVersionCreateTimestamp == other.fVersionCreateTimestamp;
@@ -146,6 +155,26 @@ PackageInfo::operator!=(const PackageInfo& other) const
 }
 
 
+uint32
+PackageInfo::DiffMask(const PackageInfo& other) const
+{
+	uint32 result = 0;
+	if (fLocalizedText != fLocalizedText)
+		result |= PKG_CHANGED_LOCALIZED_TEXT;
+	if (fScreenshotInfo != other.fScreenshotInfo)
+		result |= PKG_CHANGED_SCREENSHOTS;
+	if (fUserRatingInfo != other.fUserRatingInfo)
+		result |= PKG_CHANGED_RATINGS;
+	if (fLocalInfo != other.fLocalInfo)
+		result |= PKG_CHANGED_LOCAL_INFO;
+	if (fClassificationInfo != other.fClassificationInfo)
+		result |= PKG_CHANGED_CLASSIFICATION;
+	if (fVersionCreateTimestamp != other.fVersionCreateTimestamp)
+		result |= PKG_CHANGED_VERSION_CREATE_TIMESTAMP;
+	return result;
+}
+
+
 PackageLocalizedTextRef
 PackageInfo::LocalizedText() const
 {
@@ -159,7 +188,6 @@ PackageInfo::SetLocalizedText(PackageLocalizedTextRef value)
 	if (fLocalizedText != value) {
 		fLocalizedText = value;
 		_NotifyListeners(PKG_CHANGED_LOCALIZED_TEXT);
-		_NotifyListeners(PKG_CHANGED_CHANGELOG);
 			// TODO; separate out these later - they are bundled for now to keep the existing
 			// logic working.
 	}
@@ -203,45 +231,23 @@ PackageInfo::SetLocalInfo(PackageLocalInfoRef& localInfo)
 void
 PackageInfo::SetPackageClassificationInfo(PackageClassificationInfoRef value)
 {
-	if (value != fPackageClassificationInfo) {
+	if (value != fClassificationInfo) {
-		fPackageClassificationInfo = value;
+		fClassificationInfo = value;
 		_NotifyListeners(PKG_CHANGED_CLASSIFICATION);
 	}
 }
 
 
 void
-PackageInfo::ClearScreenshotInfos()
+PackageInfo::SetScreenshotInfo(PackageScreenshotInfoRef value)
 {
-	if (!fScreenshotInfos.empty()) {
+	if (value != fScreenshotInfo) {
-		fScreenshotInfos.clear();
+		fScreenshotInfo = value;
 		_NotifyListeners(PKG_CHANGED_SCREENSHOTS);
 	}
 }
 
 
-int32
-PackageInfo::CountScreenshotInfos() const
-{
-	return fScreenshotInfos.size();
-}
-
-
-ScreenshotInfoRef
-PackageInfo::ScreenshotInfoAtIndex(int32 index) const
-{
-	return fScreenshotInfos[index];
-}
-
-
-void
-PackageInfo::AddScreenshotInfo(const ScreenshotInfoRef& info)
-{
-	fScreenshotInfos.push_back(info);
-	_NotifyListeners(PKG_CHANGED_SCREENSHOTS);
-}
-
-
 void
 PackageInfo::SetVersionCreateTimestamp(uint64 value)
 {
@@ -255,10 +261,7 @@ PackageInfo::SetVersionCreateTimestamp(uint64 value)
 void
 PackageInfo::SetDepotName(const BString& depotName)
 {
-	if (fDepotName != depotName) {
+	fDepotName = depotName;
-		fDepotName = depotName;
-		_NotifyListeners(PKG_CHANGED_DEPOT);
-	}
 }
 
 

src/apps/haikudepot/packagemodel/PackageInfo.h

@@ -7,8 +7,6 @@
 #define PACKAGE_INFO_H
 
 
-#include <vector>
-
 #include <Referenceable.h>
 #include <package/PackageInfo.h>
 
@@ -18,6 +16,7 @@
 #include "PackageInfoListener.h"
 #include "PackageLocalInfo.h"
 #include "PackageLocalizedText.h"
+#include "PackageScreenshotInfo.h"
 #include "PublisherInfo.h"
 #include "ScreenshotInfo.h"
 #include "UserRatingInfo.h"
@@ -37,6 +36,8 @@ public:
 			bool				operator==(const PackageInfo& other) const;
 			bool				operator!=(const PackageInfo& other) const;
 
+			uint32				DiffMask(const PackageInfo& other) const;
+
 			const BString&		Name() const
 									{ return fName; }
 
@@ -57,7 +58,7 @@ public:
 									PackageClassificationInfoRef value);
 			PackageClassificationInfoRef
 								PackageClassificationInfo() const
-									{ return fPackageClassificationInfo; }
+									{ return fClassificationInfo; }
 
 			UserRatingInfoRef	UserRatingInfo() const;
 			void				SetUserRatingInfo(UserRatingInfoRef userRatingInfo);
@@ -65,11 +66,10 @@ public:
 			PackageLocalInfoRef	LocalInfo() const;
 			void				SetLocalInfo(PackageLocalInfoRef& localInfo);
 
-			void				ClearScreenshotInfos();
+			PackageScreenshotInfoRef
-			void				AddScreenshotInfo(
+								ScreenshotInfo() const
-									const ScreenshotInfoRef& info);
+									{ return fScreenshotInfo; }
-			int32				CountScreenshotInfos() const;
+			void				SetScreenshotInfo(PackageScreenshotInfoRef value);
-			ScreenshotInfoRef	ScreenshotInfoAtIndex(int32 index) const;
 
 			void				SetVersionCreateTimestamp(uint64 value);
 			uint64				VersionCreateTimestamp() const
@@ -95,26 +95,27 @@ private:
 private:
 			BString				fName;
 			BPackageVersion		fVersion;
+				// milliseconds since epoch
+			uint64				fVersionCreateTimestamp;
 			PublisherInfo		fPublisher;
+			BString				fArchitecture;
+			BString				fDepotName;
+
 			PackageLocalizedTextRef
 								fLocalizedText;
 			PackageClassificationInfoRef
-								fPackageClassificationInfo;
+								fClassificationInfo;
-			std::vector<ScreenshotInfoRef>
+			PackageScreenshotInfoRef
-								fScreenshotInfos;
+								fScreenshotInfo;
 			UserRatingInfoRef	fUserRatingInfo;
 			PackageLocalInfoRef	fLocalInfo;
 
 			std::vector<PackageInfoListenerRef>
 								fListeners;
-			BString				fArchitecture;
-			BString				fDepotName;
-
 			bool				fIsCollatingChanges;
 			uint32				fCollatedChanges;
 
-			uint64				fVersionCreateTimestamp;
+
-				// milliseconds since epoch
 };
 
 

src/apps/haikudepot/packagemodel/PackageScreenshotInfo.cpp

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2024, Andrew Lindesay <apl@lindesay.co.nz>.
+ * All rights reserved. Distributed under the terms of the MIT License.
+ */
+
+
+#include "PackageScreenshotInfo.h"
+
+
+PackageScreenshotInfo::PackageScreenshotInfo()
+{
+}
+
+
+PackageScreenshotInfo::PackageScreenshotInfo(const PackageScreenshotInfo& other)
+	:
+	fScreenshotInfos(other.fScreenshotInfos)
+{
+}
+
+
+PackageScreenshotInfo&
+PackageScreenshotInfo::operator=(const PackageScreenshotInfo& other)
+{
+	fScreenshotInfos = other.fScreenshotInfos;
+	return *this;
+}
+
+
+bool
+PackageScreenshotInfo::operator==(const PackageScreenshotInfo& other) const
+{
+	return fScreenshotInfos == other.fScreenshotInfos;
+}
+
+
+bool
+PackageScreenshotInfo::operator!=(const PackageScreenshotInfo& other) const
+{
+	return !(*this == other);
+}
+
+
+void
+PackageScreenshotInfo::Clear()
+{
+	fScreenshotInfos.clear();
+}
+
+
+int32
+PackageScreenshotInfo::Count() const
+{
+	return fScreenshotInfos.size();
+}
+
+
+ScreenshotInfoRef
+PackageScreenshotInfo::ScreenshotAtIndex(int32 index) const
+{
+	return fScreenshotInfos[index];
+}
+
+
+void
+PackageScreenshotInfo::AddScreenshot(const ScreenshotInfoRef& info)
+{
+	fScreenshotInfos.push_back(info);
+}

src/apps/haikudepot/packagemodel/PackageScreenshotInfo.h

@@ -0,0 +1,42 @@
+/*
+ * Copyright 2024, Andrew Lindesay <apl@lindesay.co.nz>.
+ * All rights reserved. Distributed under the terms of the MIT License.
+ */
+#ifndef PACKAGE_SCREENSHOT_INFO_H
+#define PACKAGE_SCREENSHOT_INFO_H
+
+
+#include <vector>
+
+#include <Referenceable.h>
+#include <String.h>
+
+#include "ScreenshotInfo.h"
+
+
+class PackageScreenshotInfo : public BReferenceable
+{
+public:
+								PackageScreenshotInfo();
+								PackageScreenshotInfo(const PackageScreenshotInfo& other);
+
+			PackageScreenshotInfo&
+								operator=(const PackageScreenshotInfo& other);
+			bool				operator==(const PackageScreenshotInfo& other) const;
+			bool				operator!=(const PackageScreenshotInfo& other) const;
+
+			void				Clear();
+			void				AddScreenshot(const ScreenshotInfoRef& info);
+			int32				Count() const;
+			ScreenshotInfoRef	ScreenshotAtIndex(int32 index) const;
+
+private:
+			std::vector<ScreenshotInfoRef>
+								fScreenshotInfos;
+};
+
+
+typedef BReference<PackageScreenshotInfo> PackageScreenshotInfoRef;
+
+
+#endif // PACKAGE_SCREENSHOT_INFO_H

src/apps/haikudepot/server/LocalPkgDataLoadProcess.cpp

@@ -235,7 +235,8 @@ LocalPkgDataLoadProcess::RunInternal()
 				HDDEBUG("pkg [%s] repository [%s] not recognized --> ignored",
 					modelInfo->Name().String(), repositoryName.String());
 			} else {
-				(*it)->AddPackage(modelInfo);
+				DepotInfoRef depot = *it;
+				depot->AddPackage(modelInfo);
 				HDTRACE("pkg [%s] assigned to [%s]",
 					modelInfo->Name().String(), repositoryName.String());
 			}

src/apps/haikudepot/server/ServerPkgDataUpdateProcess.cpp

@@ -51,7 +51,7 @@ public:
 			uint32				Count();
 
 private:
-			int32				IndexOfPackageByName(const BString& name) const;
+	static	ScreenshotInfoRef	_CreateScreenshot(DumpExportPkgScreenshot* screenshot);
 
 private:
 			BString				fDepotName;
@@ -154,17 +154,14 @@ PackageFillingPkgListener::ConsumePackage(const PackageInfoRef& package,
 		packageClassificationInfo->SetIsNativeDesktop(pkg->IsNativeDesktop());
 
 	int32 countPkgScreenshots = pkg->CountPkgScreenshots();
+	PackageScreenshotInfoRef screenshotInfo(new PackageScreenshotInfo(), true);
 
 	for (i = 0; i < countPkgScreenshots; i++) {
 		DumpExportPkgScreenshot* screenshot = pkg->PkgScreenshotsItemAt(i);
-		package->AddScreenshotInfo(ScreenshotInfoRef(new ScreenshotInfo(
+		screenshotInfo->AddScreenshot(_CreateScreenshot(screenshot));
-			*(screenshot->Code()),
-			static_cast<int32>(screenshot->Width()),
-			static_cast<int32>(screenshot->Height()),
-			static_cast<int32>(screenshot->Length())
-		), true));
 	}
 
+	package->SetScreenshotInfo(screenshotInfo);
 	package->SetLocalizedText(localizedText);
 	package->SetLocalInfo(localInfo);
 
@@ -181,6 +178,16 @@ PackageFillingPkgListener::ConsumePackage(const PackageInfoRef& package,
 }
 
 
+/*static*/ ScreenshotInfoRef
+PackageFillingPkgListener::_CreateScreenshot(DumpExportPkgScreenshot* screenshot)
+{
+	return ScreenshotInfoRef(
+		new ScreenshotInfo(*(screenshot->Code()), static_cast<int32>(screenshot->Width()),
+			static_cast<int32>(screenshot->Height()), static_cast<int32>(screenshot->Length())),
+		true);
+}
+
+
 uint32
 PackageFillingPkgListener::Count()
 {

src/apps/haikudepot/ui/PackageInfoView.cpp

@@ -1352,8 +1352,7 @@ PackageInfoView::MessageReceived(BMessage* message)
 			if ((changes & PKG_CHANGED_LOCALIZED_TEXT) != 0
 				|| (changes & PKG_CHANGED_SCREENSHOTS) != 0
 				|| (changes & PKG_CHANGED_RATINGS) != 0
-				|| (changes & PKG_CHANGED_LOCAL_INFO) != 0
+				|| (changes & PKG_CHANGED_LOCAL_INFO) != 0) {
-				|| (changes & PKG_CHANGED_CHANGELOG) != 0) {
 				fPagesView->SetPackage(package, false);
 			}
 
@@ -1460,14 +1459,22 @@ PackageInfoView::_ScreenshotThumbCoordinate(const PackageInfoRef& package)
 {
 	if (!package.IsSet())
 		return ScreenshotCoordinate();
-	if (package->CountScreenshotInfos() == 0)
+
+	PackageScreenshotInfoRef screenshotInfo = package->ScreenshotInfo();
+
+	if (!screenshotInfo.IsSet() || screenshotInfo->Count() == 0)
+		return ScreenshotCoordinate();
+
+	ScreenshotInfoRef screenshot = screenshotInfo->ScreenshotAtIndex(0);
+
+	if (!screenshot.IsSet())
 		return ScreenshotCoordinate();
 
 	uint32 screenshotSizeScaled
 		= MAX(static_cast<uint32>(BControlLook::ComposeIconSize(kScreenshotSize).Width()),
 			MAX_IMAGE_SIZE);
 
-	return ScreenshotCoordinate(package->ScreenshotInfoAtIndex(0)->Code(), screenshotSizeScaled + 1,
+	return ScreenshotCoordinate(screenshot->Code(), screenshotSizeScaled + 1,
 		screenshotSizeScaled + 1);
 }
 

src/apps/haikudepot/ui/PackageListView.cpp

@@ -1000,10 +1000,6 @@ PackageListView::MessageReceived(BMessage* message)
 				}
 				if ((changes & PKG_CHANGED_ICON) != 0)
 					row->UpdateIconAndTitle();
-				if ((changes & PKG_CHANGED_DEPOT) != 0)
-					row->UpdateRepository();
-				if ((changes & PKG_CHANGED_VERSION) != 0)
-					row->UpdateVersion();
 				if ((changes & PKG_CHANGED_VERSION_CREATE_TIMESTAMP) != 0)
 					row->UpdateVersionCreateTimestamp();
 			}

src/apps/haikudepot/ui/ScreenshotWindow.cpp

@@ -258,12 +258,14 @@ ScreenshotWindow::_DownloadThread()
 	if (!fPackage.IsSet())
 		HDINFO("package not set");
 	else {
-		if (fPackage->CountScreenshotInfos() == 0)
+		PackageScreenshotInfoRef screenshotInfo = fPackage->ScreenshotInfo();
-    		HDINFO("package has no screenshots");
+
-    	else {
+		if (!screenshotInfo.IsSet() || screenshotInfo->Count() == 0) {
-    		int32 index = atomic_get(&fCurrentScreenshotIndex);
+			HDINFO("package has no screenshots");
-    		info = fPackage->ScreenshotInfoAtIndex(index);
+		} else {
-    	}
+			int32 index = atomic_get(&fCurrentScreenshotIndex);
+			info = screenshotInfo->ScreenshotAtIndex(index);
+		}
 	}
 
 	Unlock();
@@ -306,12 +308,18 @@ ScreenshotWindow::_MaxWidthAndHeightOfAllScreenshots()
 
 	// Find out dimensions of the largest screenshot of this package
 	if (fPackage.IsSet()) {
-		int count = fPackage->CountScreenshotInfos();
+		PackageScreenshotInfoRef screenshotInfo = fPackage->ScreenshotInfo();
+		int count = 0;
+
+		if (screenshotInfo.IsSet())
+			count = screenshotInfo->Count();
+
 		for(int32 i = 0; i < count; i++) {
-			const ScreenshotInfoRef& info = fPackage->ScreenshotInfoAtIndex(i);
+			const ScreenshotInfoRef& screenshot = screenshotInfo->ScreenshotAtIndex(i);
-			if (info.Get() != NULL) {
+
-				float w = (float) info->Width();
+			if (screenshot.IsSet()) {
-				float h = (float) info->Height();
+				float w = static_cast<float>(screenshot->Width());
+				float h = static_cast<float>(screenshot->Height());
 				if (w > size.Width())
 					size.SetWidth(w);
 				if (h > size.Height())
@@ -344,7 +352,14 @@ ScreenshotWindow::_ResizeToFitAndCenter()
 void
 ScreenshotWindow::_UpdateToolBar()
 {
-	const int32 numScreenshots = fPackage->CountScreenshotInfos();
+	int32 numScreenshots = 0;
+
+	if (fPackage.IsSet()) {
+		PackageScreenshotInfoRef screenshotInfo = fPackage->ScreenshotInfo();
+		if (screenshotInfo.IsSet())
+			numScreenshots = screenshotInfo->Count();
+	}
+
 	const int32 currentIndex = atomic_get(&fCurrentScreenshotIndex);
 
 	fToolBar->SetActionEnabled(MSG_PREVIOUS_SCREENSHOT,