Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make SqliteStorage independant of any calendar. #28

Closed
wants to merge 5 commits into from

Conversation

dcaliste
Copy link
Contributor

@dcaliste dcaliste commented Apr 1, 2022

@pvuorela, this is an attempt implementation of some ideas we exchanged on IRC about making the SQLite storage independant from any calendar. The overview is the following :

  • create a new StorageBackend class that takes the methods from ExtendedCalendar related to database handling but that does not require any calendar, just incidences and their notebook ids.
  • make SqliteStorage inherits of this class, so it becomes independant from any calendar (and in order to a possible associated calendar be moved to another thread).
  • make ExtendedStorage a link between the SQLite backend storage and an ExtendedCalendar, using only observer methods. To keep backward compatibility on it, it is inheriting SqliteStorage instead of CalStorage now. The previous behaviour should be kept.

From a positive point of view, tests are still passing. From a negative one, I had to break the API compatibility on ExtendedStorageObserver, since it has been moved to StorageBackend. As far as I know, it was only used in the QML binding (recent addition, you know) and it should be simple to update it.

Some technical notes :

  • the alarm support has been moved to StorageBackend, so it will always be available event if we evolve one day ExtendedStorage as a threaded link between calendar and backend.
  • the storage backend has no clue about notebook as in KCalendarCore::Calendar so it is always dealing with list of incidences sorted by notebook ids, as in QMultiHash<QString, Incidence::Ptr> containers. Such containers have been typedef to Collection for briefty.
  • ExtendedStorage is now a simple shell mirroring changes in the DB into an ExtendedCalendar and vice-versa.

I'm submitting this before implementing the threaded version to discuss with you if it's valuable or going in the right direction. If so, I'll create a ThreadedStorage class that will basically do the same than the current ExtendedStorage one but with the SQLite backend living in a different thread.

@dcaliste
Copy link
Contributor Author

I've rebased it on #29 because it has introduced a lot of changes in default calendar handling. When we agree on #29, I'll rebased this one once again.

This class is a subset of old ExtendedStorage one, with
method purely related to storage handling, without any
relation to a calendar.

Make SqliteStorage inherits of this new class.

Implement ExtendedStorage as a backend for a calendar,
using only observers. ExtendedStorage is necessarily a
SQLite one now.
Don't assume that incidences are shared pointer when
it's not necessary, like in internal database read/write
or alarm handling.
Storage backends are not using ::Ptr anymore, the
memory management is delegated to a Manager of
the storage backend. The manager can take ownership
of the new notebooks and new incidences.
@dcaliste
Copy link
Contributor Author

dcaliste commented Dec 1, 2022

This is now superceeded by #37.

@dcaliste dcaliste closed this Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant