feat: first implement #2

villins · 2017-02-15T08:41:18Z

close eggjs/egg#366

villins · 2017-02-15T08:46:37Z

@jtyjty99999

jtyjty99999 · 2017-02-15T08:49:37Z

@atian25 @popomore 也来看下？

atian25

travis-ci is not running. @jtyjty99999

atian25 · 2017-02-15T09:03:30Z

README.zh_CN.md

+
+## 使用场景
+
+- Why and What: 描述为什么会有这个插件，它主要在完成一件什么事情。


should modify or delete

atian25 · 2017-02-15T09:03:52Z

README.zh_CN.md

+0.x | ❌
+
+### 依赖的插件
+<!--


if not use, delete it

atian25 · 2017-02-15T09:04:12Z

config/config.default.js

@@ -0,0 +1,6 @@
+'use strict';
+
+exports.mongoose = {


missing jsdoc

atian25 · 2017-02-15T09:05:53Z

lib/mongoose.js

+  assert(config.url, '[egg-mongoose] url is required on config');
+  app.coreLogger.info('[egg-mongoose] connecting %s', config.url);
+
+  const db = mongoose.createConnection(config.url);


will it disconnect ? any retry strategy?

atian25 · 2017-02-15T09:06:30Z

package.json

+  "keywords": [
+    "egg",
+    "eggPlugin",
+    "egg-plugin"


add egg-model, mongoose

atian25 · 2017-02-15T09:06:46Z

package.json

@@ -0,0 +1,69 @@
+{
+  "name": "egg-mongoose",
+  "version": "1.0.0",


0.0.1 or 0.1.0

atian25 · 2017-02-15T09:07:21Z

package.json

+  },
+  "devDependencies": {
+    "autod": "^2.7.1",
+    "egg": "^0.7.0",


should run npm run autod to update deps

atian25 · 2017-02-15T09:07:42Z

package.json

+  "homepage": "https://github.com/eggjs/egg-mongoose#readme",
+  "author": "Villins",
+  "license": "MIT",
+  "boilerplate": {


remove this

atian25

我配下 travis

atian25 · 2017-02-15T10:56:44Z

lib/mongoose.js

+    db.open(config.url, config.options);
+  }
+
+  connect()


lint 没跑?

看跑 npm test 的提示有跑的

villins · 2017-02-15T11:02:46Z

@atian25 我的测试代码有依赖本地 mongodb service，跑不了 ci...

jtyjty99999 · 2017-02-15T11:03:16Z

@villins 参考下我给你发的文章

villins · 2017-02-15T11:11:46Z

@jtyjty99999 好的，test 的 mongodb service 有点难 mock

dead-horse · 2017-02-15T11:10:31Z

lib/mongoose.js

+  loadModel(app);
+
+  app.beforeStart(function* () {
+    app.coreLogger.info('[egg-mongoose] start success');


在 beforeStart 里面必须确保链接已经建立

dead-horse · 2017-02-15T11:10:39Z

lib/mongoose.js

+    db.open(config.url, config.options);
+  }
+
+  connect()


dead-horse · 2017-02-15T11:11:41Z

lib/mongoose.js

+  connect()
+
+  db.on('error', (err) => {
+    app.coreLogger.info('[egg-mongoose] %s error: ', err.message);


要打印 error 对象，否则堆栈会丢失。

db.on('error', err => { err.message = `[egg-mongoose]${err.message}`; app.coreLogger.info(err); });

atian25 · 2017-02-15T11:12:31Z

参考 mysql 插件，可以配置 travis service。然后干掉 egg-ci 发自我的 iPhone

…

在 2017年2月15日，19:02，清玩 ***@***.***> 写道： @atian25 我的测试代码有依赖本地 mongodb service，跑不了 ci... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

popomore · 2017-02-15T11:16:45Z

lib/mongoose.js

+  app.mongoose = db;
+
+  function connect() {
+    db.open(config.url, config.options);


open 会返回一个 promise，用 beforeStart 吧

app.beforeStart(function* () { yield db.open(config.url, config.options); });

popomore · 2017-02-15T11:17:50Z

travis 能开启 mongodb 么

jtyjty99999 · 2017-02-15T11:18:43Z

支持好多 @popomore https://docs.travis-ci.com/user/database-setup/

codecov · 2017-02-15T12:42:09Z

Codecov Report

❗ No coverage uploaded for pull request base (master@31b4a38). Click here to learn what that means.
The diff coverage is 88.88%.

@@            Coverage Diff            @@
##             master       #2   +/-   ##
=========================================
  Coverage          ?   88.88%           
=========================================
  Files             ?        5           
  Lines             ?       36           
  Branches          ?        0           
=========================================
  Hits              ?       32           
  Misses            ?        4           
  Partials          ?        0

Impacted Files	Coverage Δ
config/config.default.js	`100% <100%> (ø)`
agent.js	`100% <100%> (ø)`
app/extend/context.js	`100% <100%> (ø)`
app.js	`100% <100%> (ø)`
lib/mongoose.js	`86.2% <86.2%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31b4a38...fb1b0f7. Read the comment docs.

atian25 · 2017-02-15T12:48:06Z

package.json

+    "autod": "^2.7.1",
+    "egg": "^0.12.0",
+    "egg-bin": "^2.1.0",
+    "egg-ci": "^1.1.0",


配置 travis 的 service 的话, 要干掉 egg-ci

atian25 · 2017-02-15T12:49:25Z

有 mongodb service 的测试, 需要配置 appveyor 么? 如果不需要, 要干掉 appveyor.yml 不?

cc @jtyjty99999 @fengmk2

popomore · 2017-02-15T12:56:50Z

感觉可以干掉

atian25 · 2017-02-15T12:59:20Z

lib/mongoose.js

+
+  loadModel(app);
+
+  app.beforeStart(function* () {


直接 function 返回 promise 就行了吧

app.beforeStart(function() { app.coreLogger.info('[egg-mongoose] starting...'); return db.open(config.url, config.options); });

还是不要了，连接成功后应该打个日志

popomore · 2017-02-15T12:59:54Z

README.md

+// app/controller/user.js
+exports.index = function* () {
+  this.body = yield this.model.user.find({});


加个用例测下文件名大写

popomore · 2017-02-15T13:01:05Z

lib/mongoose.js

+
+  loadModel(app);
+
+  app.beforeStart(function* () {


还是不要了，连接成功后应该打个日志

popomore · 2017-02-15T13:01:50Z

lib/mongoose.js

+  });
+
+  db.on('disconnected', () => {
+    app.coreLogger.info('[egg-mongoose] %s disconnected', config.url);


这里打了warn？

popomore · 2017-02-15T13:02:19Z

lib/mongoose.js

+  }
+
+  db.on('error', err => {
+    app.coreLogger.info('[egg-mongoose] %s error: ', err.message);


atian25

其他 OK，辛苦了

atian25 · 2017-02-15T14:28:39Z

test/mongoose.test.js

+      .expect(200);
+
+    const res = yield request(app.callback())
+    .get('/books');


缩进或者不用换行

atian25 · 2017-02-15T14:29:18Z

test/mongoose.test.js

+      .send({ name: 'mongoose' })
+      .expect(200);
+
+    const res = yield request(app.callback())


atian25 · 2017-02-15T14:30:43Z

appveyor.yml

@@ -0,0 +1,15 @@
+environment:


干掉吧，win 没法测

atian25 · 2017-02-15T14:31:25Z

config/config.default.js

+  *  * @member Config#mongoose
+  *  * @property {String} url - connect url
+  *  * @property {Object} options - options to pass to the driver and mongoose-specific
+  *  */


两个 * ？

给出 options 的文档链接

atian25 · 2017-02-15T14:34:20Z

lib/mongoose.js

+
+  app.beforeStart(function* () {
+    app.coreLogger.info('[egg-mongoose] starting...');
+    yield db.open(config.url, config.options);


后面打个日志

atian25 · 2017-02-15T14:36:41Z

test/fixtures/apps/mongoose-test/app/controller/user.js

+
+    * create() {
+      let user = new this.ctx.model.user({
+        name: this.ctx.request.body.name


末尾少个逗号，是 fixture lint 关掉了？

好吧，问题不大

object 末尾项也需要加上逗号？

嗯, 我们的 eslint 是这样的. 好处是, 如果加了一个 property, review 的时候就是一行, 而不是两行.

你把上面的 eslintignore 的 fixture 干掉, 跑跑看就知道了.

atian25 · 2017-02-15T14:37:02Z

test/fixtures/apps/mongoose-test/app/controller/user.js

+      let user = new this.ctx.model.user({
+        name: this.ctx.request.body.name
+      })
+      yield user.save()


atian25 · 2017-02-15T14:37:25Z

test/fixtures/apps/mongoose-test/app/controller/user.js

+  }
+
+  return UserController;
+}


分号，末尾多一空行

atian25 · 2017-02-15T14:38:24Z

.eslintignore

@@ -0,0 +1,2 @@
+test/fixtures


这里之前为何要忽略了？@popomore

dead-horse · 2017-02-15T15:09:54Z

package.json

+    "app",
+    "lib"
+  ],
+  "ci": {


加一个 "type": "travis"，就不会生成 appveyor 的 ci 文件了

用到 service 了，不能自动生成 travis。要干掉 egg-ci，并干掉这个配置

villins · 2017-02-16T02:21:05Z

@atian25 codecov 这个错误是需要 fix ？

atian25 · 2017-02-16T02:27:50Z

cov 先不用管吧

atian25

+1

villins · 2017-02-16T07:32:17Z

@dead-horse 在 beforeStart 做了确保连接建立的检查

dead-horse · 2017-02-16T08:57:39Z

lib/mongoose.js

+
+  db.on('error', err => {
+    err.message = `[egg-mongoose]${err.message}`;
+    app.coreLogger.info(err);


app.coreLogger.error(err);

dead-horse · 2017-02-16T08:58:59Z

lib/mongoose.js

+  });
+
+  db.on('disconnected', () => {
+    app.coreLogger.warn('[egg-mongoose] %s disconnected', config.url);


这里也改成打印到 error 里面：

app.coreLogger.error(new Error(`[egg-mongoose] ${config.url} disconnected`);

dead-horse · 2017-02-16T09:00:26Z

lib/mongoose.js

+
+  db.on('disconnected', () => {
+    app.coreLogger.warn('[egg-mongoose] %s disconnected', config.url);
+    connect();


mongoose 不会自动重连的么？需要手动再发起一次 open？

我记得底层的 node-mongodb-native 是有自动重连的，这里不做任何延迟立刻重连看起来有问题

这里我测试下

@dead-horse 嗯，node-mongodb-native 是有自动重连的，可以在 options 设置重试次数和间隔时间
这样子就不用显式的不间断重连机制，ok？
http://mongodb.github.io/node-mongodb-native/2.1/api/Server.html

dead-horse · 2017-02-16T09:01:27Z

lib/mongoose.js

+    yield db.open(config.url, config.options);
+
+    const serverStatus = yield db.db.command({ serverStatus: 1 });
+    let status = 'bad';


const status = serverStatus.ok === 1 ? 'ok' : 'not ok';

dead-horse · 2017-02-16T09:02:29Z

test/fixtures/apps/mongoose-test/app/controller/user.js

+module.exports = app => {
+  class UserController extends app.Controller {
+    * index() {
+      const users = yield this.ctx.model.user.find({});


没有用到 Book 这个 model？

Book 这个 model 这里有 book controller 里面用到

dead-horse · 2017-02-16T09:03:35Z

其他基本差不多了

villins · 2017-02-16T13:27:00Z

麻烦再 review 下，可以的话合并发布下

atian25

辛苦了，棒棒哒

dead-horse · 2017-02-16T14:41:28Z

合并发布了

dead-horse · 2017-02-16T14:50:50Z

@villins 你的 npm 账号是多少

dead-horse · 2017-02-16T14:51:35Z

可以写一篇文档放到教程里面，和 https://eggjs.org/zh-cn/tutorials/mysql.html 类似

[skip ci] ## 1.0.0 (2023-08-12) ### ⚠ BREAKING CHANGES * Drop Node.js < 14 and egg < 3 support ### Features * [BREAKING_CHANGE] add unregular model judgement ([#7](#7)) ([ffde348](ffde348)) * bump mongoose version to 5.0 ([#20](#20)) ([a3405d6](a3405d6)) * first implement ([#2](#2)) ([45419ad](45419ad)) * support mongoose global plugin ([#35](#35)) ([1f450fb](1f450fb)) * support multi client ([#15](#15)) ([22d134b](22d134b)) * support plugins for special clients ([#41](#41)) ([67f8f1f](67f8f1f)) * update mongoose's version to be compatable with typescript schemas (Nodejs >=14.x), with Egg>=3.x ([#54](#54)) ([c87f19d](c87f19d)) ### Bug Fixes * doc typo. ([#40](#40)) ([8ef2ebc](8ef2ebc)) * fix __mongoose refer ([#16](#16)) ([b281b15](b281b15)) * hide password of mongo url ([#32](#32)) ([441b6fc](441b6fc)) * remove heartbeat ([#4](#4)) ([343cc78](343cc78)) * replace auth with string replace ([#34](#34)) ([5b9f8ba](5b9f8ba)) * shall re-throw errors on first connect ([#18](#18)) ([dde9037](dde9037)) * **typescript:** types of mongoose should be dependencies ([#24](#24)) ([de7e54e](de7e54e))

feat: first implement

4ee37cc

atian25 requested changes Feb 15, 2017

View reviewed changes

feat: add disconnect && reconnect

3562427

atian25 reviewed Feb 15, 2017

View reviewed changes

style: run eslint

ddbc5f5

dead-horse requested changes Feb 15, 2017

View reviewed changes

popomore reviewed Feb 15, 2017

View reviewed changes

feat: add mongodb ci service and first open connection on beforeStart

bfa6bba

atian25 requested changes Feb 15, 2017

View reviewed changes

atian25 reviewed Feb 15, 2017

View reviewed changes

popomore reviewed Feb 15, 2017

View reviewed changes

villins added 2 commits February 15, 2017 22:22

feat: add capitalize model file name test and fix review

8366ea7

docs: rm top desc

f206fa3

atian25 requested changes Feb 15, 2017

View reviewed changes

dead-horse reviewed Feb 15, 2017

View reviewed changes

style: fix review comment and delete appveyor.yml

8c36df8

style: fix eslint warning

1842f90

atian25 approved these changes Feb 16, 2017

View reviewed changes

feat: ensure that the connection is established berfore app start

54fac28

atian25 mentioned this pull request Feb 16, 2017

can't reload worker when file change, use vagrant eggjs/egg#380

Closed

dead-horse reviewed Feb 16, 2017

View reviewed changes

feat: use mongodb native autoretry when disconnected

fb1b0f7

atian25 approved these changes Feb 16, 2017

View reviewed changes

dead-horse approved these changes Feb 16, 2017

View reviewed changes

dead-horse merged commit 45419ad into eggjs:master Feb 16, 2017


		## 使用场景

		- Why and What: 描述为什么会有这个插件，它主要在完成一件什么事情。

feat: first implement #2

feat: first implement #2

Conversation

villins commented Feb 15, 2017 • edited by atian25 Loading

villins commented Feb 15, 2017

jtyjty99999 commented Feb 15, 2017

atian25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atian25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

villins commented Feb 15, 2017

jtyjty99999 commented Feb 15, 2017

villins commented Feb 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atian25 commented Feb 15, 2017 via email

Choose a reason for hiding this comment

popomore commented Feb 15, 2017

jtyjty99999 commented Feb 15, 2017

codecov bot commented Feb 15, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

atian25 commented Feb 15, 2017

popomore commented Feb 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atian25 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

villins commented Feb 16, 2017

atian25 commented Feb 16, 2017

atian25 left a comment

Choose a reason for hiding this comment

villins commented Feb 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dead-horse commented Feb 16, 2017

villins commented Feb 16, 2017

atian25 left a comment

Choose a reason for hiding this comment

dead-horse commented Feb 16, 2017

dead-horse commented Feb 16, 2017

villins commented Feb 15, 2017 •

edited by atian25

Loading

codecov bot commented Feb 15, 2017 •

edited

Loading