Skip to content

Commit b2183b2

Browse files
simoncozenssimoncozens
committed
designer_profiles
1 parent 457c269 commit b2183b2

File tree

7 files changed

+224
-4
lines changed

7 files changed

+224
-4
lines changed

‎fontspector-py/tests/test_checks_googlefonts.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -3302,7 +3302,7 @@ def test_check_metadata_escaped_strings(check):
33023302
assert_results_contain(check(bad), FAIL, "escaped-strings")
33033303

33043304

3305-
@pytest.mark.skip("Check not ported yet.")
3305+
@pytest.mark.skip("This check is ported, but we can't mock the requests.")
33063306
@check_id("googlefonts/metadata/designer_profiles")
33073307
def test_check_metadata_designer_profiles(check, requests_mock):
33083308
"""METADATA.pb: Designer is listed with the correct name on
@@ -3328,7 +3328,7 @@ def test_check_metadata_designer_profiles(check, requests_mock):
33283328
requests_mock.get(
33293329
"https://raw.githubusercontent.com/google/fonts/master/"
33303330
"catalog/designers/sorkintype/sorkin_type.png",
3331-
content=b"\x89PNG\x0D\x0A\x1A\x0A",
3331+
content=b"\x89PNG\x0d\x0a\x1a\x0a",
33323332
)
33333333

33343334
# Delve Withrington is still not listed on the designers catalog.

‎profile-googlefonts/build.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ fn main() {
33
.pure()
44
.includes(["src/protos"])
55
.input("src/protos/fonts_public.proto")
6+
.input("src/protos/designers.proto")
67
.cargo_out_dir("protos")
78
.run_from_script();
89
}

‎profile-googlefonts/src/checks/googlefonts/metadata/designer_profiles.rs

+150
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
use crate::{
2+
checks::googlefonts::metadata::{family_proto, DesignerInfoProto},
3+
network_conditions::{is_designer_listed, DESIGNER_INFO_RAW_URL},
4+
};
5+
use fontspector_checkapi::prelude::*;
6+
7+
const NAME_DEUNICODIZATION: [(&str, &str); 19] = [
8+
("á", "a"),
9+
("é", "e"),
10+
("í", "i"),
11+
("ó", "o"),
12+
("ú", "u"),
13+
("à", "a"),
14+
("è", "e"),
15+
("ì", "i"),
16+
("ò", "o"),
17+
("ù", "u"),
18+
("ń", "n"),
19+
("ø", "o"),
20+
("ř", "r"),
21+
("ś", "s"),
22+
("ß", "ss"),
23+
("ł", "l"),
24+
("ã", "a"),
25+
("ı", "i"),
26+
("ü", "ue"),
27+
];
28+
29+
fn normalize_designer_name(designer: &str) -> String {
30+
designer
31+
.chars()
32+
.filter_map(|c| {
33+
if c.is_ascii_alphanumeric() {
34+
Some(c.to_ascii_lowercase())
35+
} else {
36+
NAME_DEUNICODIZATION
37+
.iter()
38+
.find(|(from, _)| *from == c.to_string())
39+
.map(|(_, to)| to.chars())
40+
.unwrap_or("".chars())
41+
.next()
42+
}
43+
})
44+
.collect()
45+
}
46+
47+
#[check(
48+
id = "googlefonts/metadata/designer_profiles",
49+
rationale = "
50+
51+
Google Fonts has a catalog of designers.
52+
53+
This check ensures that the online entries of the catalog can be found based
54+
on the designer names listed on the METADATA.pb file.
55+
56+
It also validates the URLs and file formats are all correctly set.
57+
58+
",
59+
proposal = "https://github.com/fonttools/fontbakery/issues/3083",
60+
title = "METADATA.pb: Designers are listed correctly on the Google Fonts catalog?",
61+
applies_to = "MDPB"
62+
)]
63+
fn designer_profiles(c: &Testable, context: &Context) -> CheckFnResult {
64+
let msg = family_proto(c).map_err(|e| {
65+
CheckError::Error(format!("METADATA.pb is not a valid FamilyProto: {:?}", e))
66+
})?;
67+
let mut problems = vec![];
68+
for designer in msg.designer().split(",") {
69+
let designer = normalize_designer_name(designer.trim());
70+
if designer == "multipledesigners" {
71+
problems.push(Status::fail(
72+
"multiple-designers",
73+
&format!(
74+
"Font family {} does not explicitely mention the names of its designers on its METADATA.pb file.",
75+
msg.name()
76+
),
77+
),
78+
);
79+
continue;
80+
}
81+
let listed = is_designer_listed(context, &designer).map_err(CheckError::Error)?;
82+
if let Some(profile) = listed {
83+
let designer_profile =
84+
protobuf::text_format::parse_from_str::<DesignerInfoProto>(&profile)
85+
.map_err(|e| CheckError::Error(format!("Error parsing info.pb: {}", e)))?;
86+
if normalize_designer_name(designer_profile.designer()) != designer {
87+
problems.push(Status::warn(
88+
"mismatch",
89+
&format!(
90+
"Designer name at METADATA.pb ({}) is not the same as listed on the designers catalog ({}) available at {}/{}/info.pb",
91+
designer,
92+
normalize_designer_name(designer_profile.designer()),
93+
DESIGNER_INFO_RAW_URL, designer
94+
),
95+
),
96+
);
97+
}
98+
if !designer_profile.link().is_empty() {
99+
problems.push(Status::warn(
100+
"link-field",
101+
"Currently the link field is not used by the GFonts API. Designer webpage links should, for now, be placed directly on the bio.html file.",
102+
),
103+
);
104+
}
105+
if designer_profile.avatar.file_name().is_empty() && designer != "Google" {
106+
problems.push(Status::warn(
107+
"missing-avatar",
108+
&format!(
109+
"Designer {} still does not have an avatar image. Please provide one.",
110+
designer
111+
),
112+
));
113+
} else {
114+
let avatar_url = format!(
115+
"{}{}/{}",
116+
DESIGNER_INFO_RAW_URL,
117+
designer,
118+
designer_profile.avatar.file_name()
119+
);
120+
let response = reqwest::blocking::get(&avatar_url).map_err(|e| {
121+
CheckError::Error(format!(
122+
"Error fetching avatar image from {}: {}",
123+
avatar_url, e
124+
))
125+
})?;
126+
if !response.status().is_success() {
127+
problems.push(Status::warn(
128+
"bad-avatar-filename",
129+
&format!(
130+
"The avatar filename provided seems to be incorrect: ({})",
131+
avatar_url
132+
),
133+
));
134+
}
135+
}
136+
} else {
137+
problems.push(Status::warn(
138+
"profile-not-found",
139+
&format!(
140+
"It seems that {} is still not listed on the designers catalog. Please submit a photo and a link to a webpage where people can learn more about the work of this designer/typefoundry.",
141+
designer
142+
),
143+
),
144+
);
145+
continue;
146+
}
147+
}
148+
149+
return_result(problems)
150+
}

‎profile-googlefonts/src/checks/googlefonts/metadata/mod.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,18 @@ mod protos {
2929
#![allow(clippy::all, clippy::unwrap_used)]
3030
include!(concat!(env!("OUT_DIR"), "/protos/mod.rs"));
3131
}
32+
pub(crate) use designers::DesignerInfoProto;
3233
pub(crate) use fonts_public::FamilyProto;
3334
use fontspector_checkapi::{CheckError, Testable};
34-
use protos::fonts_public;
35+
use protos::{designers, fonts_public};
3536

3637
pub(crate) fn family_proto(t: &Testable) -> Result<FamilyProto, CheckError> {
3738
let mdpb = std::str::from_utf8(&t.contents)
3839
.map_err(|_| CheckError::Error("METADATA.pb is not valid UTF-8".to_string()))?;
3940
protobuf::text_format::parse_from_str::<FamilyProto>(mdpb)
4041
.map_err(|e| CheckError::Error(format!("Error parsing METADATA.pb: {}", e)))
4142
}
43+
4244
mod valid_nameid25;
4345
pub use valid_nameid25::valid_nameid25;
4446
#[cfg(not(target_family = "wasm"))]
@@ -49,3 +51,5 @@ mod consistent_repo_urls;
4951
pub use consistent_repo_urls::consistent_repo_urls;
5052
mod primary_script;
5153
pub use primary_script::primary_script;
54+
mod designer_profiles;
55+
pub use designer_profiles::designer_profiles;

‎profile-googlefonts/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl fontspector_checkapi::Plugin for GoogleFonts {
5858
// checks::googlefonts::metadata::valid_filename_values // redundant, see fontbakery#4997
5959
// checks::googlefonts::metadata::undeclared_fonts // redundant, see fontbakery#4997
6060
// checks::googlefonts::metadata::nameid/font_name // redundant, see fontbakery#4581
61-
// .add_and_register_check(checks::googlefonts::metadata::designer_profiles)
61+
.add_and_register_check(checks::googlefonts::metadata::designer_profiles)
6262
.add_and_register_check(checks::googlefonts::metadata::escaped_strings)
6363
// .add_and_register_check(checks::googlefonts::metadata::family_directory_name)
6464
.add_and_register_check(checks::googlefonts::metadata::familyname)

‎profile-googlefonts/src/network_conditions.rs

+46
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,49 @@ pub(crate) fn get_url(
180180
}
181181
request.send().and_then(|r| r.error_for_status())
182182
}
183+
184+
pub const DESIGNER_INFO_RAW_URL: &str =
185+
"https://raw.githubusercontent.com/google/fonts/master/catalog/designers/";
186+
187+
#[cfg(not(target_family = "wasm"))]
188+
pub(crate) fn is_designer_listed(
189+
context: &Context,
190+
designer: &str,
191+
) -> Result<Option<String>, String> {
192+
let key = format!("is_designer_listed:{}", designer);
193+
let get_func = || {
194+
// We don't use get_url here because we don't want error_for_status
195+
let url = format!("{}/{}/info.pb", DESIGNER_INFO_RAW_URL, designer);
196+
let mut request = reqwest::blocking::Client::new().get(&url);
197+
if let Some(timeout) = context.network_timeout {
198+
request = request.timeout(std::time::Duration::new(timeout, 0));
199+
}
200+
let response = request.send();
201+
match response {
202+
Ok(r) => {
203+
if r.status() == reqwest::StatusCode::OK {
204+
Some(r.text().map_err(|e| e.to_string())).transpose()
205+
} else if r.status() == reqwest::StatusCode::NOT_FOUND {
206+
Ok(None)
207+
} else {
208+
Err(format!("Unexpected status code: {}", r.status()))
209+
}
210+
}
211+
Err(e) => Err(format!("Failed to fetch designer info: {}", e)),
212+
}
213+
};
214+
context.cached_question(
215+
&key,
216+
get_func,
217+
|r: Option<String>| Value::String(r.unwrap_or_default()),
218+
|v| {
219+
v.as_str().map_or(Ok(None), |s| {
220+
Ok(if s.is_empty() {
221+
None
222+
} else {
223+
Some(s.to_string())
224+
})
225+
})
226+
},
227+
)
228+
}

‎profile-googlefonts/src/protos/designers.proto

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
syntax = "proto2";
2+
// GF Designer Profile Protos
3+
4+
// A designer listed on the catalog:
5+
message DesignerInfoProto {
6+
// Designer or typefoundry name:
7+
optional string designer = 1;
8+
9+
// URL for more info on this designer:
10+
optional string link = 2;
11+
12+
// Photo or logo:
13+
optional AvatarProto avatar = 3;
14+
}
15+
16+
message AvatarProto {
17+
optional string file_name = 1;
18+
}
19+

0 commit comments

Comments
 (0)