Merge pull request #3 from osg-htc/pr/auto-generate-id

Automatically generate OSG IID on the backend, validate ROR ID, enable reactivating institutions

Author: mwestphall

institutions-api/app.py

@@ -57,9 +57,4 @@ def invalidate_institution(institution_id: str, request: Request):
     db.invalidate_institution(institution_id, OIDCUserInfo(request))
     return "ok"
 
-@prefix_router.get('/next_institution_id')
-@with_error_logging
-def get_next_institution_id():
-    return { "next_id": db.get_unused_osg_id() }
-
 app.include_router(prefix_router)

institutions-api/db/db.py

@@ -1,10 +1,12 @@
 from sqlalchemy import create_engine, select, delete
 from sqlalchemy.orm import sessionmaker, Session
 from os import environ
+from typing import Optional
 import urllib.parse
 from .db_models import *
 from .error_wrapper import sqlalchemy_http_exceptions
 from ..util.oidc_utils import OIDCUserInfo
+from ..util.ror_utils import validate_ror_id
 from ..models.api_models import InstitutionModel, OSG_ID_PREFIX
 from secrets import choice
 from string import ascii_lowercase, digits
@@ -26,10 +28,34 @@ def _ror_id_type(session: Session) -> IdentifierType:
     """ Get the IdentifierType entity that corresponds to ROR ID """
     return session.scalars(select(IdentifierType).where(IdentifierType.name == ROR_ID_TYPE)).first()
 
-def _full_osg_id(short_id):
+def _full_osg_id(short_id: str):
     """ Get the full osg-htc url of an institution based on its ID suffix """
     return f"{OSG_ID_PREFIX}{short_id}"
 
+def _short_osg_id(full_id: str):
+    """ Get the full osg-htc url of an institution based on its ID suffix """
+    return full_id.replace(OSG_ID_PREFIX, '')
+
+def _get_unused_osg_id(session: Session):
+    """ Generate an unused OSG ID """
+    MAX_TRIES = 1000 # Give up after hitting x collisions in a row
+    ID_LENGTH = 12
+
+    # TODO actually guaranteeing uniqueness here might be a bit overkill based on ID length
+    all_ids = set(session.scalars(select(Institution.topology_identifier)).all())
+    for _ in range(MAX_TRIES):
+        short_id = ''.join([choice(ascii_lowercase + digits) for _ in range(ID_LENGTH)])
+        next_id = f"{OSG_ID_PREFIX}{short_id}"
+        if not next_id in all_ids:
+            return next_id
+    raise HTTPException(500, "Unable to generate new unique ID")
+
+def _check_for_deactivated_institution(session: Session, name: str) -> Optional[str]:
+    """ Check if a deactivated institution with the given name exists. Return its short ID if so.
+    Used for reactivation workflows
+    """
+    deactivated_inst = session.scalar(select(Institution).where(Institution.valid == False).where(Institution.name == name))
+    return _short_osg_id(deactivated_inst.topology_identifier) if deactivated_inst else None
 
 @sqlalchemy_http_exceptions
 def get_valid_institutions() -> List[InstitutionModel]:
@@ -55,8 +81,14 @@ def get_institution_details(short_id: str) -> InstitutionModel:
 @sqlalchemy_http_exceptions
 def add_institution(institution: InstitutionModel, author: OIDCUserInfo):
     """ Create a new institution """
+    validate_ror_id(institution.ror_id)
     with DbSession() as session:
-        inst = Institution(institution.name, institution.id, author.id)
+        if deactivated_id := _check_for_deactivated_institution(session, institution.name):
+            session.rollback()
+            return update_institution(deactivated_id, institution, author)
+
+        topology_id = _get_unused_osg_id(session)
+        inst = Institution(institution.name, topology_id, author.id)
         session.add(inst)
         if institution.ror_id:
             ror_id = InstitutionIdentifier(_ror_id_type(session), institution.ror_id, inst.id)
@@ -86,6 +118,7 @@ def _update_institution_ror_id(session: Session, institution: Institution, ror_i
 @sqlalchemy_http_exceptions
 def update_institution(short_id: str, institution: InstitutionModel, author: OIDCUserInfo):
     """ Update an existing institution """
+    validate_ror_id(institution.ror_id)
     with DbSession() as session:
         to_update = session.scalar(select(Institution)
             .where(Institution.topology_identifier == _full_osg_id(short_id)))
@@ -96,6 +129,7 @@ def update_institution(short_id: str, institution: InstitutionModel, author: OID
         to_update.name = institution.name
         _update_institution_ror_id(session, to_update, institution.ror_id)
         to_update.updated_by = author.id
+        to_update.valid = True
 
         session.commit()
 
@@ -108,19 +142,3 @@ def invalidate_institution(short_id: str, author: OIDCUserInfo):
         to_invalidate.valid = False
         to_invalidate.updated_by = author.id
         session.commit()
-
-@sqlalchemy_http_exceptions
-def get_unused_osg_id():
-    """ Generate an unused OSG ID """
-    MAX_TRIES = 1000 # Give up after hitting x collisions in a row
-    ID_LENGTH = 9
-
-    with DbSession() as session:
-        all_ids = set(session.scalars(select(Institution.topology_identifier)).all())
-        for _ in range(MAX_TRIES):
-            short_id = ''.join([choice(ascii_lowercase + digits) for _ in range(ID_LENGTH)])
-            next_id = f"{OSG_ID_PREFIX}{short_id}"
-            if not next_id in all_ids:
-                return next_id
-
-        raise HTTPException(500, "Unable to generate new unique ID")

institutions-api/db/db_models.py

@@ -59,7 +59,6 @@ class InstitutionIdentifier(Base):
 
     identifier_type: Mapped["IdentifierType"] = relationship()
 
-
     def __init__(self, identifier_type: IdentifierType, identifier: str, institution_id: UUID = None):
         self.identifier_type_id = identifier_type.id
         self.identifier = identifier

institutions-api/models/api_models.py

@@ -9,7 +9,7 @@
 class InstitutionModel(BaseModel):
     """ API model for topology institutions """
     name: str = Field(..., description="The name of the institution")
-    id: str = Field(..., description="The institution's OSG ID")
+    id: Optional[str] = Field(None, description="The institution's OSG ID")
     ror_id: Optional[str] = Field(None, description="The institution's research organization registry id (https://ror.org/)")
 
     @classmethod
@@ -23,6 +23,6 @@ def from_institution(cls, inst: Institution) -> "InstitutionModel":
     @model_validator(mode='after')
     def check_id_format(self):
         assert self.name, "Name must be non-empty"
-        assert self.id.startswith(OSG_ID_PREFIX), f"OSG ID must start with '{OSG_ID_PREFIX}'"
+        assert (not self.id) or self.id.startswith(OSG_ID_PREFIX), f"OSG ID must start with '{OSG_ID_PREFIX}'"
         assert (not self.ror_id) or self.ror_id.startswith(ROR_ID_PREFIX), f"ROR ID must be empty or start with '{ROR_ID_PREFIX}'"
         return self

institutions-api/util/ror_utils.py

@@ -0,0 +1,11 @@
+from typing import Optional
+from fastapi import HTTPException
+import requests
+from ..models.api_models import ROR_ID_PREFIX
+
+
+def validate_ror_id(ror_id: Optional[str]):
+    """ Check whether an ROR ID is valid by performing a HEAD request to ror.org """
+    # TODO we might want to rate-limit this in some way
+    if ror_id and requests.head(ror_id, allow_redirects=True).status_code != 200:
+        raise HTTPException(400, f"Invalid ROR ID: institution does not exist. See {ROR_ID_PREFIX}.")

institutions-ui/src/app/institutions-editor/institutions-editor.component.html

@@ -4,9 +4,6 @@ <h3 *ngIf="!institutionId">Create Institution</h3>
     <div>
         <span>Institution Name</span> <input type="text" [(ngModel)]="institution.name"/>
     </div>
-    <div *ngIf="!institutionId">
-        <span>Institution ID</span> <input type="text" [(ngModel)]="institution.id"/>
-    </div>
     <div>
         <span>Institution ROR ID</span><input type="text" [(ngModel)]="institution.ror_id"/>
     </div>

institutions-ui/src/app/institutions-editor/institutions-editor.component.ts

@@ -20,7 +20,7 @@ export class InstitutionsEditorComponent implements OnInit {
 
   institutionId?: string;
 
-  institution: Institution = {name:'', id: this.OSG_ID_PREFIX , ror_id: this.ROR_ID_PREFIX };
+  institution: Institution = {name:'', id: '' , ror_id: this.ROR_ID_PREFIX };
 
   errorMessage?: string;
 
@@ -35,9 +35,7 @@ export class InstitutionsEditorComponent implements OnInit {
   ngOnInit(): void {
     if(this.institutionId) {
       this.instService.getInstitutionDetails(this.institutionId).subscribe(inst=>this.institution=inst)
-    } else {
-      this.instService.getNextInstitutionId().subscribe(({next_id})=>this.institution.id = next_id)
-    }
+    } 
   }
 
   extractErrorMessage(err: any) {
@@ -47,8 +45,8 @@ export class InstitutionsEditorComponent implements OnInit {
 
   sanitizeFormData() {
     // clean up form data prior to submission
-    let {name, id, ror_id } = this.institution
-    this.institution = { name: name.trim(), id: id.trim(), ror_id: ror_id?.trim() }
+    let {name, ror_id } = this.institution
+    this.institution = { name: name.trim(), ror_id: ror_id?.trim() }
   }
 
   submitInstitution(): void {

institutions-ui/src/app/institutions-list/institutions-list.component.ts

@@ -24,6 +24,6 @@ export class InstitutionsListComponent implements OnInit{
   }
 
   editRouteFor(inst: Institution) {
-    return `edit/${this.instService.shortId(inst.id)}`
+    return `edit/${this.instService.shortId(inst.id!)}`
   }
 }

institutions-ui/src/app/institutions/institutions.models.ts

@@ -1,7 +1,7 @@
 
 export interface Institution {
     name: String
-    id: String
+    id?: String
     ror_id?: String
 }
 

institutions-ui/src/app/institutions/institutions.service.ts

@@ -38,9 +38,4 @@ export class InstitutionsService {
     return this.http.delete(`${BASE_URL}/institutions/${this.shortId(institutionId)}`);
   }
 
-  getNextInstitutionId() {
-    return this.http.get<NextId>(`${BASE_URL}/next_institution_id`)
-  }
-
-
 }