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

add SliceableRepository #6

Merged
merged 6 commits into from
Oct 25, 2020
Merged

Conversation

katagiri-kazumune
Copy link
Collaborator

@katagiri-kazumune katagiri-kazumune commented Mar 18, 2020

概要

#2

内容

Service / Repository 向け
(実際に SQL を意識するのは spring-data-mirage 側の PR を参照)

  • SliceableRepository
    • この PR のメイン
    • この interface を implement すると Slice を取得する SQL が発行可能
  • Slice
    • Slice 形式の SQL 発行結果を保持する interface
    • 実装は SliceImpl
  • Sliceable
    • SQL 発行時のパラメータの interface
    • 実装は SliceRequest

Controller 向け

  • SlicedResources
    • Controller 側で意識する Value Object
    • レスポンス JSON の構成情報(Chunk で返す場合の ChunkedResources 相当)
  • SliceableDefault
    • Controller のメソッドのパラメータとして設定するアノテーション(Chunk のパラメータを受け取る場合の @ChunkableDefault 相当)
    • デフォルト size=10, direction=ASC, page_number=0
  • SliceableHandlerMethodArgumentResolver
    • Controller のリクエストを元に Sliceable インスタンスを生成する
    • WebMvcConfigurer の実装クラスで追加する必要がある
    • SpringDataSliceableAnnotationUtils はその Utils

@katagiri-kazumune katagiri-kazumune added the enhancement New feature or request label Mar 18, 2020
@katagiri-kazumune katagiri-kazumune self-assigned this Mar 18, 2020
@@ -11,4 +11,5 @@ dependencies {

testCompile "com.fasterxml.jackson.core:jackson-databind"
testCompile "com.jayway.jsonpath:json-path-assert:2.2.0"
testCompile "org.skyscreamer:jsonassert:1.5.0"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

JSON 文字列の比較で使用

@katagiri-kazumune katagiri-kazumune changed the title add SliceableRepository [WIP] add SliceableRepository Mar 18, 2020
@katagiri-kazumune katagiri-kazumune changed the title [WIP] add SliceableRepository add SliceableRepository Mar 18, 2020
です。

ソート順は、ユニークになるように指定してください。
例えば、`ORDER BY create_at ASC` にした場合、create_at が同じ値のレコードのソート順は不定です。
Copy link
Contributor

Choose a reason for hiding this comment

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

ページングで全部の値が取得できることが保証できない、まで書いちゃってよいかと(ソートにこだわらなければユニークでなくてもよいようにとられそう)

Copy link
Collaborator Author

@katagiri-kazumune katagiri-kazumune Mar 25, 2020

Choose a reason for hiding this comment

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

言い切りました。
0f6f479

* SliceableHandlerMethodArgumentResolver
* Slice のリクエストを受け取る場合

インスタンスを add してください。
Copy link
Contributor

Choose a reason for hiding this comment

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

なんとなくサンプルまでのせちゃってよさそうという気持ちになりました

public class WebMvcConfiguration implements WebMvcConfigurer {

	@Override
	public void addArgumentResolvers(List<HandlerMethodArgumentResolver> argumentResolvers) {
		argumentResolvers.add(new ChunkableHandlerMethodArgumentResolver());
		argumentResolvers.add(new SliceableHandlerMethodArgumentResolver());
	}
}	

Copy link
Collaborator Author

@katagiri-kazumune katagiri-kazumune Mar 25, 2020

Choose a reason for hiding this comment

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

書きました
0f6f479

@Setter(AccessLevel.PACKAGE)
@XmlElement(name = "embedded")
@JsonProperty("_embedded")
@JsonInclude(JsonInclude.Include.NON_NULL)
Copy link
Contributor

Choose a reason for hiding this comment

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

これNullのケースないような気がしますがどうでしょうか(コンストラクタ的にも仕様的にも)
https://github.com/classmethod/cmapi-five-rings/blob/24ebeff38f9f8a6ab257b23c30ab44a1f5e0f7f0/content/section2/pagination-offset.md

Copy link
Collaborator Author

@katagiri-kazumune katagiri-kazumune Mar 25, 2020

Choose a reason for hiding this comment

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

setter で設定できるからコンパイル的には...と思ったのですが、setter もデフォルトコンストラクタも撲滅で解決。
0f6f479

@katagiri-kazumune katagiri-kazumune force-pushed the feature/fix-issue-2-add-slice branch 2 times, most recently from a03800d to 0f6f479 Compare March 25, 2020 10:29
@katagiri-kazumune katagiri-kazumune force-pushed the feature/fix-issue-2-add-slice branch from 282f4de to 47dcbd6 Compare March 30, 2020 05:09
@katagiri-kazumune
Copy link
Collaborator Author

@inabajunmr
https://github.com/classmethod/cmapi-five-rings/pull/47/files/3eda71a4d1f0cdec945a27cbf7153914492f2e8a#r399915846
の実装が
47dcbd6
になります。

SliceableHandlerMethodArgumentResolver でチェックしても良かったのですが、エラーハンドリングが柔軟にできなさそうだったので(Controller に入る前に throw する)、spring-data-mirage 側で Sliceable#validate を呼び出す想定です。

@inabajunmr
Copy link
Contributor

この観点のバリデーションエラーについては固定で400で良い気がしますが、SliceableHandlerMethodArgumentResolverだと困るのってどういうのを想定してますか?(もしくは実装がしづらい?)

mirageでやるようにするとControllerのハンドリング漏れで500返るとかそういうイベント起きそうだなあと

@katagiri-kazumune katagiri-kazumune force-pushed the feature/fix-issue-2-add-slice branch from c376134 to 1d55963 Compare March 31, 2020 00:13
@katagiri-kazumune
Copy link
Collaborator Author

katagiri-kazumune commented Mar 31, 2020

SliceableHandlerMethodArgumentResolverだと困る

直接 spring-data-mirage を呼んだ時のことを想定したのですが、SliceableHandlerMethodArgumentResolver でもチェックはしておいて良いとも思ったのでチェック処理追加してみました。

bd4bda3

@katagiri-kazumune katagiri-kazumune force-pushed the feature/fix-issue-2-add-slice branch from 1d55963 to bd4bda3 Compare March 31, 2020 00:19
@katagiri-kazumune katagiri-kazumune force-pushed the feature/fix-issue-2-add-slice branch from bd4bda3 to 21810a4 Compare July 27, 2020 01:12
@katagiri-kazumune katagiri-kazumune added this to the 0.42.0 milestone Jul 28, 2020
@katagiri-kazumune katagiri-kazumune force-pushed the feature/fix-issue-2-add-slice branch from fa67641 to 56b0195 Compare October 25, 2020 23:49
@katagiri-kazumune katagiri-kazumune merged commit 816d89e into master Oct 25, 2020
@katagiri-kazumune katagiri-kazumune deleted the feature/fix-issue-2-add-slice branch October 25, 2020 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants