Skip to content

Commit 516844b

Browse files
BetreeBetree
authored
fix: normalize image sizes (#629)
1 parent 1295ace commit 516844b

File tree

11 files changed

+712
-197
lines changed

11 files changed

+712
-197
lines changed

‎package-lock.json‎

Lines changed: 185 additions & 184 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
"npm": "10.x"
1717
},
1818
"dependencies": {
19+
"@apollo/client": "^3.13.9",
1920
"@hyperwatch/hyperwatch": "4.0.0",
20-
"apollo-boost": "0.4.9",
2121
"bluebird": "3.7.2",
2222
"cached-request": "3.0.0",
2323
"debug": "4.3.4",
@@ -47,6 +47,7 @@
4747
"build:clean": "rm -rf dist",
4848
"build:server": "babel --copy-files src --out-dir dist",
4949
"test": "npm run test:server",
50+
"test:watch": "npm run test:server -- --watch",
5051
"test:server": "TZ=UTC jest test/server/*",
5152
"lint": "eslint \"src/**/*.js\" \"test/**/*.js\"",
5253
"lint:fix": "npm run lint -- --fix",

‎src/server/controllers/avatar.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ import debug from 'debug';
55
import sizeOf from 'image-size';
66
import mime from 'mime-types';
77

8+
import { MAX_AVATAR_HEIGHT } from '../lib/constants';
89
import { fetchMembersWithCache } from '../lib/graphql';
10+
import { normalizeSize } from '../lib/image-size';
911
import { imageRequest } from '../lib/request';
1012
import { getCloudinaryUrl, parseToBooleanDefaultFalse, parseToBooleanDefaultTrue } from '../lib/utils';
1113
import { logger } from '../logger';
@@ -114,6 +116,9 @@ export default async function avatar(req, res) {
114116

115117
if (req.query.avatarHeight) {
116118
maxHeight = Number(req.query.avatarHeight);
119+
if (maxHeight > MAX_AVATAR_HEIGHT) {
120+
maxHeight = normalizeSize(maxHeight, MAX_AVATAR_HEIGHT);
121+
}
117122
} else {
118123
maxHeight = format === 'svg' ? 128 : 64;
119124
if (selector.match(/silver/)) {

‎src/server/controllers/background.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import mime from 'mime-types';
33
import sharp from 'sharp';
44

55
import { fetchCollectiveWithCache } from '../lib/graphql';
6+
import { normalizeSize } from '../lib/image-size';
67
import { asyncRequest } from '../lib/request';
78
import { logger } from '../logger';
89

@@ -34,11 +35,14 @@ export default async function background(req, res, next) {
3435

3536
const params = {};
3637

38+
// Profile page hero uses ~1800x800
3739
if (Number(width)) {
3840
params['width'] = Number(width);
41+
params.width = normalizeSize(params.width, 1800);
3942
}
4043
if (Number(height)) {
4144
params['height'] = Number(height);
45+
params.height = normalizeSize(params.height, 800);
4246
}
4347

4448
const image = await getImageData(imageUrl);

‎src/server/controllers/logo.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import fetch from 'node-fetch';
99
import sharp from 'sharp';
1010

1111
import { generateAsciiLogo } from '../lib/ascii-logo';
12+
import { MAX_AVATAR_HEIGHT } from '../lib/constants';
1213
import { fetchCollectiveWithCache } from '../lib/graphql';
14+
import { normalizeSize } from '../lib/image-size';
1315
import { getUiAvatarUrl, parseToBooleanDefaultFalse, parseToBooleanDefaultTrue } from '../lib/utils';
1416
import { logger } from '../logger';
1517

@@ -111,10 +113,12 @@ export default async function logo(req, res) {
111113

112114
if (Number(height)) {
113115
params['height'] = Number(height);
116+
params.height = normalizeSize(params.height, MAX_AVATAR_HEIGHT);
114117
}
115118

116119
if (Number(width)) {
117120
params['width'] = Number(width);
121+
params.width = normalizeSize(params.width, MAX_AVATAR_HEIGHT);
118122
}
119123

120124
let imageUrl;

‎src/server/lib/constants.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const MAX_AVATAR_HEIGHT = 512;

‎src/server/lib/graphql.js‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import ApolloClient from 'apollo-boost';
1+
import { ApolloClient, ApolloLink, HttpLink, InMemoryCache } from '@apollo/client/core';
22
import debug from 'debug';
33
import gql from 'graphql-tag';
44
import { flatten, pick, uniqBy } from 'lodash';
@@ -25,10 +25,30 @@ const getGraphqlUrl = ({ version = 'v1' } = {}) => {
2525

2626
let client;
2727

28+
/**
29+
* @returns {ApolloClient}
30+
*/
2831
function getClient() {
2932
if (!client) {
30-
// client = new GraphQLClient(getGraphqlUrl(), { headers });
31-
client = new ApolloClient({ fetch, uri: getGraphqlUrl({ version: 'v1' }) });
33+
client = new ApolloClient({
34+
cache: new InMemoryCache(),
35+
link: ApolloLink.from([
36+
new HttpLink({
37+
fetch,
38+
uri: getGraphqlUrl({ version: 'v1' }),
39+
}),
40+
]),
41+
defaultOptions: {
42+
watchQuery: {
43+
fetchPolicy: 'no-cache',
44+
errorPolicy: 'all',
45+
},
46+
query: {
47+
fetchPolicy: 'no-cache',
48+
errorPolicy: 'all',
49+
},
50+
},
51+
});
3252
}
3353
return client;
3454
}
@@ -38,9 +58,7 @@ function graphqlRequest(query, variables) {
3858
// return getClient().request(query, variables);
3959

4060
// With ApolloClient as client
41-
return getClient()
42-
.query({ query, variables, fetchPolicy: 'network-only' })
43-
.then((result) => result.data);
61+
return getClient().query({ query, variables, fetchPolicy: 'no-cache' });
4462
}
4563

4664
/*
@@ -59,16 +77,20 @@ export async function fetchCollective(collectiveSlug) {
5977
backgroundImage
6078
isGuest
6179
parentCollective {
80+
id
6281
image
6382
backgroundImage
6483
}
6584
}
6685
}
6786
`;
6887

69-
const result = await graphqlRequest(query, { collectiveSlug });
88+
const { data, errors } = await graphqlRequest(query, { collectiveSlug });
89+
if (errors?.length) {
90+
throw new Error(errors[0].message);
91+
}
7092

71-
return result.Collective;
93+
return data.Collective;
7294
}
7395

7496
export async function fetchCollectiveWithCache(collectiveSlug, options = {}) {
@@ -94,7 +116,9 @@ export async function fetchMembersStats(variables) {
94116
query = gql`
95117
query fetchMembersStats($collectiveSlug: String) {
96118
Collective(slug: $collectiveSlug) {
119+
id
97120
stats {
121+
id
98122
backers {
99123
all
100124
users
@@ -122,7 +146,9 @@ export async function fetchMembersStats(variables) {
122146
query = gql`
123147
query fetchMembersStatsForTier($collectiveSlug: String, $tierSlug: String) {
124148
Collective(slug: $collectiveSlug) {
149+
id
125150
tiers(slug: $tierSlug) {
151+
id
126152
slug
127153
name
128154
stats {
@@ -143,8 +169,11 @@ export async function fetchMembersStats(variables) {
143169
};
144170
};
145171
}
146-
const result = await graphqlRequest(query, variables);
147-
const count = processResult(result);
172+
const { data, errors } = await graphqlRequest(query, variables);
173+
if (errors?.length) {
174+
throw new Error(errors[0].message);
175+
}
176+
const count = processResult(data);
148177
return count;
149178
}
150179

@@ -218,9 +247,13 @@ export async function fetchMembers({ collectiveSlug, tierSlug, backerType, isAct
218247
query = gql`
219248
query fetchMembersWithTier($collectiveSlug: String, $tierSlug: [String], $isActive: Boolean) {
220249
Collective(slug: $collectiveSlug) {
250+
id
221251
tiers(slugs: $tierSlug) {
252+
id
222253
orders(isActive: $isActive, isProcessed: true) {
254+
id
223255
fromCollective {
256+
id
224257
type
225258
slug
226259
name
@@ -241,14 +274,17 @@ export async function fetchMembers({ collectiveSlug, tierSlug, backerType, isAct
241274
};
242275
}
243276

244-
const result = await graphqlRequest(query, {
277+
const { data, errors } = await graphqlRequest(query, {
245278
collectiveSlug,
246279
tierSlug,
247280
type,
248281
role,
249282
isActive,
250283
});
251-
const members = processResult(result);
284+
if (errors?.length) {
285+
throw new Error(errors[0].message);
286+
}
287+
const members = processResult(data);
252288
return members;
253289
}
254290

‎src/server/lib/image-size.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { isNil } from 'lodash';
2+
3+
/**
4+
* Normalize the size to 8, 16, 32, 64, 128, etc. up to maxSize
5+
*
6+
* @param {number} size
7+
* @param {number} maxSize
8+
* @returns {number}
9+
*/
10+
export const normalizeSize = (size, maxSize) => {
11+
if (isNil(size)) {
12+
return size;
13+
} else if (size > maxSize) {
14+
return maxSize;
15+
} else if (size <= 8) {
16+
return 8;
17+
} else {
18+
const result = 2 ** Math.ceil(Math.log2(size));
19+
return result > maxSize ? maxSize : result; // Need to re-check maxSize in case it's not a power of 2
20+
}
21+
};

‎test/server/avatar.routes.test.js‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import '../../src/server/env';
2+
3+
import fetch from 'node-fetch';
4+
import sharp from 'sharp';
5+
6+
const imagesUrl = process.env.IMAGES_URL;
7+
const timeout = 30000;
8+
const cacheBurst = `cacheBurst=${Math.round(Math.random() * 100000)}`;
9+
10+
const fetchResponse = (path) => {
11+
const pathWithCacheBurst = [path, cacheBurst].join(path.indexOf('?') === -1 ? '?' : '&');
12+
return fetch(`${imagesUrl}${pathWithCacheBurst}`);
13+
};
14+
15+
describe('avatar.routes.test.js', () => {
16+
describe('Size Normalization', () => {
17+
describe('avatarHeight normalization', () => {
18+
test(
19+
'should cap avatarHeight at 512px when requesting 2000px',
20+
async () => {
21+
const res = await fetchResponse('/apex/backers/0/avatar.png?avatarHeight=2000');
22+
expect(res.status).toEqual(200);
23+
24+
const buffer = await res.buffer();
25+
const metadata = await sharp(buffer).metadata();
26+
expect(metadata.height).toEqual(512); // Capped at MAX_AVATAR_HEIGHT
27+
},
28+
timeout,
29+
);
30+
31+
test(
32+
'should normalize 1px avatarHeight to 8px minimum',
33+
async () => {
34+
const res = await fetchResponse('/apex/backers/0/avatar.png?avatarHeight=1');
35+
expect(res.status).toEqual(200);
36+
37+
const buffer = await res.buffer();
38+
const metadata = await sharp(buffer).metadata();
39+
expect(metadata.height).toEqual(8); // Minimum size
40+
},
41+
timeout,
42+
);
43+
44+
test(
45+
'should round avatarHeight to next power of 2 (200px -> 256px)',
46+
async () => {
47+
const res = await fetchResponse('/apex/backers/0/avatar.png?avatarHeight=200');
48+
expect(res.status).toEqual(200);
49+
50+
const buffer = await res.buffer();
51+
const metadata = await sharp(buffer).metadata();
52+
expect(metadata.height).toEqual(256); // Next power of 2
53+
},
54+
timeout,
55+
);
56+
57+
test(
58+
'should keep exact power of 2 avatarHeight unchanged (128px)',
59+
async () => {
60+
const res = await fetchResponse('/apex/backers/0/avatar.png?avatarHeight=128');
61+
expect(res.status).toEqual(200);
62+
63+
const buffer = await res.buffer();
64+
const metadata = await sharp(buffer).metadata();
65+
expect(metadata.height).toEqual(128); // Already power of 2
66+
},
67+
timeout,
68+
);
69+
});
70+
71+
describe('different backer types and tiers maintain normalization', () => {
72+
test(
73+
'should normalize size for sponsors',
74+
async () => {
75+
const res = await fetchResponse('/apex/sponsors/0/avatar.png?avatarHeight=1000');
76+
expect(res.status).toEqual(200);
77+
78+
const buffer = await res.buffer();
79+
const metadata = await sharp(buffer).metadata();
80+
expect(metadata.height).toEqual(64); // Capped at MAX_AVATAR_HEIGHT
81+
},
82+
timeout,
83+
);
84+
85+
test(
86+
'should normalize size for tier-specific avatars',
87+
async () => {
88+
const res = await fetchResponse('/apex/tiers/backers/0/avatar.png?avatarHeight=3');
89+
expect(res.status).toEqual(200);
90+
91+
const buffer = await res.buffer();
92+
const metadata = await sharp(buffer).metadata();
93+
expect(metadata.height).toEqual(64); // Minimum size
94+
},
95+
timeout,
96+
);
97+
98+
test(
99+
'should normalize size for organization avatars',
100+
async () => {
101+
const res = await fetchResponse('/apex/organizations/0/avatar.png?avatarHeight=75');
102+
expect(res.status).toEqual(200);
103+
104+
const buffer = await res.buffer();
105+
const metadata = await sharp(buffer).metadata();
106+
expect(metadata.height).toEqual(128); // Next power of 2 (75 -> 128)
107+
},
108+
timeout,
109+
);
110+
});
111+
112+
describe('default size handling with tier multipliers', () => {
113+
test(
114+
'should respect size normalization even with tier multipliers',
115+
async () => {
116+
// Test with a size that would exceed MAX_AVATAR_HEIGHT after tier multiplier
117+
const res = await fetchResponse('/apex/tiers/diamond/0/avatar.png?avatarHeight=300');
118+
expect(res.status).toEqual(200);
119+
120+
const buffer = await res.buffer();
121+
const metadata = await sharp(buffer).metadata();
122+
expect(metadata.height).toEqual(64); // Should be capped despite multiplier
123+
},
124+
timeout,
125+
);
126+
});
127+
});
128+
});

0 commit comments

Comments
 (0)