Skip to content
This repository has been archived by the owner on Feb 25, 2022. It is now read-only.

feat(data): use modern data format and truncate result if too large #136

Merged
merged 1 commit into from
Sep 30, 2020

Conversation

tripodsan
Copy link
Contributor

@tripodsan tripodsan commented Jul 20, 2020

fixes #119

BREAKING CHANGE: the response body is no longer a JSON array but an object that contains a limit, offset, total and data property.

see examples:

describe('Index result Tests', () => {
it('handles limit correctly', async () => {
const { main: customMain } = proxyquire('../src/index.js', {
'./embed.js': () => ({
body: TEST_DATA,
}),
});
const result = await customMain({
src: 'https://foo.com',
'hlx_p.limit': 10,
});
assert.deepEqual(result, {
body: {
data: TEST_DATA.slice(0, 10),
limit: 10,
offset: 0,
total: 10000,
},
});
});
it('handles offset correctly', async () => {
const { main: customMain } = proxyquire('../src/index.js', {
'./embed.js': () => ({
body: TEST_DATA,
}),
});
const result = await customMain({
src: 'https://foo.com',
'hlx_p.offset': 9000,
});
assert.deepEqual(result, {
body: {
data: TEST_DATA.slice(9000),
limit: 1000,
offset: 9000,
total: 10000,
},
});
});
it('handles limit and offset correctly', async () => {
const { main: customMain } = proxyquire('../src/index.js', {
'./embed.js': () => ({
body: TEST_DATA,
}),
});
const result = await customMain({
src: 'https://foo.com',
'hlx_p.limit': 50,
'hlx_p.offset': 100,
});
assert.deepEqual(result, {
body: {
data: TEST_DATA.slice(100, 150),
limit: 50,
offset: 100,
total: 10000,
},
});
});
it('truncates result if too large for action response', async () => {
const { main: customMain } = proxyquire('../src/index.js', {
'./embed.js': () => ({
body: TEST_DATA,
}),
});
const result = await customMain({
src: 'https://foo.com',
});
assert.deepEqual(result, {
body: {
data: TEST_DATA.slice(0, 4970),
limit: 4970,
offset: 0,
total: 10000,
},
});
});
});

@github-actions
Copy link

This PR will trigger a major release when merged.

@tripodsan tripodsan requested review from rofe and removed request for profesori July 20, 2020 05:47
Copy link

@kptdobe kptdobe left a comment

Choose a reason for hiding this comment

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

Since this is a breaking change and I suspect it is consumed by theblog, I think we should create the follow tickets in the various consumer repos (if there are any...)

@tripodsan
Copy link
Contributor Author

Since this is a breaking change and I suspect it is consumed by theblog, I think we should create the follow tickets in the various consumer repos (if there are any...)

yes, we should. we can also prepare the js clients:

const data = Array.isArray(response) ? response : response.data;

It will also effect the data-embed in the pipeline.

Copy link
Contributor

@trieloff trieloff left a comment

Choose a reason for hiding this comment

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

Not a fan of breaking compatibility and complicating the format, but if it works, ok. The helix-pipeline fix should make sure to be able to handle both the old and the new format.

@codecov
Copy link

codecov bot commented Aug 31, 2020

Codecov Report

Merging #136 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #136   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          320       331   +11     
=========================================
+ Hits           320       331   +11     
Impacted Files Coverage Δ
src/index.js 100.00% <100.00%> (ø)
src/querybuilder/filter.js 100.00% <100.00%> (ø)

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 7c43d03...02f4351. Read the comment docs.

fixes #119

BREAKING CHANGE: the response body is no longer a JSON array but an
                 object that contains a limit, offset, total and
                 data property.
@tripodsan tripodsan merged commit 9d1e924 into master Sep 30, 2020
@tripodsan tripodsan deleted the modern-data-representation branch September 30, 2020 07:30
trieloff pushed a commit that referenced this pull request Sep 30, 2020
# [2.0.0](v1.9.13...v2.0.0) (2020-09-30)

### Features

* **data:** use modern data format and truncate result if too large ([#136](#136)) ([9d1e924](9d1e924)), closes [#119](#119)

### BREAKING CHANGES

* **data:** the response body is no longer a JSON array but an
                 object that contains a limit, offset, total and
                 data property.
@trieloff
Copy link
Contributor

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modernize data representation
4 participants