Skip to content

Commit 50046f5

Browse files
partheaparthea
authored
fix: Support additional api paths for an existing library (#2666)
Fixes #2651
1 parent 7c52da2 commit 50046f5

File tree

2 files changed

+241
-6
lines changed

2 files changed

+241
-6
lines changed

‎internal/librarian/generate_command.go‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,19 @@ func (r *generateRunner) generateSingleLibrary(ctx context.Context, libraryID, o
266266
}
267267

268268
func (r *generateRunner) needsConfigure() bool {
269-
return r.api != "" && r.library != "" && r.state.LibraryByID(r.library) == nil
269+
if r.api == "" || r.library == "" {
270+
return false
271+
}
272+
libraryState := r.state.LibraryByID(r.library)
273+
if libraryState == nil {
274+
return true
275+
}
276+
for _, api := range libraryState.APIs {
277+
if api.Path == r.api {
278+
return false
279+
}
280+
}
281+
return true
270282
}
271283

272284
func (r *generateRunner) updateLastGeneratedCommitState(libraryID string) error {
@@ -312,11 +324,7 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context, outputDir stri
312324
}
313325

314326
setAllAPIStatus(r.state, config.StatusExisting)
315-
// Record to state, not write to state.yaml
316-
r.state.Libraries = append(r.state.Libraries, &config.LibraryState{
317-
ID: r.library,
318-
APIs: []*config.API{{Path: r.api, Status: config.StatusNew}},
319-
})
327+
addAPIToLibrary(r.state, r.library, r.api)
320328

321329
if err := populateServiceConfigIfEmpty(
322330
r.state,
@@ -465,3 +473,28 @@ func (r *generateRunner) shouldGenerate(library *config.LibraryState) (bool, err
465473
slog.Info("no APIs have changed; skipping", "library", library.ID)
466474
return false, nil
467475
}
476+
477+
// addAPIToLibrary adds a new API to a library in the state.
478+
// If the library does not exist, it creates a new one.
479+
// If the API already exists in the library, do nothing.
480+
func addAPIToLibrary(state *config.LibrarianState, libraryID, apiPath string) {
481+
lib := state.LibraryByID(libraryID)
482+
if lib == nil {
483+
// If the library is not found, create a new one.
484+
state.Libraries = append(state.Libraries, &config.LibraryState{
485+
ID: libraryID,
486+
APIs: []*config.API{{Path: apiPath, Status: config.StatusNew}},
487+
})
488+
return
489+
}
490+
491+
// If the library is found, check if the API already exists.
492+
for _, api := range lib.APIs {
493+
if api.Path == apiPath {
494+
return
495+
}
496+
}
497+
498+
// For new API paths, set the status to "new".
499+
lib.APIs = append(lib.APIs, &config.API{Path: apiPath, Status: config.StatusNew})
500+
}

‎internal/librarian/generate_command_test.go‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,3 +1433,205 @@ func TestShouldGenerate(t *testing.T) {
14331433
})
14341434
}
14351435
}
1436+
1437+
func TestAddAPIToLibrary(t *testing.T) {
1438+
t.Parallel()
1439+
testCases := []struct {
1440+
name string
1441+
initialState *config.LibrarianState
1442+
libraryID string
1443+
apiPath string
1444+
expectedState *config.LibrarianState
1445+
}{
1446+
{
1447+
name: "add api to existing library",
1448+
initialState: &config.LibrarianState{
1449+
Libraries: []*config.LibraryState{
1450+
{
1451+
ID: "lib1",
1452+
APIs: []*config.API{
1453+
{Path: "api1"},
1454+
},
1455+
},
1456+
},
1457+
},
1458+
libraryID: "lib1",
1459+
apiPath: "api2",
1460+
expectedState: &config.LibrarianState{
1461+
Libraries: []*config.LibraryState{
1462+
{
1463+
ID: "lib1",
1464+
APIs: []*config.API{
1465+
{Path: "api1"},
1466+
{Path: "api2", Status: config.StatusNew},
1467+
},
1468+
},
1469+
},
1470+
},
1471+
},
1472+
{
1473+
name: "add api to new library",
1474+
initialState: &config.LibrarianState{
1475+
Libraries: []*config.LibraryState{
1476+
{
1477+
ID: "lib1",
1478+
APIs: []*config.API{
1479+
{Path: "api1"},
1480+
},
1481+
},
1482+
},
1483+
},
1484+
libraryID: "lib2",
1485+
apiPath: "api2",
1486+
expectedState: &config.LibrarianState{
1487+
Libraries: []*config.LibraryState{
1488+
{
1489+
ID: "lib1",
1490+
APIs: []*config.API{
1491+
{Path: "api1"},
1492+
},
1493+
},
1494+
{
1495+
ID: "lib2",
1496+
APIs: []*config.API{
1497+
{Path: "api2", Status: config.StatusNew},
1498+
},
1499+
},
1500+
},
1501+
},
1502+
},
1503+
{
1504+
name: "add existing api to existing library",
1505+
initialState: &config.LibrarianState{
1506+
Libraries: []*config.LibraryState{
1507+
{
1508+
ID: "lib1",
1509+
APIs: []*config.API{
1510+
{Path: "api1"},
1511+
},
1512+
},
1513+
},
1514+
},
1515+
libraryID: "lib1",
1516+
apiPath: "api1",
1517+
expectedState: &config.LibrarianState{
1518+
Libraries: []*config.LibraryState{
1519+
{
1520+
ID: "lib1",
1521+
APIs: []*config.API{
1522+
{Path: "api1"},
1523+
},
1524+
},
1525+
},
1526+
},
1527+
},
1528+
{
1529+
name: "add api to empty state",
1530+
initialState: &config.LibrarianState{},
1531+
libraryID: "lib1",
1532+
apiPath: "api1",
1533+
expectedState: &config.LibrarianState{
1534+
Libraries: []*config.LibraryState{
1535+
{
1536+
ID: "lib1",
1537+
APIs: []*config.API{
1538+
{Path: "api1", Status: config.StatusNew},
1539+
},
1540+
},
1541+
},
1542+
},
1543+
},
1544+
}
1545+
1546+
for _, tc := range testCases {
1547+
t.Run(tc.name, func(t *testing.T) {
1548+
addAPIToLibrary(tc.initialState, tc.libraryID, tc.apiPath)
1549+
if diff := cmp.Diff(tc.expectedState, tc.initialState); diff != "" {
1550+
t.Errorf("addAPIToLibrary() mismatch (-want +got):\n%s", diff)
1551+
}
1552+
})
1553+
}
1554+
}
1555+
1556+
func TestNeedsConfigure(t *testing.T) {
1557+
t.Parallel()
1558+
testCases := []struct {
1559+
name string
1560+
api string
1561+
library string
1562+
state *config.LibrarianState
1563+
want bool
1564+
}{
1565+
{
1566+
name: "api and library set, library does not exist",
1567+
api: "some/api",
1568+
library: "some-library",
1569+
state: &config.LibrarianState{},
1570+
want: true,
1571+
},
1572+
{
1573+
name: "api and library set, library exists",
1574+
api: "some/api",
1575+
library: "some-library",
1576+
state: &config.LibrarianState{
1577+
Libraries: []*config.LibraryState{
1578+
{
1579+
ID: "some-library",
1580+
},
1581+
},
1582+
},
1583+
want: true,
1584+
},
1585+
{
1586+
name: "api not set",
1587+
api: "",
1588+
library: "some-library",
1589+
state: &config.LibrarianState{},
1590+
want: false,
1591+
},
1592+
{
1593+
name: "library not set",
1594+
api: "some/api",
1595+
library: "",
1596+
state: &config.LibrarianState{},
1597+
want: false,
1598+
},
1599+
{
1600+
name: "api and library not set",
1601+
api: "",
1602+
library: "",
1603+
state: &config.LibrarianState{},
1604+
want: false,
1605+
},
1606+
{
1607+
name: "api and library set, library and api exist",
1608+
api: "some/api",
1609+
library: "some-library",
1610+
state: &config.LibrarianState{
1611+
Libraries: []*config.LibraryState{
1612+
{
1613+
ID: "some-library",
1614+
APIs: []*config.API{
1615+
{Path: "some/api"},
1616+
},
1617+
},
1618+
},
1619+
},
1620+
want: false,
1621+
},
1622+
}
1623+
1624+
for _, tc := range testCases {
1625+
t.Run(tc.name, func(t *testing.T) {
1626+
r := &generateRunner{
1627+
api: tc.api,
1628+
library: tc.library,
1629+
state: tc.state,
1630+
}
1631+
got := r.needsConfigure()
1632+
if got != tc.want {
1633+
t.Errorf("needsConfigure() = %v, want %v", got, tc.want)
1634+
}
1635+
})
1636+
}
1637+
}

0 commit comments

Comments
 (0)