Skip to content

Commit bda5c0b

Browse files
srl295srl295
committed
CLDR-18165 cla: sso: cleanup, tests
- add tests - other cleanup per code review
1 parent fff6b45 commit bda5c0b

File tree

6 files changed

+268
-61
lines changed

6 files changed

+268
-61
lines changed

‎tools/cldr-apps/js/src/esm/cldrAuth.mjs

+1-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import * as cldrClient from "../esm/cldrClient.mjs";
22
import * as cldrStatus from "../esm/cldrStatus.mjs";
33

44
/**
5-
* Get the oauth login URL
5+
* Get the oauth login URL. See GithubLoginFactory#getLoginUrl()
66
* @param {object} o options bag
77
* @param {string} o.service which service, currently only 'github' is accepted
88
* @param {string} o.intent what the login URL is used for, currently only 'cla'
@@ -17,7 +17,6 @@ export async function getLoginUrl(o) {
1717
const client = await cldrClient.getClient();
1818
const { url } = (await client.apis.auth.oauthUrl()).body;
1919
const u = new URL(url);
20-
// TODO: may move this into the GithubLoginFactory#getLoginUrl()
2120
const redir = new URL(window.location);
2221
redir.search = "";
2322
redir.hash = "";

‎tools/cldr-apps/pom.xml

+8
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,14 @@
5959
<scope>test</scope>
6060
</dependency>
6161

62+
<dependency>
63+
<groupId>org.assertj</groupId>
64+
<artifactId>assertj-core</artifactId>
65+
<version>${assertj-version}</version>
66+
<scope>test</scope>
67+
</dependency>
68+
69+
6270
<!-- icu -->
6371
<dependency>
6472
<groupId>com.ibm.icu</groupId>

‎tools/cldr-apps/src/main/java/org/unicode/cldr/web/ClaGithubList.java

+136-58
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.google.gson.stream.JsonReader;
55
import java.io.File;
66
import java.io.FileReader;
7+
import java.io.IOException;
78
import java.io.Reader;
89
import java.nio.charset.StandardCharsets;
910
import java.nio.file.Path;
@@ -16,23 +17,51 @@
1617
import org.unicode.cldr.util.CLDRConfig;
1718
import org.unicode.cldr.util.CLDRConfigImpl;
1819

20+
/**
21+
* Utility class for reading a signatures.json file This file can be downloaded from
22+
* cla-assistant.io and placed in the cldr/ server directory. location can be overridden with the
23+
* CLA_FILE configuration preference. Currently, the file is re-read every time a query occurs, to
24+
* allow the file to be updated.
25+
*
26+
* <p>The simplest way to use this is as follows (given the id “github”): <code>
27+
* ClaGithubList.getInstance().getSignStatus("github")</code>
28+
*/
1929
public class ClaGithubList {
30+
/** Only 'signed' indicates a valid CLA. */
2031
public enum SignStatus {
32+
/** The signature was not found. Used by higher level APIs. */
2133
missing,
34+
/** A good CLA was found. */
2235
signed,
36+
/** The CLA was found, but had been revoked. */
2337
revoked,
2438
};
2539

40+
/** One entry in the CLA file. */
2641
public static final class SignEntry {
42+
/** The GitHub user ID */
2743
public String user_name;
44+
45+
/** Signing date, if any. */
2846
public String signed_at;
47+
48+
/** Revocation date, if any. */
2949
public String revoked_at;
50+
51+
/** User's real name. */
3052
public String name;
53+
54+
/** Type of signature */
3155
public String category;
56+
57+
/** User's employer */
3258
public String employer;
59+
60+
/** User's email */
3361
public String email;
3462

35-
// etc - more fields we don't need.
63+
// Note: There are additional fields in the JSON file which
64+
// are not currently read, and these are ignored.
3665

3766
public SignStatus getSignStatus() {
3867
if (revoked_at != null && !revoked_at.isBlank()) {
@@ -68,6 +97,7 @@ public Date getSignedAt() {
6897

6998
static final Logger logger = SurveyLog.forClass(ClaGithubList.class);
7099

100+
/** get the singleton instance */
71101
public static final ClaGithubList getInstance() {
72102
return Helper.INSTANCE;
73103
}
@@ -77,76 +107,46 @@ private static final class Helper {
77107
static final ClaGithubList INSTANCE = new ClaGithubList();
78108
}
79109

80-
static final Gson gson = new Gson();
110+
/** default file path, from configuration */
111+
private final String CLA_FILE;
81112

82-
private final CLDRConfig instance = CLDRConfig.getInstance();
83-
84-
private final String CLA_FILE = instance.getProperty("CLA_FILE", "./signatures.json");
113+
/** full path to .json file */
85114
private final Path claFilePath;
86115

87116
ClaGithubList() {
88-
final File homeFile = ((CLDRConfigImpl) instance).getHomeFile();
89-
claFilePath = new File(homeFile, CLA_FILE).toPath();
90-
logger.info("CLA_FILE=" + claFilePath.toString());
117+
final CLDRConfig instance = CLDRConfig.getInstance();
118+
CLA_FILE = instance.getProperty("CLA_FILE", "./signatures.json");
119+
if (instance instanceof CLDRConfigImpl) {
120+
final File homeFile = ((CLDRConfigImpl) instance).getHomeFile();
121+
claFilePath = new File(homeFile, CLA_FILE).toPath();
122+
logger.fine("CLA_FILE=" + claFilePath.toString());
123+
} else {
124+
claFilePath = null;
125+
logger.fine("claFile path not set, could not get CLDRConfigImpl");
126+
}
91127
}
92128

129+
/**
130+
* @returns the SignEntry for an id, or null if not found
131+
*/
93132
public SignEntry getSignEntry(final String id) {
94-
// Impl: reread each time
95-
Map<String, SignEntry> allSigners = new HashMap<>();
96-
try (final Reader r = new FileReader(claFilePath.toFile(), StandardCharsets.UTF_8);
97-
final JsonReader jr = gson.newJsonReader(r); ) {
98-
jr.beginArray();
99-
while (jr.hasNext()) {
100-
SignEntry e = new SignEntry();
101-
jr.beginObject();
102-
while (jr.hasNext()) {
103-
switch (jr.nextName()) {
104-
case "user_name":
105-
e.user_name = jr.nextString();
106-
break;
107-
case "signed_at":
108-
e.signed_at = jr.nextString();
109-
break;
110-
case "revoked_at":
111-
e.revoked_at = jr.nextString();
112-
break;
113-
case "name":
114-
e.name = jr.nextString();
115-
break;
116-
case "category":
117-
e.category = jr.nextString();
118-
break;
119-
case "employer":
120-
e.employer = jr.nextString();
121-
break;
122-
case "email":
123-
e.email = jr.nextString();
124-
break;
125-
default:
126-
jr.skipValue();
127-
break; // ignore
128-
}
129-
}
130-
jr.endObject();
131-
132-
// the list is in REVERSE order (latest first). So we only track the first
133-
// appearance.
134-
if (!allSigners.containsKey(e.user_name)) {
135-
allSigners.put(e.user_name, e);
136-
// TODO: we could break here.
137-
138-
} // else: ignore, only count first entry
139-
}
140-
jr.endArray();
141-
} catch (Throwable t) {
142-
logger.log(Level.SEVERE, "Trying to read signatures for " + id, t);
133+
Map<String, SignEntry> allSigners;
134+
try {
135+
allSigners = readSigners();
136+
} catch (IOException t) {
137+
logger.log(Level.SEVERE, "Trying to read signatures", t);
138+
return null;
143139
}
144-
logger.info("Read " + allSigners.size() + " signatures");
145140
// Get response
146141
final SignEntry requestedEntry = allSigners.get(id);
147142
return requestedEntry;
148143
}
149144

145+
/**
146+
* This is the simplest and most recommended API for most use cases.
147+
*
148+
* @return the signing status of a GitHub ID, or missing.
149+
*/
150150
public SignStatus getSignStatus(final String id) {
151151
final SignEntry e = getSignEntry(id);
152152
if (e == null) {
@@ -155,4 +155,82 @@ public SignStatus getSignStatus(final String id) {
155155
return e.getSignStatus();
156156
}
157157
}
158+
159+
/** read the default .json file */
160+
Map<String, SignEntry> readSigners() throws IOException {
161+
if (claFilePath == null) {
162+
throw new NullPointerException(
163+
"CLA_FILE=" + CLA_FILE + " but could not find file path.");
164+
}
165+
return readSigners(claFilePath);
166+
}
167+
168+
/** read a specific path */
169+
Map<String, SignEntry> readSigners(final Path path) throws IOException {
170+
try (final Reader r = new FileReader(path.toFile(), StandardCharsets.UTF_8); ) {
171+
return readSigners(r);
172+
}
173+
}
174+
175+
/** read from a Reader */
176+
Map<String, SignEntry> readSigners(final Reader r) throws IOException {
177+
final Gson gson = new Gson();
178+
Map<String, SignEntry> allSigners = new HashMap<>();
179+
try (final JsonReader jr = gson.newJsonReader(r); ) {
180+
jr.beginArray();
181+
while (jr.hasNext()) {
182+
SignEntry e;
183+
try {
184+
e = parseSignEntry(jr);
185+
// the list is in REVERSE order (latest first). So we only track the first
186+
// appearance.
187+
if (!allSigners.containsKey(e.user_name)) {
188+
allSigners.put(e.user_name, e);
189+
}
190+
// else: ignore, we only count the first (most recent entry).
191+
192+
} catch (Throwable t) {
193+
logger.log(Level.SEVERE, "reading SignEntry - will skip", t);
194+
}
195+
}
196+
jr.endArray();
197+
}
198+
logger.info("Read " + allSigners.size() + " signatures");
199+
return allSigners;
200+
}
201+
202+
private SignEntry parseSignEntry(final JsonReader jr) throws IOException {
203+
final SignEntry e = new SignEntry();
204+
jr.beginObject();
205+
while (jr.hasNext()) {
206+
switch (jr.nextName()) {
207+
case "user_name":
208+
e.user_name = jr.nextString();
209+
break;
210+
case "signed_at":
211+
e.signed_at = jr.nextString();
212+
break;
213+
case "revoked_at":
214+
e.revoked_at = jr.nextString();
215+
break;
216+
case "name":
217+
e.name = jr.nextString();
218+
break;
219+
case "category":
220+
e.category = jr.nextString();
221+
break;
222+
case "employer":
223+
e.employer = jr.nextString();
224+
break;
225+
case "email":
226+
e.email = jr.nextString();
227+
break;
228+
default:
229+
jr.skipValue();
230+
break; // ignore
231+
}
232+
}
233+
jr.endObject();
234+
return e;
235+
}
158236
}

‎tools/cldr-apps/src/test/java/org/unicode/cldr/web/TestClaGithubList.java

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
package org.unicode.cldr.web;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.junit.jupiter.api.Assertions.assertAll;
5+
import static org.junit.jupiter.api.Assertions.assertEquals;
6+
import static org.junit.jupiter.api.Assertions.assertFalse;
7+
import static org.junit.jupiter.api.Assertions.assertNotNull;
8+
import static org.junit.jupiter.api.Assertions.assertThrows;
9+
10+
import java.io.IOException;
11+
import java.io.InputStream;
12+
import java.io.InputStreamReader;
13+
import java.io.Reader;
14+
import java.util.Map;
15+
import org.junit.jupiter.api.Test;
16+
import org.unicode.cldr.web.ClaGithubList.SignEntry;
17+
import org.unicode.cldr.web.ClaGithubList.SignStatus;
18+
19+
public class TestClaGithubList {
20+
@Test
21+
void testNull() {
22+
assertNotNull(ClaGithubList.getInstance());
23+
}
24+
25+
@Test
26+
void testFailureMode() {
27+
// this will throw in unit test mode
28+
assertThrows(
29+
NullPointerException.class,
30+
() -> ClaGithubList.getInstance().getSignStatus("github"));
31+
}
32+
33+
@Test
34+
void testCannedData() throws IOException {
35+
final ClaGithubList l = new ClaGithubList();
36+
Map<String, SignEntry> m = null;
37+
38+
// directly read
39+
try (final InputStream is =
40+
TestClaGithubList.class
41+
.getResource("data/" + "test-signatures.json")
42+
.openStream();
43+
final Reader r = new InputStreamReader(is); ) {
44+
m = l.readSigners(r);
45+
}
46+
47+
// make this a final for the assertAll()
48+
final Map<String, SignEntry> s = m;
49+
50+
// some basic asserts before we dig into the data
51+
assertNotNull(s);
52+
assertFalse(s.isEmpty());
53+
assertAll(
54+
"ClaGithubList tests",
55+
() -> assertEquals(3, s.size()),
56+
() ->
57+
assertThat(s.keySet())
58+
.containsExactlyInAnyOrder(
59+
"TEST$ind-signer", "TEST$revokr", "TEST$corp-signer"),
60+
() -> assertEquals(SignStatus.signed, s.get("TEST$ind-signer").getSignStatus()),
61+
() -> assertEquals(SignStatus.signed, s.get("TEST$corp-signer").getSignStatus()),
62+
() -> assertEquals(SignStatus.revoked, s.get("TEST$revoker").getSignStatus()));
63+
}
64+
}

0 commit comments

Comments
 (0)