Просмотр исходного кода

Merge branch 'master' into imprv/duplicate-Page-with-child

白石誠 5 лет назад
Родитель
Сommit
522e438589

+ 57 - 0
.github/workflows/list-unhealthy-branches.yml

@@ -0,0 +1,57 @@
+name: List Unhealthy Branches
+
+on:
+  schedule:
+    - cron: '0 6 * * wed'
+
+jobs:
+  list:
+
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        fetch-depth: 0
+
+    - uses: actions/setup-node@v2-beta
+      with:
+        node-version: '14'
+
+    - name: List branches
+      id: list-branches
+      run: |
+        export SLACK_ATTACHMENTS_ILLEGAL=`node bin/github-actions/list-branches --illegal`
+        export SLACK_ATTACHMENTS_INACTIVE=`node bin/github-actions/list-branches --inactive`
+        echo ::set-output name=SLACK_ATTACHMENTS_ILLEGAL::$SLACK_ATTACHMENTS_ILLEGAL
+        echo ::set-output name=SLACK_ATTACHMENTS_INACTIVE::$SLACK_ATTACHMENTS_INACTIVE
+        echo ::set-output name=SLACK_ATTACHMENTS_LENGTH_ILLEGAL::$(echo $SLACK_ATTACHMENTS_ILLEGAL | jq '. | length')
+        echo ::set-output name=SLACK_ATTACHMENTS_LENGTH_INACTIVE::$(echo $SLACK_ATTACHMENTS_INACTIVE | jq '. | length')
+
+    - name: Slack Notification for illegal named branches
+      if: steps.list-branches.outputs.SLACK_ATTACHMENTS_LENGTH_ILLEGAL > 0
+      uses: sonots/slack-notice-action@v3
+      env:
+        SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}
+      with:
+        status: custom
+        payload: |
+          {
+            text: '<!channel> There is some *illegal named branches* on GitHub.',
+            channel: '#ci',
+            attachments: ${{ steps.list-branches.outputs.SLACK_ATTACHMENTS_ILLEGAL }}
+          }
+
+    - name: Slack Notification for inactive branches
+      if: steps.list-branches.outputs.SLACK_ATTACHMENTS_LENGTH_INACTIVE > 0
+      uses: sonots/slack-notice-action@v3
+      env:
+        SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}
+      with:
+        status: custom
+        payload: |
+          {
+            text: '<!channel> There is some *illegal named branches* on GitHub.',
+            channel: '#ci',
+            attachments: ${{ steps.list-branches.outputs.SLACK_ATTACHMENTS_INACTIVE }}
+          }

+ 2 - 0
CHANGES.md

@@ -3,6 +3,8 @@
 ## v4.1.3-RC
 
 * Feature: Create/edit linker with GUI
+* Improvement: Paging page histories
+* Fix: To be able to delete attachment metadata even when the actual data does not exist
 
 ## v4.1.2
 

+ 154 - 0
bin/github-actions/list-branches.js

