Merge pull request #2126 from weseek/imprv/update-cache-bulk

Imprv/update cache bulk

src/client/js/app.jsx

@@ -28,7 +28,8 @@ import PageStatusAlert from './components/PageStatusAlert';
 import PagePathAutoComplete from './components/PagePathAutoComplete';
 import RecentCreated from './components/RecentCreated/RecentCreated';
 import MyDraftList from './components/MyDraftList/MyDraftList';
-import UserPictureList from './components/User/UserPictureList';
+import SeenUserPictureList from './components/User/SeenUserPictureList';
+import LikerPictureList from './components/User/LikerPictureList';
 import TableOfContents from './components/TableOfContents';
 
 import PersonalSettings from './components/Me/PersonalSettings';
@@ -93,8 +94,8 @@ if (pageContainer.state.pageId != null) {
     'page-management': <PageManagement />,
 
     'revision-toc': <TableOfContents />,
-    'seen-user-list': <UserPictureList userIds={pageContainer.state.seenUserIds} />,
-    'liker-list': <UserPictureList userIds={pageContainer.state.likerUserIds} />,
+    'seen-user-list': <SeenUserPictureList />,
+    'liker-list': <LikerPictureList />,
     'rename-page-name-input': <PagePathAutoComplete crowi={appContainer} initializedPath={pageContainer.state.path} />,
 
     'user-created-list': <RecentCreated />,

src/client/js/components/User/LikerPictureList.jsx

@@ -0,0 +1,32 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+
+import UserPictureList from './UserPictureList';
+
+import { createSubscribedElement } from '../UnstatedUtils';
+
+import PageContainer from '../../services/PageContainer';
+
+class LikerPictureList extends React.Component {
+
+  render() {
+    const { pageContainer } = this.props;
+    return (
+      <UserPictureList users={pageContainer.state.likerUsers} />
+    );
+  }
+
+}
+
+LikerPictureList.propTypes = {
+  pageContainer: PropTypes.instanceOf(PageContainer).isRequired,
+};
+
+/**
+ * Wrapper component for using unstated
+ */
+const LikerPictureListWrapper = (props) => {
+  return createSubscribedElement(LikerPictureList, props, [PageContainer]);
+};
+
+export default (LikerPictureListWrapper);

src/client/js/components/User/SeenUserPictureList.jsx

@@ -0,0 +1,32 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+
+import UserPictureList from './UserPictureList';
+
+import { createSubscribedElement } from '../UnstatedUtils';
+
+import PageContainer from '../../services/PageContainer';
+
+class SeenUserPictureList extends React.Component {
+
+  render() {
+    const { pageContainer } = this.props;
+    return (
+      <UserPictureList users={pageContainer.state.seenUsers} />
+    );
+  }
+
+}
+
+SeenUserPictureList.propTypes = {
+  pageContainer: PropTypes.instanceOf(PageContainer).isRequired,
+};
+
+/**
+ * Wrapper component for using unstated
+ */
+const SeenUserPictureListWrapper = (props) => {
+  return createSubscribedElement(SeenUserPictureList, props, [PageContainer]);
+};
+
+export default (SeenUserPictureListWrapper);

src/client/js/components/User/UserPictureList.jsx

@@ -1,31 +1,12 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 
-import { createSubscribedElement } from '../UnstatedUtils';
-import AppContainer from '../../services/AppContainer';
-
 import UserPicture from './UserPicture';
 
-class UserPictureList extends React.Component {
-
-  constructor(props) {
-    super(props);
-
-    const userIds = this.props.userIds;
-
-    const users = this.props.users.concat(
-      // FIXME: user data cache
-      this.props.appContainer.findUserByIds(userIds),
-    );
-
-    this.state = {
-      users,
-    };
-
-  }
+export default class UserPictureList extends React.Component {
 
   render() {
-    return this.state.users.map(user => (
+    return this.props.users.map(user => (
       <span key={user._id}>
         <UserPicture user={user} size="xs" />
       </span>
@@ -34,23 +15,10 @@ class UserPictureList extends React.Component {
 
 }
 
-/**
- * Wrapper component for using unstated
- */
-const UserPictureListWrapper = (props) => {
-  return createSubscribedElement(UserPictureList, props, [AppContainer]);
-};
-
 UserPictureList.propTypes = {
-  appContainer: PropTypes.instanceOf(AppContainer).isRequired,
-
-  userIds: PropTypes.arrayOf(PropTypes.string),
   users: PropTypes.arrayOf(PropTypes.object),
 };
 
 UserPictureList.defaultProps = {
-  userIds: [],
   users: [],
 };
-
-export default UserPictureListWrapper;

src/client/js/services/CommentContainer.js

@@ -63,6 +63,7 @@ export default class CommentContainer extends Container {
    * Load data of comments and store them in state
    */
   retrieveComments() {
+    // [TODO][GW - 1942] add method for updating imageUrlCached
     const { pageId } = this.getPageContainer().state;
 
     // get data (desc order array)

src/client/js/services/PageContainer.js

@@ -43,8 +43,8 @@ export default class PageContainer extends Container {
       path,
       tocHtml: '',
       isLiked: JSON.parse(mainContent.getAttribute('data-page-is-liked')),
-      seenUserIds: [],
-      likerUserIds: [],
+      seenUsers: [],
+      likerUsers: [],
       createdAt: mainContent.getAttribute('data-page-created-at'),
       creator: JSON.parse(mainContent.getAttribute('data-page-creator')),
       updatedAt: mainContent.getAttribute('data-page-updated-at'),
@@ -126,19 +126,49 @@ export default class PageContainer extends Container {
     this.state.markdown = markdown;
   }
 
-  initStateOthers() {
+  async initStateOthers() {
 
     const seenUserListElem = document.getElementById('seen-user-list');
     if (seenUserListElem != null) {
       const userIdsStr = seenUserListElem.dataset.userIds;
-      this.state.seenUserIds = userIdsStr.split(',');
+      if (userIdsStr === '') {
+        return;
+      }
+
+      const { users } = await this.appContainer.apiGet('/users.list', { user_ids: userIdsStr });
+      this.setState({ seenUsers: users });
+
+      await this.updateImageUrlCached(users);
     }
 
 
     const likerListElem = document.getElementById('liker-list');
     if (likerListElem != null) {
       const userIdsStr = likerListElem.dataset.userIds;
-      this.state.likerUserIds = userIdsStr.split(',');
+      if (userIdsStr === '') {
+        return;
+      }
+
+      const { users } = await this.appContainer.apiGet('/users.list', { user_ids: userIdsStr });
+      this.setState({ likerUsers: users });
+
+      await this.updateImageUrlCached(users);
+    }
+  }
+
+  async updateImageUrlCached(users) {
+    const noImageCacheUsers = users.filter((user) => { return user.imageUrlCached == null });
+    if (noImageCacheUsers.length === 0) {
+      return;
+    }
+
+    const noImageCacheUserIds = noImageCacheUsers.map((user) => { return user.id });
+    try {
+      await this.appContainer.apiv3Put('/users/update.imageUrlCache', { userIds: noImageCacheUserIds });
+    }
+    catch (err) {
+      // Error alert doesn't apear, because user don't need to notice this error.
+      logger.error(err);
     }
   }
 

src/server/models/user.js

@@ -189,7 +189,7 @@ module.exports = function(crowi) {
 
   userSchema.methods.updateIsGravatarEnabled = async function(isGravatarEnabled) {
     this.isGravatarEnabled = isGravatarEnabled;
-    this.imageUrlCached = this.generateImageUrlCached();
+    await this.updateImageUrlCached();
     const userData = await this.save();
     return userData;
   };
@@ -225,7 +225,7 @@ module.exports = function(crowi) {
 
   userSchema.methods.updateImage = async function(attachment) {
     this.imageAttachment = attachment;
-    this.imageUrlCached = this.generateImageUrlCached();
+    await this.updateImageUrlCached();
     return this.save();
   };
 
@@ -241,21 +241,27 @@ module.exports = function(crowi) {
     }
 
     this.imageAttachment = undefined;
-    this.imageUrlCached = this.generateImageUrlCached();
+    this.updateImageUrlCached();
     return this.save();
   };
 
-  userSchema.methods.generateImageUrlCached = function() {
+  userSchema.methods.updateImageUrlCached = async function() {
+    this.imageUrlCached = await this.generateImageUrlCached();
+  };
+
+  userSchema.methods.generateImageUrlCached = async function() {
     if (this.isGravatarEnabled) {
       const email = this.email || '';
       const hash = md5(email.trim().toLowerCase());
       return `https://gravatar.com/avatar/${hash}`;
     }
-    if (this.image) {
+    if (this.image != null) {
       return this.image;
     }
-    if (this.imageAttachment) {
-      return this.imageAttachment.filePathProxied;
+    if (this.imageAttachment != null && this.imageAttachment._id != null) {
+      const Attachment = crowi.model('Attachment');
+      const imageAttachment = await Attachment.findById(this.imageAttachment);
+      return imageAttachment.filePathProxied;
     }
     return '/images/icons/user.svg';
   };

src/server/routes/apiv3/users.js

@@ -544,5 +544,58 @@ module.exports = (crowi) => {
       return res.apiv3Err(new ErrorV3(msg + err.message, 'extenral-account-delete-failed'));
     }
   });
+
+  /**
+   * @swagger
+   *
+   *  paths:
+   *    /users/update.imageUrlCache:
+   *      put:
+   *        tags: [Users]
+   *        operationId: update.imageUrlCache
+   *        summary: /users/update.imageUrlCache
+   *        description: update imageUrlCache
+   *        parameters:
+   *          - name:  userIds
+   *            in: query
+   *            description: user id list
+   *            schema:
+   *              type: string
+   *        responses:
+   *          200:
+   *            description: success creating imageUrlCached
+   *            content:
+   *              application/json:
+   *                schema:
+   *                  properties:
+   *                    userData:
+   *                      type: object
+   *                      description: users updated with imageUrlCached
+   */
+  router.put('/update.imageUrlCache', loginRequiredStrictly, adminRequired, csrf, async(req, res) => {
+    try {
+      const userIds = req.body.userIds;
+      const users = await User.find({ _id: { $in: userIds } });
+      const requests = await Promise.all(users.map(async(user) => {
+        return {
+          updateOne: {
+            filter: { _id: user._id },
+            update: { $set: { imageUrlCached: await user.generateImageUrlCached() } },
+          },
+        };
+      }));
+
+      if (requests.length > 0) {
+        await User.bulkWrite(requests);
+      }
+
+      return res.apiv3({});
+    }
+    catch (err) {
+      logger.error('Error', err);
+      return res.apiv3Err(new ErrorV3(err));
+    }
+  });
+
   return router;
 };

src/server/service/passport.js

@@ -854,6 +854,10 @@ class PassportService {
         if (user == null) {
           throw new Error('user not found');
         }
+        if (user.imageUrlCached == null) {
+          await user.updateImageUrlCached();
+          await user.save();
+        }
         done(null, user);
       }
       catch (err) {

src/server/views/widget/user_page_header.html

@@ -4,7 +4,6 @@
     <h4 id="revision-path"></h4>
 
     <div class="users-info d-flex align-items-center">
-    <!-- [TODO][GW-1942] add method for updating imageUrlCached -->
       <img src="{{ pageUser.imageUrlCached }}" class="picture img-circle">
       <div class="users-meta" style="flex: 1;">
         <div class="d-flex align-items-center">