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

Chore: split user data from feed data #1706

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@
* [markusj](mailto:markusj@users.noreply.github.com)
* [mnassabain](mailto:34754819+mnassabain@users.noreply.github.com)
* [mormegil](mailto:mormegil@centrum.cz)
* [nextcloud486153](mailto:78801830+nextcloud486153@users.noreply.github.com)
* [nexus-uw](mailto:you@example.com)
* [repat](mailto:repat@repat.de)
* [ritchiewilson](mailto:rawilson52@gmail.com)
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ The format is mostly based on [Keep a Changelog](https://keepachangelog.com/en/1
# Unreleased
## [18.x.x]
### Changed
- Split user data out of feed fields for de-duplication

### Fixed

Expand Down
20 changes: 11 additions & 9 deletions lib/Db/FeedMapperV2.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
class FeedMapperV2 extends NewsMapperV2
{
const TABLE_NAME = 'news_feeds';
const USER_TABLE_NAME = 'news_user_feeds';

/**
* FeedMapper constructor.
Expand All @@ -38,7 +39,7 @@ class FeedMapperV2 extends NewsMapperV2
*/
public function __construct(IDBConnection $db, Time $time)
{
parent::__construct($db, $time, Feed::class);
parent::__construct($db, $time, Feed::class, self::USER_TABLE_NAME);
}

/**
Expand All @@ -53,7 +54,7 @@ public function findAllFromUser(string $userId, array $params = []): array
{
$builder = $this->db->getQueryBuilder();
$builder->select('feeds.*', $builder->func()->count('items.id', 'unreadCount'))
->from(static::TABLE_NAME, 'feeds')
->from(static::USER_TABLE_NAME, 'feeds')
->leftJoin(
'feeds',
ItemMapperV2::TABLE_NAME,
Expand Down Expand Up @@ -84,7 +85,7 @@ public function findFromUser(string $userId, int $id): Entity
{
$builder = $this->db->getQueryBuilder();
$builder->select('*')
->from(static::TABLE_NAME)
->from(static::USER_TABLE_NAME)
->where('user_id = :user_id')
->andWhere('id = :id')
->setParameter('user_id', $userId)
Expand All @@ -102,7 +103,7 @@ public function findAll(): array
{
$builder = $this->db->getQueryBuilder();
$builder->select('*')
->from(static::TABLE_NAME)
->from(static::USER_TABLE_NAME)
->where('deleted_at = 0');

return $this->findEntities($builder);
Expand All @@ -123,7 +124,8 @@ public function findByURL(string $userId, string $url): Entity
{
$builder = $this->db->getQueryBuilder();
$builder->select('*')
->from(static::TABLE_NAME)
->from(static::TABLE_NAME, 'feeds')
->innerJoin('feeds', self::USER_TABLE_NAME, 'users', 'feeds.id = users.feed_id')
->where('user_id = :user_id')
->andWhere('url = :url')
->setParameter('user_id', $userId)
Expand All @@ -143,7 +145,7 @@ public function findAllFromFolder(?int $id): array
{
$builder = $this->db->getQueryBuilder();
$builder->select('*')
->from(static::TABLE_NAME);
->from(static::USER_TABLE_NAME);

if (is_null($id)) {
$builder->where('folder_id IS NULL');
Expand All @@ -168,8 +170,8 @@ public function read(string $userId, int $id, ?int $maxItemID = null): int
{
$idBuilder = $this->db->getQueryBuilder();
$idBuilder->select('items.id')
->from(ItemMapperV2::TABLE_NAME, 'items')
->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->from(ItemMapperV2::USER_TABLE_NAME, 'items')
->innerJoin('items', FeedMapperV2::USER_TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->andWhere('feeds.user_id = :userId')
->andWhere('feeds.id = :feedId')
->setParameter('userId', $userId)
Expand All @@ -188,7 +190,7 @@ function ($value): int {
);

$builder = $this->db->getQueryBuilder();
$builder->update(ItemMapperV2::TABLE_NAME)
$builder->update(ItemMapperV2::USER_TABLE_NAME)
->set('unread', $builder->createParameter('unread'))
->andWhere('id IN (:idList)')
->andWhere('unread != :unread')
Expand Down
6 changes: 3 additions & 3 deletions lib/Db/FolderMapperV2.php
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ public function read(string $userId, int $id, ?int $maxItemId = null): int
{
$idBuilder = $this->db->getQueryBuilder();
$idBuilder->select('items.id')
->from(ItemMapperV2::TABLE_NAME, 'items')
->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->from(ItemMapperV2::USER_TABLE_NAME, 'items')
->innerJoin('items', FeedMapperV2::USER_TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->andWhere('feeds.user_id = :userId')
->andWhere('feeds.folder_id = :folderId')
->setParameter('userId', $userId)
Expand All @@ -129,7 +129,7 @@ public function read(string $userId, int $id, ?int $maxItemId = null): int
}, $this->db->executeQuery($idBuilder->getSQL(), $idBuilder->getParameters())->fetchAll());

$builder = $this->db->getQueryBuilder();
$builder->update(ItemMapperV2::TABLE_NAME)
$builder->update(ItemMapperV2::USER_TABLE_NAME)
->set('unread', $builder->createParameter('unread'))
->andWhere('id IN (:idList)')
->andWhere('unread != :unread')
Expand Down
26 changes: 13 additions & 13 deletions lib/Db/ItemMapperV2.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
class ItemMapperV2 extends NewsMapperV2
{
const TABLE_NAME = 'news_items';
const USER_TABLE_NAME = 'news_user_items';

/**
* ItemMapper constructor.
Expand All @@ -40,7 +41,7 @@ class ItemMapperV2 extends NewsMapperV2
*/
public function __construct(IDBConnection $db, Time $time)
{
parent::__construct($db, $time, Item::class);
parent::__construct($db, $time, Item::class, self::USER_TABLE_NAME);
}

/**
Expand All @@ -56,9 +57,8 @@ public function findAllFromUser(string $userId, array $params = []): array
$builder = $this->db->getQueryBuilder();
$builder->select('items.*')
->from($this->tableName, 'items')
->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->where('feeds.user_id = :user_id')
->andWhere('feeds.deleted_at = 0')
->innerJoin('items', self::USER_TABLE_NAME, 'users', 'items.id = users.item_id')
->where('users.user_id = :user_id')
->setParameter('user_id', $userId, IQueryBuilder::PARAM_STR);

foreach ($params as $key => $value) {
Expand Down Expand Up @@ -92,10 +92,9 @@ public function findFromUser(string $userId, int $id): Entity
$builder = $this->db->getQueryBuilder();
$builder->select('items.*')
->from($this->tableName, 'items')
->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->where('feeds.user_id = :user_id')
->innerJoin('items', self::USER_TABLE_NAME, 'users', 'items.id = users.item_id')
->where('users.user_id = :user_id')
->andWhere('items.id = :item_id')
->andWhere('feeds.deleted_at = 0')
->setParameter('user_id', $userId, IQueryBuilder::PARAM_STR)
->setParameter('item_id', $id, IQueryBuilder::PARAM_INT);

Expand Down Expand Up @@ -444,9 +443,10 @@ public function findAllFeed(
): array {
$builder = $this->db->getQueryBuilder();

$builder->select('items.*')
$builder->select('items.*', 'users.unread', 'users.starred', 'users.shared_by')
->from($this->tableName, 'items')
->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->innerJoin('items', self::USER_TABLE_NAME, 'users', 'items.id = users.item_id')
->andWhere('feeds.deleted_at = 0')
->andWhere('feeds.user_id = :userId')
->andWhere('items.feed_id = :feedId')
Expand Down Expand Up @@ -501,9 +501,10 @@ public function findAllFolder(
$folderWhere = $builder->expr()->eq('feeds.folder_id', new Literal($folderId), IQueryBuilder::PARAM_INT);
}

$builder->select('items.*')
$builder->select('items.*', 'users.unread', 'users.starred', 'users.shared_by')
->from($this->tableName, 'items')
->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->innerJoin('items', self::USER_TABLE_NAME, 'users', 'items.id = users.item_id')
->andWhere('feeds.user_id = :userId')
->andWhere('feeds.deleted_at = 0')
->andWhere($folderWhere)
Expand Down Expand Up @@ -550,11 +551,10 @@ public function findAllItems(
): array {
$builder = $this->db->getQueryBuilder();

$builder->select('items.*')
$builder->select('items.*', 'users.unread', 'users.starred', 'users.shared_by')
->from($this->tableName, 'items')
->innerJoin('items', FeedMapperV2::TABLE_NAME, 'feeds', 'items.feed_id = feeds.id')
->andWhere('feeds.user_id = :userId')
->andWhere('feeds.deleted_at = 0')
->innerJoin('items', self::USER_TABLE_NAME, 'users', 'items.id = users.item_id')
->andWhere('users.user_id = :userId')
->setParameter('userId', $userId)
->addOrderBy('items.id', ($oldestFirst ? 'ASC' : 'DESC'));

Expand Down
5 changes: 3 additions & 2 deletions lib/Db/NewsMapperV2.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ abstract class NewsMapperV2 extends QBMapper
public function __construct(
IDBConnection $db,
Time $time,
string $entity
string $entity,
?string $table = null
) {
parent::__construct($db, static::TABLE_NAME, $entity);
parent::__construct($db, $table ?? static::TABLE_NAME, $entity);
$this->time = $time;
}

Expand Down
141 changes: 141 additions & 0 deletions lib/Migration/Version160100Date20210821130702.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
<?php

declare(strict_types=1);

namespace OCA\News\Migration;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

/**
* Auto-generated migration step: Please modify to your needs!
*/
class Version160100Date20210821130702 extends SimpleMigrationStep {

/**
* @var IDBConnection
*/
protected $connection;

public function __construct(IDBConnection $connection)
{
$this->connection = $connection;
}

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
*/
public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
}

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

if (!$schema->hasTable('news_user_items')) {
$table = $schema->createTable('news_user_items');
$table->addColumn('item_id', 'bigint', [
'notnull' => true,
'length' => 8,
'unsigned' => true,
]);
$table->addColumn('user_id', 'string', [
'notnull' => true,
'length' => 64,
]);
$table->addColumn('unread', 'boolean', [
'notnull' => true,
'default' => false,
]);
$table->addColumn('starred', 'boolean', [
'notnull' => true,
'default' => false,
]);
$table->addColumn('last_modified', 'bigint', [
'notnull' => false,
'length' => 8,
'default' => 0,
'unsigned' => true,
]);
$table->addColumn('shared_by', 'string', [
'notnull' => false,
'length' => 64
]);
$table->setPrimaryKey(['item_id', 'user_id']);
}

if (!$schema->hasTable('news_user_feeds')) {
$table = $schema->createTable('news_user_feeds');
$table->addColumn('feed_id', 'bigint', [
'notnull' => true,
'length' => 8,
'unsigned' => true,
]);
$table->addColumn('user_id', 'string', [
'notnull' => true,
'length' => 64,
]);
$table->addColumn('folder_id', 'bigint', [
'notnull' => false,
'length' => 8,
]);
$table->addColumn('deleted_at', 'bigint', [
'notnull' => false,
'length' => 8,
'default' => 0,
'unsigned' => true,
]);
$table->addColumn('added', 'bigint', [
'notnull' => false,
'length' => 8,
'default' => 0,
'unsigned' => true,
]);
$table->addColumn('title', 'text', [
'notnull' => true,
]);
$table->addColumn('last_modified', 'bigint', [
'notnull' => false,
'length' => 8,
'default' => 0,
'unsigned' => true,
]);
$table->setPrimaryKey(['feed_id', 'user_id']);
}

return $schema;
}

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
* @param array $options
*/
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
$qb = $this->connection->getQueryBuilder();
$user_item_table = $qb->getTableName('news_user_items');
$user_feed_table = $qb->getTableName('news_user_feeds');
$item_table = $qb->getTableName('news_items');
$feed_table = $qb->getTableName('news_feeds');

$items_query = "REPLACE INTO $user_item_table SELECT id AS 'item_id', ? AS 'user_id',`unread`,`starred`,`last_modified`,`shared_by` FROM $item_table where feed_id = ?;";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might have to find a different solution here, since PostgreSQL doesn't have REPLACE INTO statements.
Alternatively, you could check the database type and use another suitable statement.


$feeds = $this->connection->executeQuery("SELECT `id`,`user_id` FROM $feed_table;")->fetchAll();
foreach ($feeds as $feed) {
$this->connection->executeUpdate($items_query, [$feed['user_id'], $feed['id']]);
}

$this->connection->executeUpdate("REPLACE INTO $user_feed_table SELECT id AS 'feed_id',user_id,folder_id,deleted_at,added,title,last_modified FROM $feed_table;");
}
}
Loading