@@ -0,0 +1,154 @@
+/* eslint-disable no-console */
+
+/*
+ * USAGE:
+ *  node list-branches [OPTION]
+ *
+ * OPTIONS:
+ *  --inactive : Return inactive branches (default)
+ *  --illegal : Return illegal named branches
+ */
+
+const { execSync } = require('child_process');
+const url = require('url');
+
+const EXCLUDE_TERM_DAYS = 14;
+const EXCLUDE_PATTERNS = [
+  /^feat\/custom-sidebar-2$/,
+  // https://regex101.com/r/Lnx7Pz/3
+  /^dev\/[\d.x]*$/,
+];
+const LEGAL_PATTERNS = [
+  /^master$/,
+  // https://regex101.com/r/p9xswM/4
+  /^(dev|feat|imprv|support|fix|rc|release|tmp)\/.+$/,
+];
+const GITHUB_REPOS_URI = 'https://github.com/weseek/growi/';
+
+class BranchSummary {
+
+  constructor(line) {
+    const splitted = line.split('\t'); // split with '%09'
+
+    this.authorDate = new Date(splitted[0].trim());
+    this.authorName = splitted[1].trim();
+    this.branchName = splitted[2].trim().replace(/^origin\//, '');
+    this.subject = splitted[3].trim();
+  }
+
+}
+
+function getExcludeTermDate() {
+  const date = new Date();
+  date.setDate(date.getDate() - EXCLUDE_TERM_DAYS);
+  return date;
+}
+
+function getBranchSummaries() {
+  // exec git for-each-ref
+  const out = execSync(`\
+    git for-each-ref refs/remotes \
+      --sort=-committerdate \
+      --format='%(authordate:iso) %09 %(authorname) %09 %(refname:short) %09 %(subject)'
+  `).toString();
+
+  // parse
+  const summaries = out
+    .split('\n')
+    .filter(v => v !== '') // trim empty string
+    .map(line => new BranchSummary(line))
+    .filter((summary) => { // exclude branches that matches to patterns
+      return !EXCLUDE_PATTERNS.some(pattern => pattern.test(summary.branchName));
+    });
+
+  return summaries;
+}
+
+function getGitHubCommitsUrl(branchName) {
+  return url.resolve(GITHUB_REPOS_URI, `commits/${branchName}`);
+}
+
+function getGitHubComparingLink(branchName) {
+  const label = `master &lt;- ${branchName}`;
+  const link = url.resolve(GITHUB_REPOS_URI, `compare/${branchName}`);
+  return `<${link}|${label}>`;
+}
+
+/**
+ * @see https://api.slack.com/messaging/composing/layouts#building-attachments
+ * @see https://github.com/marketplace/actions/slack-incoming-webhook
+ *
+ * @param {string} mode
+ * @param {BranchSummary} summaries
+ */
+function printSlackAttachments(mode, summaries) {
+  const color = (mode === 'illegal') ? 'warning' : '#999999';
+
+  const attachments = summaries.map((summary) => {
+    const {
+      authorName, authorDate, branchName, subject,
+    } = summary;
+
+    return {
+      color,
+      title: branchName,
+      title_link: getGitHubCommitsUrl(branchName),
+      fields: [
+        {
+          title: 'Author Date',
+          value: authorDate,
+          short: true,
+        },
+        {
+          title: 'Author',
+          value: authorName,
+          short: true,
+        },
+        {
+          title: 'Last Commit Subject',
+          value: subject,
+        },
+        {
+          title: 'Comparing Link',
+          value: getGitHubComparingLink(branchName),
+        },
+      ],
+    };
+  });
+
+  console.log(JSON.stringify(attachments));
+}
+
+async function main(mode) {
+  const summaries = getBranchSummaries();
+
+  let filteredSummaries;
+
+  switch (mode) {
+    case 'illegal':
+      filteredSummaries = summaries
+        .filter((summary) => { // exclude branches that matches to patterns
+          return !LEGAL_PATTERNS.some(pattern => pattern.test(summary.branchName));
+        });
+      break;
+    default: {
+      const excludeTermDate = getExcludeTermDate();
+      filteredSummaries = summaries
+        .filter((summary) => {
+          return summary.authorDate < excludeTermDate;
+        });
+      break;
+    }
+  }
+
+  printSlackAttachments(mode, filteredSummaries);
+}
+
+const args = process.argv.slice(2);
+
+let mode = 'inactive';
+if (args.length > 0 && args[0] === '--illegal') {
+  mode = 'illegal';
+}
+
+main(mode);

+ 6 - 2
src/client/js/app.jsx

@@ -32,6 +32,7 @@ import TableOfContents from './components/TableOfContents';
 import PersonalSettings from './components/Me/PersonalSettings';
 import NavigationContainer from './services/NavigationContainer';
 import PageContainer from './services/PageContainer';
+import PageHistoryContainer from './services/PageHistoryContainer';
 import CommentContainer from './services/CommentContainer';
 import EditorContainer from './services/EditorContainer';
 import TagContainer from './services/TagContainer';
@@ -51,12 +52,13 @@ const socketIoContainer = appContainer.getContainer('SocketIoContainer');
 // create unstated container instance
 const navigationContainer = new NavigationContainer(appContainer);
 const pageContainer = new PageContainer(appContainer);
+const pageHistoryContainer = new PageHistoryContainer(appContainer, pageContainer);
 const commentContainer = new CommentContainer(appContainer);
 const editorContainer = new EditorContainer(appContainer, defaultEditorOptions, defaultPreviewOptions);
 const tagContainer = new TagContainer(appContainer);
 const personalContainer = new PersonalContainer(appContainer);
 const injectableContainers = [
-  appContainer, socketIoContainer, navigationContainer, pageContainer, commentContainer, editorContainer, tagContainer, personalContainer,
+  appContainer, socketIoContainer, navigationContainer, pageContainer, pageHistoryContainer, commentContainer, editorContainer, tagContainer, personalContainer,
 ];
 
 logger.info('unstated containers have been initialized');
@@ -141,7 +143,9 @@ $('a[data-toggle="tab"][href="#revision-history"]').on('show.bs.tab', () => {
   ReactDOM.render(
     <I18nextProvider i18n={i18n}>
       <ErrorBoundary>
-        <PageHistory shareLinkId={pageContainer.state.shareLinkId} pageId={pageContainer.state.pageId} crowi={appContainer} />
+        <Provider inject={injectableContainers}>
+          <PageHistory />
+        </Provider>
       </ErrorBoundary>
     </I18nextProvider>, document.getElementById('revision-history'),
   );

+ 5 - 7
src/client/js/components/Page/RevisionLoader.jsx

@@ -40,22 +40,20 @@ class RevisionLoader extends React.Component {
       this.setState({ isLoading: true });
     }
 
-    const requestData = {
-      page_id: this.props.pageId,
-      revision_id: this.props.revisionId,
-    };
+    const { pageId, revisionId } = this.props;
+
 
     // load data with REST API
     try {
-      const res = await this.props.appContainer.apiGet('/revisions.get', requestData);
+      const res = await this.props.appContainer.apiv3Get(`/revisions/${revisionId}`, { pageId });
 
       this.setState({
-        markdown: res.revision.body,
+        markdown: res.data.revision.body,
         error: null,
       });
 
       if (this.props.onRevisionLoaded != null) {
-        this.props.onRevisionLoaded(res.revision);
+        this.props.onRevisionLoaded(res.data.revision);
       }
     }
     catch (error) {

+ 55 - 144
src/client/js/components/PageHistory.jsx

@@ -1,178 +1,89 @@
-import React from 'react';
+import React, { useCallback } from 'react';
 import PropTypes from 'prop-types';
 import loggerFactory from '@alias/logger';
 
-import { withTranslation } from 'react-i18next';
+import { withUnstatedContainers } from './UnstatedUtils';
+import { toastError } from '../util/apiNotification';
 
+import { withLoadingSppiner } from './SuspenseUtils';
 import PageRevisionList from './PageHistory/PageRevisionList';
 
-const logger = loggerFactory('growi:PageHistory');
-class PageHistory extends React.Component {
-
-  constructor(props) {
-    super(props);
+import PageHistroyContainer from '../services/PageHistoryContainer';
+import PaginationWrapper from './PaginationWrapper';
 
-    this.state = {
-      isLoaded: false,
-      isLoading: false,
-      errorMessage: null,
-      revisions: [],
-      diffOpened: {},
-    };
 
-    this.getPreviousRevision = this.getPreviousRevision.bind(this);
-    this.onDiffOpenClicked = this.onDiffOpenClicked.bind(this);
-  }
+const logger = loggerFactory('growi:PageHistory');
 
-  async componentWillMount() {
-    const pageId = this.props.pageId;
-    const shareLinkId = this.props.shareLinkId || null;
 
-    if (!pageId) {
-      return;
-    }
+function PageHistory(props) {
+  const { pageHistoryContainer } = props;
 
-    let res;
+  const handlePage = useCallback(async(selectedPage) => {
     try {
-      this.setState({ isLoading: true });
-      res = await this.props.crowi.apiGet('/revisions.ids', { page_id: pageId, share_link_id: shareLinkId });
+      await props.pageHistoryContainer.retrieveRevisions(selectedPage);
     }
     catch (err) {
+      toastError(err);
+      props.pageHistoryContainer.setState({ errorMessage: err.message });
       logger.error(err);
-      this.setState({ errorMessage: err });
-      return;
     }
-    finally {
-      this.setState({ isLoading: false });
-    }
-
-    const rev = res.revisions;
-    const diffOpened = {};
-    const lastId = rev.length - 1;
-    res.revisions.forEach((revision, i) => {
-      const user = revision.author;
-      if (user) {
-        rev[i].author = user;
-      }
-
-      if (i === 0 || i === lastId) {
-        diffOpened[revision._id] = true;
-      }
-      else {
-        diffOpened[revision._id] = false;
-      }
-    });
-
-    this.setState({
-      isLoaded: true,
-      revisions: rev,
-      diffOpened,
-    });
+  }, [props.pageHistoryContainer]);
 
-    // load 0, and last default
-    if (rev[0]) {
-      this.fetchPageRevisionBody(rev[0]);
-    }
-    if (rev[1]) {
-      this.fetchPageRevisionBody(rev[1]);
-    }
-    if (lastId !== 0 && lastId !== 1 && rev[lastId]) {
-      this.fetchPageRevisionBody(rev[lastId]);
-    }
+  if (pageHistoryContainer.state.errorMessage != null) {
+    return (
+      <div className="my-5">
+        <div className="text-danger">{pageHistoryContainer.state.errorMessage}</div>
+      </div>
+    );
   }
 
-  getPreviousRevision(currentRevision) {
-    let cursor = null;
-    for (const revision of this.state.revisions) {
-      // comparing ObjectId
-      // eslint-disable-next-line eqeqeq
-      if (cursor && cursor._id == currentRevision._id) {
-        cursor = revision;
-        break;
+  if (pageHistoryContainer.state.revisions === pageHistoryContainer.dummyRevisions) {
+    throw new Promise(async() => {
+      try {
+        await props.pageHistoryContainer.retrieveRevisions(1);
+      }
+      catch (err) {
+        toastError(err);
+        pageHistoryContainer.setState({ errorMessage: err.message });
+        logger.error(err);
       }
-
-      cursor = revision;
-    }
-
-    return cursor;
-  }
-
-  onDiffOpenClicked(revision) {
-    const diffOpened = this.state.diffOpened;
-    const revisionId = revision._id;
-
-    diffOpened[revisionId] = !(diffOpened[revisionId]);
-    this.setState({
-      diffOpened,
     });
-
-    this.fetchPageRevisionBody(revision);
-    this.fetchPageRevisionBody(this.getPreviousRevision(revision));
   }
 
-  fetchPageRevisionBody(revision) {
-    const shareLinkId = this.props.shareLinkId || null;
-
-    if (revision.body) {
-      return;
-    }
-
-    this.props.crowi.apiGet('/revisions.get',
-      { page_id: this.props.pageId, revision_id: revision._id, share_link_id: shareLinkId })
-      .then((res) => {
-        if (res.ok) {
-          this.setState({
-            revisions: this.state.revisions.map((rev) => {
-              // comparing ObjectId
-              // eslint-disable-next-line eqeqeq
-              if (rev._id == res.revision._id) {
-                return res.revision;
-              }
-
-              return rev;
-            }),
-          });
-        }
-      })
-      .catch((err) => {
-
-      });
-  }
 
-  render() {
+  function pager() {
     return (
-      <div className="mt-4">
-        { this.state.isLoading && (
-          <div className="my-5 text-center">
-            <i className="fa fa-lg fa-spinner fa-pulse mx-auto text-muted"></i>
-          </div>
-        ) }
-        { this.state.errorMessage && (
-          <div className="my-5">
-            <div className="text-danger">{this.state.errorMessage}</div>
-          </div>
-        ) }
-        { this.state.isLoaded && (
-          <PageRevisionList
-            t={this.props.t}
-            revisions={this.state.revisions}
-            diffOpened={this.state.diffOpened}
-            getPreviousRevision={this.getPreviousRevision}
-            onDiffOpenClicked={this.onDiffOpenClicked}
-          />
-        ) }
+      <div className="my-3">
+        <PaginationWrapper
+          activePage={pageHistoryContainer.state.activePage}
+          changePage={handlePage}
+          totalItemsCount={pageHistoryContainer.state.totalPages}
+          pagingLimit={pageHistoryContainer.state.pagingLimit}
+        />
       </div>
     );
   }
 
+
+  return (
+    <div className="mt-4">
+      {pager()}
+      <PageRevisionList
+        revisions={pageHistoryContainer.state.revisions}
+        diffOpened={pageHistoryContainer.state.diffOpened}
+        getPreviousRevision={pageHistoryContainer.getPreviousRevision}
+        onDiffOpenClicked={pageHistoryContainer.onDiffOpenClicked}
+      />
+      {pager()}
+    </div>
+  );
+
 }
 
-PageHistory.propTypes = {
-  t: PropTypes.func.isRequired, // i18next
+const RenderPageHistoryWrapper = withUnstatedContainers(withLoadingSppiner(PageHistory), [PageHistroyContainer]);
 
-  shareLinkId: PropTypes.string,
-  pageId: PropTypes.string,
-  crowi: PropTypes.object.isRequired,
+PageHistory.propTypes = {
+  pageHistoryContainer: PropTypes.instanceOf(PageHistroyContainer).isRequired,
 };
 
-export default withTranslation()(PageHistory);
+export default RenderPageHistoryWrapper;

+ 6 - 3
src/client/js/components/PageHistory/PageRevisionList.jsx

@@ -1,10 +1,12 @@
 import React from 'react';
 import PropTypes from 'prop-types';
 
+import { withTranslation } from 'react-i18next';
+
 import Revision from './Revision';
 import RevisionDiff from './RevisionDiff';
 
-export default class PageRevisionList extends React.Component {
+class PageRevisionList extends React.Component {
 
   constructor(props) {
     super(props);
@@ -65,8 +67,6 @@ export default class PageRevisionList extends React.Component {
     const { t } = this.props;
 
     const revisions = this.props.revisions;
-
-
     const revisionCount = this.props.revisions.length;
 
     let hasDiffPrev;
@@ -117,7 +117,10 @@ export default class PageRevisionList extends React.Component {
 
 PageRevisionList.propTypes = {
   t: PropTypes.func.isRequired, // i18next
+
   revisions: PropTypes.array,
   diffOpened: PropTypes.object,
   onDiffOpenClicked: PropTypes.func.isRequired,
 };
+
+export default withTranslation()(PageRevisionList);

+ 21 - 0
src/client/js/components/SuspenseUtils.jsx

@@ -0,0 +1,21 @@
+/* eslint-disable import/prefer-default-export */
+import React, { Suspense } from 'react';
+
+/**
+ * If you throw a Promise in the component, it will display a sppiner
+ * @param {object} Component A React.Component or functional component
+ */
+export function withLoadingSppiner(Component) {
+  return (props => (
+    // wrap with <Suspense></Suspense>
+    <Suspense
+      fallback={(
+        <div className="my-5 text-center">
+          <i className="fa fa-lg fa-spinner fa-pulse mx-auto text-muted"></i>
+        </div>
+      )}
+    >
+      <Component {...props} />
+    </Suspense>
+  ));
+}

+ 162 - 0
src/client/js/services/PageHistoryContainer.js

@@ -0,0 +1,162 @@
+import { Container } from 'unstated';
+
+import loggerFactory from '@alias/logger';
+
+import { toastError } from '../util/apiNotification';
+
+const logger = loggerFactory('growi:PageHistoryContainer');
+
+/**
+ * Service container for personal settings page (PageHistory.jsx)
+ * @extends {Container} unstated Container
+ */
+export default class PageHistoryContainer extends Container {
+
+  constructor(appContainer, pageContainer) {
+    super();
+
+    this.appContainer = appContainer;
+    this.pageContainer = pageContainer;
+
+    this.dummyRevisions = 0;
+
+    this.state = {
+      errorMessage: null,
+
+      // set dummy rivisions for using suspense
+      revisions: this.dummyRevisions,
+      diffOpened: {},
+
+      totalPages: 0,
+      activePage: 1,
+      pagingLimit: Infinity,
+    };
+
+    this.retrieveRevisions = this.retrieveRevisions.bind(this);
+    this.onDiffOpenClicked = this.onDiffOpenClicked.bind(this);
+    this.getPreviousRevision = this.getPreviousRevision.bind(this);
+    this.fetchPageRevisionBody = this.fetchPageRevisionBody.bind(this);
+  }
+
+  /**
+   * Workaround for the mangling in production build to break constructor.name
+   */
+  static getClassName() {
+    return 'PageHistoryContainer';
+  }
+
+  /**
+   * syncRevisions of selectedPage
+   * @param {number} selectedPage
+   */
+  async retrieveRevisions(selectedPage) {
+    const { pageId, shareLinkId } = this.pageContainer.state;
+    if (!pageId) {
+      return;
+    }
+
+    const res = await this.appContainer.apiv3Get('/revisions/list', { page_id: pageId, share_link_id: shareLinkId, selectedPage });
+    const rev = res.data.docs;
+
+    // set Pagination state
+    this.setState({
+      activePage: selectedPage,
+      totalPages: res.data.totalDocs,
+      pagingLimit: res.data.limit,
+    });
+
+    const diffOpened = {};
+    const lastId = rev.length - 1;
+
+    res.data.docs.forEach((revision, i) => {
+      const user = revision.author;
+      if (user) {
+        rev[i].author = user;
+      }
+
+      if (i === 0 || i === lastId) {
+        diffOpened[revision._id] = true;
+      }
+      else {
+        diffOpened[revision._id] = false;
+      }
+    });
+
+    this.setState({ revisions: rev });
+    this.setState({ diffOpened });
+
+    // load 0, and last default
+    if (rev[0]) {
+      this.fetchPageRevisionBody(rev[0]);
+    }
+    if (rev[1]) {
+      this.fetchPageRevisionBody(rev[1]);
+    }
+    if (lastId !== 0 && lastId !== 1 && rev[lastId]) {
+      this.fetchPageRevisionBody(rev[lastId]);
+    }
+
+    return;
+  }
+
+  onDiffOpenClicked(revision) {
+    const { diffOpened } = this.state;
+    const revisionId = revision._id;
+
+    diffOpened[revisionId] = !(diffOpened[revisionId]);
+    this.setState(diffOpened);
+
+    this.fetchPageRevisionBody(revision);
+    this.fetchPageRevisionBody(this.getPreviousRevision(revision));
+  }
+
+  getPreviousRevision(currentRevision) {
+    let cursor = null;
+    for (const revision of this.state.revisions) {
+      // comparing ObjectId
+      // eslint-disable-next-line eqeqeq
+      if (cursor && cursor._id == currentRevision._id) {
+        cursor = revision;
+        break;
+      }
+
+      cursor = revision;
+    }
+
+    return cursor;
+  }
+
+  /**
+   * fetch page revision body by revision in argument
+   * @param {object} revision
+   */
+  async fetchPageRevisionBody(revision) {
+    const { pageId, shareLinkId } = this.pageContainer.state;
+
+    if (revision.body) {
+      return;
+    }
+
+    try {
+      const res = await this.appContainer.apiv3Get(`/revisions/${revision._id}`, { page_id: pageId, share_link_id: shareLinkId });
+      this.setState({
+        revisions: this.state.revisions.map((rev) => {
+          // comparing ObjectId
+          // eslint-disable-next-line eqeqeq
+          if (rev._id == res.data.revision._id) {
+            return res.data.revision;
+          }
+
+          return rev;
+        }),
+      });
+    }
+    catch (err) {
+      toastError(err);
+      this.setState({ errorMessage: err.message });
+      logger.error(err);
+    }
+  }
+
+
+}

+ 12 - 0
src/server/crowi/index.js

@@ -106,6 +106,7 @@ Crowi.prototype.init = async function() {
     this.setupSlack(),
     this.setupCsrf(),
     this.setUpFileUpload(),
+    this.setupAttachmentService(),
     this.setUpAcl(),
     this.setUpCustomize(),
     this.setUpRestQiitaAPI(),
@@ -144,6 +145,7 @@ Crowi.prototype.initForTest = async function() {
     // this.setupSlack(),
     // this.setupCsrf(),
     // this.setUpFileUpload(),
+    // this.setupAttachmentService(),
     this.setUpAcl(),
     // this.setUpCustomize(),
     // this.setUpRestQiitaAPI(),
@@ -565,6 +567,16 @@ Crowi.prototype.setUpFileUpload = async function() {
   }
 };
 
+/**
+ * setup AttachmentService
+ */
+Crowi.prototype.setupAttachmentService = async function() {
+  const AttachmentService = require('../service/attachment');
+  if (this.attachmentService == null) {
+    this.attachmentService = new AttachmentService(this);
+  }
+};
+
 /**
  * setup RestQiitaAPIService
  */

+ 2 - 26
src/server/models/attachment.js

@@ -42,8 +42,7 @@ module.exports = function(crowi) {
   attachmentSchema.set('toJSON', { virtuals: true });
 
 
-  attachmentSchema.statics.create = async function(pageId, user, fileStream, originalName, fileFormat, fileSize) {
-    const fileUploader = require('../service/file-uploader')(crowi);
+  attachmentSchema.statics.createWithoutSave = function(pageId, user, fileStream, originalName, fileFormat, fileSize) {
     const Attachment = this;
 
     const extname = path.extname(originalName);
@@ -52,7 +51,7 @@ module.exports = function(crowi) {
       fileName = `${fileName}${extname}`;
     }
 
-    let attachment = new Attachment();
+    const attachment = new Attachment();
     attachment.page = pageId;
     attachment.creator = user._id;
     attachment.originalName = originalName;
@@ -61,31 +60,8 @@ module.exports = function(crowi) {
     attachment.fileSize = fileSize;
     attachment.createdAt = Date.now();
 
-    // upload file
-    await fileUploader.uploadFile(fileStream, attachment);
-    // save attachment
-    attachment = await attachment.save();
-
     return attachment;
   };
 
-  attachmentSchema.statics.removeAttachmentsByPageId = async function(pageId) {
-    const attachments = await this.find({ page: pageId });
-
-    const promises = attachments.map(async(attachment) => {
-      return this.removeWithSubstanceById(attachment._id);
-    });
-
-    return Promise.all(promises);
-  };
-
-  attachmentSchema.statics.removeWithSubstanceById = async function(id) {
-    const fileUploader = require('../service/file-uploader')(crowi);
-    // retrieve data from DB to get a completely populated instance
-    const attachment = await this.findById(id);
-    await fileUploader.deleteFile(attachment);
-    return await attachment.remove();
-  };
-
   return mongoose.model('Attachment', attachmentSchema);
 };

+ 8 - 20
src/server/models/page.js

@@ -1130,6 +1130,7 @@ module.exports = function(crowi) {
     }));
   };
 
+  // TODO: transplant to service/page.js because page deletion affects various models data
   pageSchema.statics.revertDeletedPage = async function(page, user, options = {}) {
     const newPath = this.getRevertDeletedPageName(page.path);
 
@@ -1173,31 +1174,16 @@ module.exports = function(crowi) {
   /**
    * This is danger.
    */
+  // TODO: transplant to service/page.js because page deletion affects various models data
   pageSchema.statics.completelyDeletePage = async function(pageData, user, options = {}) {
     validateCrowi();
 
-    // Delete Bookmarks, Attachments, Revisions, Pages and emit delete
-    const Bookmark = crowi.model('Bookmark');
-    const Attachment = crowi.model('Attachment');
-    const Comment = crowi.model('Comment');
-    const PageTagRelation = crowi.model('PageTagRelation');
-    const ShareLink = crowi.model('ShareLink');
-    const Revision = crowi.model('Revision');
-    const pageId = pageData._id;
+    const { _id, path } = pageData;
     const socketClientId = options.socketClientId || null;
 
-    debug('Completely delete', pageData.path);
-
-    await Promise.all([
-      Bookmark.removeBookmarksByPageId(pageId),
-      Attachment.removeAttachmentsByPageId(pageId),
-      Comment.removeCommentsByPageId(pageId),
-      PageTagRelation.remove({ relatedPage: pageId }),
-      ShareLink.remove({ relatedPage: pageId }),
-      Revision.removeRevisionsByPath(pageData.path),
-      this.findByIdAndRemove(pageId),
-      this.removeRedirectOriginPageByPath(pageData.path),
-    ]);
+    logger.debug('Deleting completely', path);
+
+    await crowi.pageService.deleteCompletely(_id, path);
 
     if (socketClientId != null) {
       pageEvent.emit('delete', pageData, user, socketClientId); // update as renamed page
@@ -1208,6 +1194,7 @@ module.exports = function(crowi) {
   /**
    * Delete Bookmarks, Attachments, Revisions, Pages and emit delete
    */
+  // TODO: transplant to service/page.js because page deletion affects various models data
   pageSchema.statics.completelyDeletePageRecursively = async function(targetPage, user, options = {}) {
     const findOpts = { includeTrashed: true };
 
@@ -1304,6 +1291,7 @@ module.exports = function(crowi) {
     return targetPage;
   };
 
+  // TODO: transplant to service/page.js because page deletion affects various models data
   pageSchema.statics.handlePrivatePagesForDeletedGroup = async function(deletedGroup, action, transferToUserGroupId) {
     const Page = mongoose.model('Page');
 

+ 2 - 0
src/server/models/revision.js

@@ -6,6 +6,7 @@ module.exports = function(crowi) {
   const logger = require('@alias/logger')('growi:models:revision');
 
   const mongoose = require('mongoose');
+  const mongoosePaginate = require('mongoose-paginate-v2');
 
   const ObjectId = mongoose.Schema.Types.ObjectId;
   const revisionSchema = new mongoose.Schema({
@@ -24,6 +25,7 @@ module.exports = function(crowi) {
     createdAt: { type: Date, default: Date.now },
     hasDiffToPrev: { type: Boolean },
   });
+  revisionSchema.plugin(mongoosePaginate);
 
   /*
    * preparation for https://github.com/weseek/growi/issues/216

+ 4 - 2
src/server/models/user.js

@@ -207,21 +207,23 @@ module.exports = function(crowi) {
     return userData;
   };
 
+  // TODO: create UserService and transplant this method because image uploading depends on AttachmentService
   userSchema.methods.updateImage = async function(attachment) {
     this.imageAttachment = attachment;
     await this.updateImageUrlCached();
     return this.save();
   };
 
+  // TODO: create UserService and transplant this method because image deletion depends on AttachmentService
   userSchema.methods.deleteImage = async function() {
     validateCrowi();
-    const Attachment = crowi.model('Attachment');
 
     // the 'image' field became DEPRECATED in v3.3.8
     this.image = undefined;
 
     if (this.imageAttachment != null) {
-      Attachment.removeWithSubstanceById(this.imageAttachment._id);
+      const { attachmentService } = crowi;
+      attachmentService.removeAttachment(this.imageAttachment._id);
     }
 
     this.imageAttachment = undefined;

+ 2 - 0
src/server/routes/apiv3/index.js

@@ -39,6 +39,8 @@ module.exports = (crowi) => {
 
   router.use('/page', require('./page')(crowi));
   router.use('/pages', require('./pages')(crowi));
+  router.use('/revisions', require('./revisions')(crowi));
+
   router.use('/share-links', require('./share-links')(crowi));
 
   router.use('/bookmarks', require('./bookmarks')(crowi));

+ 145 - 0
src/server/routes/apiv3/revisions.js

@@ -0,0 +1,145 @@
+const loggerFactory = require('@alias/logger');
+
+const logger = loggerFactory('growi:routes:apiv3:pages');
+
+const express = require('express');
+
+const { query, param } = require('express-validator/check');
+const ErrorV3 = require('../../models/vo/error-apiv3');
+
+const router = express.Router();
+
+const PAGE_ITEMS = 30;
+
+/**
+ * @swagger
+ *  tags:
+ *    name: Revisions
+ */
+module.exports = (crowi) => {
+  const certifySharedPage = require('../../middlewares/certify-shared-page')(crowi);
+  const accessTokenParser = require('../../middlewares/access-token-parser')(crowi);
+  const loginRequired = require('../../middlewares/login-required')(crowi, true);
+  const apiV3FormValidator = require('../../middlewares/apiv3-form-validator')(crowi);
+
+  const {
+    Revision,
+    Page,
+    User,
+  } = crowi.models;
+
+  const validator = {
+    retrieveRevisions: [
+      query('page_id').isMongoId().withMessage('pageId is required'),
+      query('selectedPage').isInt({ min: 0 }).withMessage('selectedPage must be int'),
+    ],
+    retrieveRevisionById: [
+      query('page_id').isMongoId().withMessage('pageId is required'),
+      param('id').isMongoId().withMessage('id is required'),
+    ],
+  };
+
+  /**
+   * @swagger
+   *
+   *    /revisions/list:
+   *      get:
+   *        tags: [Revisions]
+   *        description: Get revisions by page id
+   *        parameters:
+   *          - in: query
+   *            name: pageId
+   *            schema:
+   *              type: string
+   *              description:  page id
+   *        responses:
+   *          200:
+   *            description: Return revisions belong to page
+   *
+   */
+  router.get('/list', certifySharedPage, accessTokenParser, loginRequired, validator.retrieveRevisions, apiV3FormValidator, async(req, res) => {
+    const pageId = req.query.page_id;
+    const { isSharedPage } = req;
+
+    const selectedPage = parseInt(req.query.selectedPage) || 1;
+
+    // check whether accessible
+    if (!isSharedPage && !(await Page.isAccessiblePageByViewer(pageId, req.user))) {
+      return res.apiv3Err(new ErrorV3('Current user is not accessible to this page.', 'forbidden-page'), 403);
+    }
+
+    try {
+      const page = await Page.findOne({ _id: pageId });
+
+      const paginateResult = await Revision.paginate(
+        { path: page.path },
+        {
+          page: selectedPage,
+          limit: PAGE_ITEMS,
+          sort: { createdAt: -1 },
+          populate: {
+            path: 'author',
+            select: User.USER_PUBLIC_FIELDS,
+          },
+        },
+      );
+
+      return res.apiv3(paginateResult);
+    }
+    catch (err) {
+      const msg = 'Error occurred in getting revisions by poge id';
+      logger.error('Error', err);
+      return res.apiv3Err(new ErrorV3(msg, 'faild-to-find-revisions'), 500);
+    }
+
+  });
+
+  /**
+   * @swagger
+   *
+   *    /revisions/{id}:
+   *      get:
+   *        tags: [Revisions]
+   *        description: Get one revision by id
+   *        parameters:
+   *          - in: query
+   *            name: pageId
+   *            required: true
+   *            description: page id
+   *            schema:
+   *              type: string
+   *          - in: path
+   *            name: id
+   *            required: true
+   *            description: revision id
+   *            schema:
+   *              type: string
+   *        responses:
+   *          200:
+   *            description: Return revision
+   *
+   */
+  router.get('/:id', certifySharedPage, accessTokenParser, loginRequired, validator.retrieveRevisionById, apiV3FormValidator, async(req, res) => {
+    const revisionId = req.params.id;
+    const pageId = req.query.page_id;
+    const { isSharedPage } = req;
+
+    // check whether accessible
+    if (!isSharedPage && !(await Page.isAccessiblePageByViewer(pageId, req.user))) {
+      return res.apiv3Err(new ErrorV3('Current user is not accessible to this page.', 'forbidden-page'), 403);
+    }
+
+    try {
+      const revision = await Revision.findById(revisionId).populate('author', User.USER_PUBLIC_FIELDS);
+      return res.apiv3({ revision });
+    }
+    catch (err) {
+      const msg = 'Error occurred in getting revision data by id';
+      logger.error('Error', err);
+      return res.apiv3Err(new ErrorV3(msg, 'faild-to-find-revision'), 500);
+    }
+
+  });
+
+  return router;
+};

+ 14 - 31
src/server/routes/attachment.js

@@ -3,8 +3,6 @@
 
 const logger = require('@alias/logger')('growi:routes:attachment');
 
-const fs = require('fs');
-
 const ApiResponse = require('../util/apiResponse');
 
 /**
@@ -131,7 +129,7 @@ module.exports = function(crowi, app) {
   const Attachment = crowi.model('Attachment');
   const User = crowi.model('User');
   const Page = crowi.model('Page');
-  const fileUploader = require('../service/file-uploader')(crowi, app);
+  const { fileUploadService, attachmentService } = crowi;
 
 
   /**
@@ -193,13 +191,13 @@ module.exports = function(crowi, app) {
       return res.sendStatus(304);
     }
 
-    if (fileUploader.canRespond()) {
-      return fileUploader.respond(res, attachment);
+    if (fileUploadService.canRespond()) {
+      return fileUploadService.respond(res, attachment);
     }
 
     let fileStream;
     try {
-      fileStream = await fileUploader.findDeliveryFile(attachment);
+      fileStream = await fileUploadService.findDeliveryFile(attachment);
     }
     catch (e) {
       logger.error(e);
@@ -234,32 +232,17 @@ module.exports = function(crowi, app) {
     }
   }
 
-  async function createAttachment(file, user, pageId = null) {
-    // check limit
-    const res = await fileUploader.checkLimit(file.size);
-    if (!res.isUploadable) {
-      throw new Error(res.errorMessage);
-    }
+  async function removeAttachment(attachmentId) {
+    const { fileUploadService } = crowi;
 
-    const fileStream = fs.createReadStream(file.path, {
-      flags: 'r', encoding: null, fd: null, mode: '0666', autoClose: true,
-    });
+    // retrieve data from DB to get a completely populated instance
+    const attachment = await Attachment.findById(attachmentId);
 
-    // create an Attachment document and upload file
-    let attachment;
-    try {
-      attachment = await Attachment.create(pageId, user, fileStream, file.originalname, file.mimetype, file.size);
-    }
-    catch (err) {
-      // delete temporary file
-      fs.unlink(file.path, (err) => { if (err) { logger.error('Error while deleting tmp file.') } });
-      throw err;
-    }
+    await fileUploadService.deleteFile(attachment);
 
-    return attachment;
+    return attachment.remove();
   }
 
-
   const actions = {};
   const api = {};
 
@@ -409,7 +392,7 @@ module.exports = function(crowi, app) {
    */
   api.limit = async function(req, res) {
     const fileSize = Number(req.query.fileSize);
-    return res.json(ApiResponse.success(await fileUploader.checkLimit(fileSize)));
+    return res.json(ApiResponse.success(await fileUploadService.checkLimit(fileSize)));
   };
 
   /**
@@ -522,7 +505,7 @@ module.exports = function(crowi, app) {
 
     let attachment;
     try {
-      attachment = await createAttachment(file, req.user, pageId);
+      attachment = await attachmentService.createAttachment(file, req.user, pageId);
     }
     catch (err) {
       logger.error(err);
@@ -618,7 +601,7 @@ module.exports = function(crowi, app) {
     let attachment;
     try {
       req.user.deleteImage();
-      attachment = await createAttachment(file, req.user);
+      attachment = await attachmentService.createAttachment(file, req.user);
       await req.user.updateImage(attachment);
     }
     catch (err) {
@@ -687,7 +670,7 @@ module.exports = function(crowi, app) {
     }
 
     try {
-      await Attachment.removeWithSubstanceById(id);
+      await removeAttachment(attachment);
     }
     catch (err) {
       logger.error(err);

+ 0 - 5
src/server/routes/index.js

@@ -10,7 +10,6 @@ module.exports = function(crowi, app) {
   const loginRequiredStrictly = require('../middlewares/login-required')(crowi);
   const loginRequired = require('../middlewares/login-required')(crowi, true);
   const adminRequired = require('../middlewares/admin-required')(crowi);
-  const certifySharedPage = require('../middlewares/certify-shared-page')(crowi);
   const certifySharedFile = require('../middlewares/certify-shared-file')(crowi);
   const csrf = require('../middlewares/csrf')(crowi);
 
@@ -26,7 +25,6 @@ module.exports = function(crowi, app) {
   const attachment = require('./attachment')(crowi, app);
   const comment = require('./comment')(crowi, app);
   const tag = require('./tag')(crowi, app);
-  const revision = require('./revision')(crowi, app);
   const search = require('./search')(crowi, app);
   const hackmd = require('./hackmd')(crowi, app);
 
@@ -164,9 +162,6 @@ module.exports = function(crowi, app) {
   app.post('/_api/attachments.removeProfileImage'   , accessTokenParser , loginRequiredStrictly , csrf, attachment.api.removeProfileImage);
   app.get('/_api/attachments.limit'   , accessTokenParser , loginRequiredStrictly, attachment.api.limit);
 
-  app.get('/_api/revisions.get'       , certifySharedPage , accessTokenParser , loginRequired , revision.api.get);
-  app.get('/_api/revisions.ids'       , certifySharedPage , accessTokenParser , loginRequired , revision.api.ids);
-
   app.get('/trash$'                   , loginRequired , page.trashPageShowWrapper);
   app.get('/trash/$'                  , loginRequired , page.trashPageListShowWrapper);
   app.get('/trash/*/$'                , loginRequired , page.deletedPageListShowWrapper);

+ 0 - 190
src/server/routes/revision.js

@@ -1,190 +0,0 @@
-/**
- * @swagger
- *  tags:
- *    name: Revisions
- */
-
-/**
- * @swagger
- *
- *  components:
- *    schemas:
- *      Revision:
- *        description: Revision
- *        type: object
- *        properties:
- *          _id:
- *            type: string
- *            description: revision ID
- *            example: 5e0734e472560e001761fa68
- *          __v:
- *            type: number
- *            description: DB record version
- *            example: 0
- *          author:
- *            $ref: '#/components/schemas/User/properties/_id'
- *          body:
- *            type: string
- *            description: content body
- *            example: |
- *              # test
- *
- *              test
- *          format:
- *            type: string
- *            description: format
- *            example: markdown
- *          path:
- *            type: string
- *            description: path
- *            example: /user/alice/test
- *          createdAt:
- *            type: string
- *            description: date created at
- *            example: 2010-01-01T00:00:00.000Z
- */
-
-module.exports = function(crowi, app) {
-  const logger = require('@alias/logger')('growi:routes:revision');
-  const Page = crowi.model('Page');
-  const Revision = crowi.model('Revision');
-  const User = crowi.model('User');
-  const ApiResponse = require('../util/apiResponse');
-
-  const actions = {};
-  actions.api = {};
-
-  /**
-   * @swagger
-   *
-   *    /revisions.get:
-   *      get:
-   *        tags: [Revisions, CrowiCompatibles]
-   *        operationId: revisions.get
-   *        summary: /revisions.get
-   *        description: Get revision
-   *        parameters:
-   *          - in: query
-   *            name: page_id
-   *            schema:
-   *              $ref: '#/components/schemas/Page/properties/_id'
-   *            required: true
-   *          - in: query
-   *            name: revision_id
-   *            schema:
-   *              $ref: '#/components/schemas/Revision/properties/_id'
-   *            required: true
-   *        responses:
-   *          200:
-   *            description: Succeeded to get revision.
-   *            content:
-   *              application/json:
-   *                schema:
-   *                  properties:
-   *                    ok:
-   *                      $ref: '#/components/schemas/V1Response/properties/ok'
-   *                    revision:
-   *                      $ref: '#/components/schemas/Revision'
-   *          403:
-   *            $ref: '#/components/responses/403'
-   *          500:
-   *            $ref: '#/components/responses/500'
-   */
-  /**
-   * @api {get} /revisions.get Get revision
-   * @apiName GetRevision
-   * @apiGroup Revision
-   *
-   * @apiParam {String} page_id Page Id.
-   * @apiParam {String} revision_id Revision Id.
-   */
-  actions.api.get = async function(req, res) {
-    const pageId = req.query.page_id;
-    const revisionId = req.query.revision_id;
-    const { isSharedPage } = req;
-
-    if (!pageId || !revisionId) {
-      return res.json(ApiResponse.error('Parameter page_id and revision_id are required.'));
-    }
-
-    // check whether accessible
-    if (!isSharedPage && !(await Page.isAccessiblePageByViewer(pageId, req.user))) {
-      return res.json(ApiResponse.error('Current user is not accessible to this page.'));
-    }
-
-    try {
-      const revision = await Revision.findById(revisionId).populate('author', User.USER_PUBLIC_FIELDS);
-      return res.json(ApiResponse.success({ revision }));
-    }
-    catch (err) {
-      logger.error('Error revisios.get', err);
-      return res.json(ApiResponse.error(err));
-    }
-  };
-
-  /**
-   * @swagger
-   *
-   *    /revisions.ids:
-   *      get:
-   *        tags: [Revisions, CrowiCompatibles]
-   *        operationId: revisions.ids
-   *        summary: /revisions.ids
-   *        description: Get revision id list of the page
-   *        parameters:
-   *          - in: query
-   *            name: page_id
-   *            schema:
-   *              $ref: '#/components/schemas/Page/properties/_id'
-   *            required: true
-   *        responses:
-   *          200:
-   *            description: Succeeded to get revision id list of the page.
-   *            content:
-   *              application/json:
-   *                schema:
-   *                  properties:
-   *                    ok:
-   *                      $ref: '#/components/schemas/V1Response/properties/ok'
-   *                    revisions:
-   *                      type: array
-   *                      items:
-   *                        $ref: '#/components/schemas/Revision'
-   *          403:
-   *            $ref: '#/components/responses/403'
-   *          500:
-   *            $ref: '#/components/responses/500'
-   */
-  /**
-   * @api {get} /revisions.ids Get revision id list of the page
-   * @apiName ids
-   * @apiGroup Revision
-   *
-   * @apiParam {String} page_id      Page Id.
-   */
-  actions.api.ids = async function(req, res) {
-    const pageId = req.query.page_id;
-    const { isSharedPage } = req;
-
-    if (pageId == null) {
-      return res.json(ApiResponse.error('Parameter page_id is required.'));
-    }
-
-    // check whether accessible
-    if (!isSharedPage && !(await Page.isAccessiblePageByViewer(pageId, req.user))) {
-      return res.json(ApiResponse.error('Current user is not accessible to this page.'));
-    }
-
-    try {
-      const page = await Page.findOne({ _id: pageId });
-      const revisions = await Revision.findRevisionIdList(page.path);
-      return res.json(ApiResponse.success({ revisions }));
-    }
-    catch (err) {
-      logger.error('Error revisios.ids', err);
-      return res.json(ApiResponse.error(err));
-    }
-  };
-
-  return actions;
-};

+ 58 - 0
src/server/service/attachment.js

@@ -0,0 +1,58 @@
+const logger = require('@alias/logger')('growi:service:AttachmentService'); // eslint-disable-line no-unused-vars
+
+const fs = require('fs');
+
+
+/**
+ * the service class for Attachment and file-uploader
+ */
+class AttachmentService {
+
+  constructor(crowi) {
+    this.crowi = crowi;
+  }
+
+  async createAttachment(file, user, pageId = null) {
+    const { fileUploadService } = this.crowi;
+
+    // check limit
+    const res = await fileUploadService.checkLimit(file.size);
+    if (!res.isUploadable) {
+      throw new Error(res.errorMessage);
+    }
+
+    const Attachment = this.crowi.model('Attachment');
+
+    const fileStream = fs.createReadStream(file.path, {
+      flags: 'r', encoding: null, fd: null, mode: '0666', autoClose: true,
+    });
+
+    // create an Attachment document and upload file
+    let attachment;
+    try {
+      attachment = Attachment.createWithoutSave(pageId, user, fileStream, file.originalname, file.mimetype, file.size);
+      await fileUploadService.uploadFile(fileStream, attachment);
+      await attachment.save();
+    }
+    catch (err) {
+      // delete temporary file
+      fs.unlink(file.path, (err) => { if (err) { logger.error('Error while deleting tmp file.') } });
+      throw err;
+    }
+
+    return attachment;
+  }
+
+  async removeAttachment(attachmentId) {
+    const Attachment = this.crowi.model('Attachment');
+    const { fileUploadService } = this.crowi;
+
+    const attachment = await Attachment.findById(attachmentId);
+    await fileUploadService.deleteFile(attachment);
+
+    return attachment.remove();
+  }
+
+}
+
+module.exports = AttachmentService;

+ 34 - 5
src/server/service/file-uploader/aws.js

@@ -49,6 +49,23 @@ module.exports = function(crowi) {
     return filePath;
   }
 
+  async function isFileExists(s3, params) {
+    // check file exists
+    try {
+      await s3.headObject(params).promise();
+    }
+    catch (err) {
+      if (err != null && err.code === 'NotFound') {
+        return false;
+      }
+
+      // error except for 'NotFound
+      throw err;
+    }
+
+    return true;
+  }
+
   lib.isValidUploadSettings = function() {
     return this.configManager.getConfig('crowi', 'aws:accessKeyId') != null
       && this.configManager.getConfig('crowi', 'aws:secretAccessKey') != null
@@ -74,7 +91,12 @@ module.exports = function(crowi) {
       Key: filePath,
     };
 
-    // TODO: ensure not to throw error even when the file does not exist
+    // check file exists
+    const isExists = await isFileExists(s3, params);
+    if (!isExists) {
+      logger.warn(`Any object that relate to the Attachment (${filePath}) does not exist in AWS S3`);
+      return;
+    }
 
     return s3.deleteObject(params).promise();
   };
@@ -108,12 +130,19 @@ module.exports = function(crowi) {
     const awsConfig = getAwsConfig();
     const filePath = getFilePathOnStorage(attachment);
 
+    const params = {
+      Bucket: awsConfig.bucket,
+      Key: filePath,
+    };
+
+    // check file exists
+    const isExists = await isFileExists(s3, params);
+    if (!isExists) {
+      throw new Error(`Any object that relate to the Attachment (${filePath}) does not exist in AWS S3`);
+    }
+
     let stream;
     try {
-      const params = {
-        Bucket: awsConfig.bucket,
-        Key: filePath,
-      };
       stream = s3.getObject(params).createReadStream();
     }
     catch (err) {

+ 26 - 3
src/server/service/file-uploader/gcs.js

@@ -38,6 +38,16 @@ module.exports = function(crowi) {
     return filePath;
   }
 
+  /**
+   * check file existence
+   * @param {File} file https://googleapis.dev/nodejs/storage/latest/File.html
+   */
+  async function isFileExists(file) {
+    // check file exists
+    const res = await file.exists();
+    return res[0];
+  }
+
   lib.isValidUploadSettings = function() {
     return this.configManager.getConfig('crowi', 'gcs:apiKeyJsonPath') != null
       && this.configManager.getConfig('crowi', 'gcs:bucket') != null;
@@ -51,10 +61,16 @@ module.exports = function(crowi) {
   lib.deleteFileByFilePath = async function(filePath) {
     const gcs = getGcsInstance(this.getIsUploadable());
     const myBucket = gcs.bucket(getGcsBucket());
+    const file = myBucket.file(filePath);
 
-    // TODO: ensure not to throw error even when the file does not exist
+    // check file exists
+    const isExists = await isFileExists(file);
+    if (!isExists) {
+      logger.warn(`Any object that relate to the Attachment (${filePath}) does not exist in GCS`);
+      return;
+    }
 
-    return myBucket.file(filePath).delete();
+    return file.delete();
   };
 
   lib.uploadFile = function(fileStream, attachment) {
@@ -80,10 +96,17 @@ module.exports = function(crowi) {
     const gcs = getGcsInstance(this.getIsUploadable());
     const myBucket = gcs.bucket(getGcsBucket());
     const filePath = getFilePathOnStorage(attachment);
+    const file = myBucket.file(filePath);
+
+    // check file exists
+    const isExists = await isFileExists(file);
+    if (!isExists) {
+      throw new Error(`Any object that relate to the Attachment (${filePath}) does not exist in GCS`);
+    }
 
     let stream;
     try {
-      stream = myBucket.file(filePath).createReadStream();
+      stream = file.createReadStream();
     }
     catch (err) {
       logger.error(err);

+ 1 - 0
src/server/service/file-uploader/local.js

@@ -42,6 +42,7 @@ module.exports = function(crowi) {
     }
     catch (err) {
       logger.warn(`Any AttachmentFile which path is '${filePath}' does not exist in local fs`);
+      return;
     }
 
     return fs.unlinkSync(filePath);

+ 35 - 0
src/server/service/page.js

@@ -24,6 +24,41 @@ class PageService {
     return returnObj;
   }
 
+  async deleteCompletely(pageId, pagePath) {
+    // Delete Bookmarks, Attachments, Revisions, Pages and emit delete
+    const Bookmark = this.crowi.model('Bookmark');
+    const Comment = this.crowi.model('Comment');
+    const Page = this.crowi.model('Page');
+    const PageTagRelation = this.crowi.model('PageTagRelation');
+    const ShareLink = this.crowi.model('ShareLink');
+    const Revision = this.crowi.model('Revision');
+
+    return Promise.all([
+      Bookmark.removeBookmarksByPageId(pageId),
+      Comment.removeCommentsByPageId(pageId),
+      PageTagRelation.remove({ relatedPage: pageId }),
+      ShareLink.remove({ relatedPage: pageId }),
+      Revision.removeRevisionsByPath(pagePath),
+      Page.findByIdAndRemove(pageId),
+      Page.removeRedirectOriginPageByPath(pagePath),
+      this.removeAllAttachments(pageId),
+    ]);
+  }
+
+  async removeAllAttachments(pageId) {
+    const Attachment = this.crowi.model('Attachment');
+    const { attachmentService } = this.crowi;
+
+    const attachments = await Attachment.find({ page: pageId });
+
+    const promises = attachments.map(async(attachment) => {
+      return attachmentService.removeAttachment(attachment._id);
+    });
+
+    return Promise.all(promises);
+  }
+
+
 }
 
 module.exports = PageService;

+ 0 - 2
src/server/util/swigFunctions.js

@@ -12,7 +12,6 @@ module.exports = function(crowi, req, locals) {
     passportService,
     appService,
     aclService,
-    fileUploadService,
     customizeService,
   } = crowi;
   debug('initializing swigFunctions');
@@ -69,7 +68,6 @@ module.exports = function(crowi, req, locals) {
    */
   locals.appService = appService;
   locals.aclService = aclService;
-  locals.fileUploadService = fileUploadService;
   locals.customizeService = customizeService;
   locals.passportService = passportService;
   locals.pathUtils = pathUtils;