[MediaWiki-commits] [Gerrit] Switch to BasicObjectMapper for TopicListEntry - change (mediawiki...Flow)

2016-02-17 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged.

Change subject: Switch to BasicObjectMapper for TopicListEntry
..


Switch to BasicObjectMapper for TopicListEntry

Before, depending on what was loaded and ordering, either a full
(with last_updated) or regular (just what's in flow_topic_list)
version of TopicListEntry would be cached by CachingObjectMapper.

That meant that moving would fail on some boards due to topic
moderation.

The default ordering is now last updated, and moves
trigger the ordering issue and force a re-render that requires
an offset to avoid moderated topics.

I looked at an alternative:

Add a separate mapper for TopicListLastUpdatedStorage (even just
any separate mapper would work, but I was going to make the updated
one uses a tri-key PK including last updated).

This was somewhat feasible, but still turned out more invasive than
seemed justified (e.g. we would have needed a separate
ObjectManager just for this reason, and then a way to get that out
of ManagerGroup, and conditionally pass it to Pager...).

I doubt there are any actual significant performance issues with this
(fromStorageRow and toStorageRow are both simple for TopicListEntry).

At some point, we could consider changing how CachingObjectMapper
works to avoid this issue in general, or revisiting whether we need
this caching layer.

Bug: T126701
Change-Id: I43872e87b0cf181e1a351bb1e258645413d07fbb
(cherry picked from commit b563a6e2bfbc89a7a6df3442e63a9adb1d1cc062)
---
M container.php
1 file changed, 3 insertions(+), 1 deletion(-)

Approvals:
  Catrope: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/container.php b/container.php
index ed7be41..70070ee 100644
--- a/container.php
+++ b/container.php
@@ -505,7 +505,9 @@
);
 };
 $c['storage.topic_list.mapper'] = function( $c ) {
-   return CachingObjectMapper::model(
+   // Must be BasicObjectMapper, due to variance in when
+   // we have workflow_last_update_timestamp
+   return BasicObjectMapper::model(
$c['storage.topic_list.class'],
$c['storage.topic_list.primary_key']
);

-- 
To view, visit https://gerrit.wikimedia.org/r/271327
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I43872e87b0cf181e1a351bb1e258645413d07fbb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: wmf/1.27.0-wmf.13
Gerrit-Owner: Mattflaschen 
Gerrit-Reviewer: Catrope 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] Switch to BasicObjectMapper for TopicListEntry - change (mediawiki...Flow)

2016-02-17 Thread Mattflaschen (Code Review) 
Mattflaschen has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/271327

Change subject: Switch to BasicObjectMapper for TopicListEntry
..

Switch to BasicObjectMapper for TopicListEntry

Before, depending on what was loaded and ordering, either a full
(with last_updated) or regular (just what's in flow_topic_list)
version of TopicListEntry would be cached by CachingObjectMapper.

That meant that moving would fail on some boards due to topic
moderation.

The default ordering is now last updated, and moves
trigger the ordering issue and force a re-render that requires
an offset to avoid moderated topics.

I looked at an alternative:

Add a separate mapper for TopicListLastUpdatedStorage (even just
any separate mapper would work, but I was going to make the updated
one uses a tri-key PK including last updated).

This was somewhat feasible, but still turned out more invasive than
seemed justified (e.g. we would have needed a separate
ObjectManager just for this reason, and then a way to get that out
of ManagerGroup, and conditionally pass it to Pager...).

I doubt there are any actual significant performance issues with this
(fromStorageRow and toStorageRow are both simple for TopicListEntry).

At some point, we could consider changing how CachingObjectMapper
works to avoid this issue in general, or revisiting whether we need
this caching layer.

Bug: T126701
Change-Id: I43872e87b0cf181e1a351bb1e258645413d07fbb
(cherry picked from commit b563a6e2bfbc89a7a6df3442e63a9adb1d1cc062)
---
M container.php
1 file changed, 3 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow 
refs/changes/27/271327/1

diff --git a/container.php b/container.php
index ed7be41..70070ee 100644
--- a/container.php
+++ b/container.php
@@ -505,7 +505,9 @@
);
 };
 $c['storage.topic_list.mapper'] = function( $c ) {
-   return CachingObjectMapper::model(
+   // Must be BasicObjectMapper, due to variance in when
+   // we have workflow_last_update_timestamp
+   return BasicObjectMapper::model(
$c['storage.topic_list.class'],
$c['storage.topic_list.primary_key']
);

-- 
To view, visit https://gerrit.wikimedia.org/r/271327
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I43872e87b0cf181e1a351bb1e258645413d07fbb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: wmf/1.27.0-wmf.13
Gerrit-Owner: Mattflaschen 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] Switch to BasicObjectMapper for TopicListEntry - change (mediawiki...Flow)

2016-02-17 Thread jenkins-bot (Code Review) 
jenkins-bot has submitted this change and it was merged.

Change subject: Switch to BasicObjectMapper for TopicListEntry
..


Switch to BasicObjectMapper for TopicListEntry

Before, depending on what was loaded and ordering, either a full
(with last_updated) or regular (just what's in flow_topic_list)
version of TopicListEntry would be cached by CachingObjectMapper.

That meant that moving would fail on some boards due to topic
moderation.

The default ordering is now last updated, and moves
trigger the ordering issue and force a re-render that requires
an offset to avoid moderated topics.

I looked at an alternative:

Add a separate mapper for TopicListLastUpdatedStorage (even just
any separate mapper would work, but I was going to make the updated
one uses a tri-key PK including last updated).

This was somewhat feasible, but still turned out more invasive than
seemed justified (e.g. we would have needed a separate
ObjectManager just for this reason, and then a way to get that out
of ManagerGroup, and conditionally pass it to Pager...).

I doubt there are any actual significant performance issues with this
(fromStorageRow and toStorageRow are both simple for TopicListEntry).

At some point, we could consider changing how CachingObjectMapper
works to avoid this issue in general, or revisiting whether we need
this caching layer.

Bug: T126701
Change-Id: I43872e87b0cf181e1a351bb1e258645413d07fbb
---
M container.php
1 file changed, 3 insertions(+), 1 deletion(-)

Approvals:
  Mattflaschen: Looks good to me, approved
  Matthias Mullie: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/container.php b/container.php
index ed7be41..70070ee 100644
--- a/container.php
+++ b/container.php
@@ -505,7 +505,9 @@
);
 };
 $c['storage.topic_list.mapper'] = function( $c ) {
-   return CachingObjectMapper::model(
+   // Must be BasicObjectMapper, due to variance in when
+   // we have workflow_last_update_timestamp
+   return BasicObjectMapper::model(
$c['storage.topic_list.class'],
$c['storage.topic_list.primary_key']
);

-- 
To view, visit https://gerrit.wikimedia.org/r/271190
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I43872e87b0cf181e1a351bb1e258645413d07fbb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen 
Gerrit-Reviewer: Mattflaschen 
Gerrit-Reviewer: Matthias Mullie 
Gerrit-Reviewer: jenkins-bot <>

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

[MediaWiki-commits] [Gerrit] Switch to BasicObjectMapper for TopicListEntry - change (mediawiki...Flow)

2016-02-16 Thread Mattflaschen (Code Review) 
Mattflaschen has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/271190

Change subject: Switch to BasicObjectMapper for TopicListEntry
..

Switch to BasicObjectMapper for TopicListEntry

Before, depending on what was loaded and ordering, either a full
(with last_updated) or regular (just what's in flow_topic_list)
version of TopicListEntry would be cached by CachingObjectMapper.

That meant that moving would fail on some boards due to topic
moderation.

The default ordering is now last updated, and moves
trigger the ordering issue and force a re-render that requires
an offset to avoid moderated topics.

I looked at an alternative:

Add a separate mapper for TopicListLastUpdatedStorage (even just
any separate mapper would work, but I was going to make the updated
one uses a tri-key PK including last updated).

This was somewhat feasible, but still turned out more invasive than
seemed justified (e.g. we would have needed a separate
ObjectManager just for this reason, and then a way to get that out
of ManagerGroup, and conditionally pass it to Pager...).

I doubt there are any actual significant performance issues with this
(fromStorageRow and toStorageRow are both simple for TopicListEntry).

At some point, we could consider changing how CachingObjectMapper
works to avoid this issue in general, or revisiting whether we need
this caching layer.

Bug: T126701
Change-Id: I43872e87b0cf181e1a351bb1e258645413d07fbb
---
M container.php
1 file changed, 3 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow 
refs/changes/90/271190/1

diff --git a/container.php b/container.php
index ed7be41..70070ee 100644
--- a/container.php
+++ b/container.php
@@ -505,7 +505,9 @@
);
 };
 $c['storage.topic_list.mapper'] = function( $c ) {
-   return CachingObjectMapper::model(
+   // Must be BasicObjectMapper, due to variance in when
+   // we have workflow_last_update_timestamp
+   return BasicObjectMapper::model(
$c['storage.topic_list.class'],
$c['storage.topic_list.primary_key']
);

-- 
To view, visit https://gerrit.wikimedia.org/r/271190
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I43872e87b0cf181e1a351bb1e258645413d07fbb
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: master
Gerrit-Owner: Mattflaschen 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits