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

feat: include query pieces #212

Merged
merged 10 commits into from
Sep 10, 2023
Merged

feat: include query pieces #212

merged 10 commits into from
Sep 10, 2023

Conversation

1244453393
Copy link
Contributor

@1244453393 1244453393 commented Sep 6, 2023

自己业务需求需要,做了扩展,这种需求或许其他人也有

@wey-gu wey-gu requested a review from CorvusYe September 6, 2023 10:40
@wey-gu
Copy link
Member

wey-gu commented Sep 6, 2023

@1244453393 👍,感谢这个扩充!能把文档部分例子、描述也修改一下么?

@wey-gu wey-gu changed the title 支持对nGQL语句片段的引用,类似于mybatis中的include一个sql片段。目前仅支持同文件的nGQL片段引用 feat: include query pieces Sep 6, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 6, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (ed810f3) 0.00% compared to head (1ae88e9) 0.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #212   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files          72      74    +2     
  Lines        2471    2513   +42     
  Branches      270     275    +5     
======================================
- Misses       2471    2513   +42     
Files Changed Coverage Δ
...rib/ngbatis/binding/beetl/functions/IncludeFn.java 0.00% <0.00%> (ø)
...ebula/contrib/ngbatis/io/MapperResourceLoader.java 0.00% <0.00%> (ø)
.../org/nebula/contrib/ngbatis/models/ClassModel.java 0.00% <0.00%> (ø)
...a/org/nebula/contrib/ngbatis/models/NgqlModel.java 0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@1244453393
Copy link
Contributor Author

@1244453393 👍,感谢这个扩充!能把文档部分例子、描述也修改一下么?

@wey-gu 没问题,吃完晚饭后做修改

Copy link
Collaborator

@CorvusYe CorvusYe left a comment

Choose a reason for hiding this comment

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

如果可以做成跨文件的,试试从 ENV 的全局上下文着手,里面有全部的 ClassModel.
或者跨文件的功能也可以放到下次的 PR。
一激动,忘记说我很喜欢这个功能了~

@@ -11,5 +11,17 @@
RETURN 1
</select>

<nGQL id="ngql-include-test-value">
666
Copy link
Collaborator

Choose a reason for hiding this comment

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

看到这里我有个大胆的想法,在引入的时候可以在 ng.include 中指定变量名来承接DAO所传的参数,并 ${ 指定的变量名 } 进行参数传入,那简直就太爽了。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

当前已经支持dao方法传入的参数了,不需要在调用ng.include函数的时候单独指定。

@wey-gu
Copy link
Member

wey-gu commented Sep 7, 2023

@1244453393 Sorry I meant maybe you could add the example of your extended expression to the docs, not to include my PR related to docs :-P,

@1244453393
Copy link
Contributor Author

@1244453393 Sorry I meant maybe you could add the example of your extended expression to the docs, not to include my PR related to docs :-P,

@wey-gu 额不好意思,我还没改完文档,我刚刚是想从原仓库拉取新内容到我的仓库里,不知道刚刚是不是搞错了,我平时用svn,git用的少,不太熟

@wey-gu
Copy link
Member

wey-gu commented Sep 7, 2023

@1244453393 Sorry I meant maybe you could add the example of your extended expression to the docs, not to include my PR related to docs :-P,

@wey-gu 额不好意思,我还没改完文档,我刚刚是想从原仓库拉取新内容到我的仓库里,不知道刚刚是不是搞错了,我平时用svn,git用的少,不太熟

别担心哈,明白了,我以为是误解了我提到的文档的部分,原来你是想 rebase

# 我一般是这么弄哈
# 看远端的 repo 有哪些
git remote -v
# 假设 “origin” 已经是自己的 fork,可以再增加 upstream 为你要 PR 的 repo,比如
git remote add upstream https://github.com/nebula-contrib/ngbatis.git
# 把自己的 branch 从 master 分出来叫作 my_work,我们回头就会把它推导远端 1244453393/ngbatis 的 my_work ,这里,我看你弄得是 master (这也是可以的)
# git checkout -b my_work
# 我们现在就假设用 1244453393/ngbatis 的 master

# 如果在 pr 过程中,上游变了,想获得更新,我习惯的方式是:
# 设定当前的上游为 remote 中的 upstream 的 master
git branch --set-upstream-to=upstream/master
# 拉取一下上游,用 fetch 只会更新本地缓存,不会改当前 branch
git fetch upstream
# 然后 rebase,rebase 的意思就是让后来新的变化放在你的修改的前边
git pull --rebase
# 再次提交你的变化,因为 base 变了,需要 -f 去强制推到你自己 fork 的远端,也是和 pr 关联的 branch
git push -f

补充了相关文档
添加了相关代码例子
@1244453393
Copy link
Contributor Author

Now:
函数支持跨mapper文件引用
补充了相关文档
添加了相关代码例子

@wey-gu
Copy link
Member

wey-gu commented Sep 7, 2023

赞!

@1244453393
Copy link
Contributor Author

赞!

英文水平有限,英文文档就麻烦您弄一下了
build失败是maven检查代码风格插件报的错吗,缩进规范是2个字符,我的idea默认缩进4个字符,抱歉抱歉

@wey-gu wey-gu requested a review from CorvusYe September 7, 2023 06:36
@wey-gu
Copy link
Member

wey-gu commented Sep 7, 2023

赞,英文回头我来稿,大叶老师瞅瞅咋样了哈 @CorvusYe

This reverts commit 736e56c.
Copy link
Collaborator

@CorvusYe CorvusYe left a comment

Choose a reason for hiding this comment

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

是个很棒的实现。可以的话把调用方式以 test 的方式做,避免之后的单元测试会有所遗漏。
当然,现阶段也没问题,回头可以补上。

</nGQL>

<nGQL id="include-test-value">
${myInt}
Copy link
Collaborator

Choose a reason for hiding this comment

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

关于昨天提的参数的问题,调用的例子大概是这样的:

testIncludeParam( @Param("person") Person person );
testIncludeParam1( @Param("map") Map<String, Object> map);

testIncludeParam2( @Param("people") List<Person> people );
    <nGQL id="include-test">
        ${ myInt }
    </nGQL>

<select id="testIncludeParam">
   RETURN @ng.include( 'include-test', { "myInt": "person.age" } );
</select>

<select id="testIncludeParam1">
   RETURN @ng.include( 'include-test', { "myInt": "map.age" } );
</select>

<select id="testIncludeParam2">
   @for ( person in people ) {
       RETURN @ng.include( 'include-test', { "myInt": "person.age" } );
   }
</select>
<!-- 如果第二个参数不传,可以认为直接使用与接口相同的参数 globalVar -->

当前的情况,就要求 <nGQL> 标签的变量名要严格的与接口的声明名称一致
但是在正常开发中,这样的情况下不一定是多见的,更多的是不同接口使用不同的参数名,以确保参数名具备更强的实际意义,而不会因为了使用 include 而影响接口本身的命名。

这样的改动可能具备更强的实际使用意义。

这是一点点小建议,总的来说,我依然觉得现在的版本是一个很棒的实现!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个建议确实是我没想到的,我在后续的版本中加上

@1244453393
Copy link
Contributor Author

2023-09-09:
ng.include函数支持额外参数
增加了一个适用于ngbatis-mapper文件的dtd约束,使得开发mapper能有所提示

@wey-gu
Copy link
Member

wey-gu commented Sep 10, 2023

👍,这次 ci pass 了 我就先 merge 了哈,后边可以分开 pr 再跟进。

@wey-gu wey-gu merged commit 3dbb1cb into nebula-contrib:master Sep 10, 2023
1 check passed
@CorvusYe CorvusYe mentioned this pull request Nov 14, 2023
@CorvusYe CorvusYe mentioned this pull request Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants