Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify allowed organisation numbers for document sharing #76

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

hermanwh
Copy link
Contributor

No description provided.

@hermanwh hermanwh force-pushed the share-docs-spesific-org branch from e27591d to efc5d28 Compare March 12, 2024 14:20
@hermanwh hermanwh marked this pull request as ready for review March 12, 2024 14:21
@hermanwh hermanwh requested a review from a team March 12, 2024 14:21
@hermanwh hermanwh merged commit b253b01 into main Mar 12, 2024
5 checks passed
@hermanwh hermanwh deleted the share-docs-spesific-org branch March 12, 2024 14:30
@@ -394,6 +394,7 @@
<share-documents-request xmlns="http://api.digipost.no/schema/datatypes">
<max-share-duration-seconds>1209600</max-share-duration-seconds>
<purpose>We require to see your latest pay slip in order to grant you a loan.</purpose>
<allowed-origin-organisation-numbers>984661185</allowed-origin-organisation-numbers>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bare en heads-up på at du har navngitt selve elementet, som kan opptre ingen eller flere ganger, i flertall.
Altså at hvert orgnr. er et "numbers".

<allowed-origin-organisation-numbers>984661185</allowed-origin-organisation-numbers>
<allowed-origin-organisation-numbers>984661186</allowed-origin-organisation-numbers>
<allowed-origin-organisation-numbers>984661187</allowed-origin-organisation-numbers>

Det er nok mer korrekt å navngi elementet som angir ett orgnr i entall, og tillate flere av de (som du allerede gjør).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jeg tenkte på det, men valgte å følge samme scheme som allerede var gjort for tags:

 <xs:element maxOccurs="unbounded" minOccurs="0" name="tags" type="tns:tag"/>

Åpen for at det var et dårlig valg, men det er i det minste konsistent 😅 Eller overser jeg noe?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, det er allerede etablert konvensjon. Nei, skal ikke legge meg opp i det, men et element som angir 1 verdi er nok riktigst å navngi som nettopp en verdi, også kan elementet opptre flere ganger. JAXB har også innebygget støtte for å generere f.eks. Set<Tag> tags, altså at den slenger på flertalls-S på generert feltnavn når maxOccurs > 1 i XSD-en.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja enig 🙌

Jeg har noen ganger valgt å wrappe slike tilfeller i en egen type, f.eks ala

<xsd:complexType name="shared-documents">
    <xsd:sequence>
	<xsd:element name="shared-document" type="shared-document" minOccurs="0" maxOccurs="unbounded" />
    </xsd:sequence>
</xsd:complexType>

<xsd:complexType name="shared-document">
    <xsd:sequence>
	<xsd:element name="delivery-time" type="xsd:dateTime" />
	<xsd:element name="file-type" type="xsd:string" />
        ...
    </xsd:sequence>
</xsd:complexType>

Samme kunne f.eks vært gjort her med tags og tag, allowed-...-numbers og allowed-...-number osv.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja absolutt, det er ofte røddig det! For det så har du jo også @XmlElementWrapper i JAXB som gjør at du kan unngå en ekstra klasse som uansett bare inneholder en collection av elementer.

@runeflobakk
Copy link
Member

Det er ofte når man på et senere tidspunkt finner ut at man gjerne vil assigne "noe mer data" til en samling elementer som opptrer flere ganger, at man skulle ønske at man innførte et wrapper-element rundt de.

F.eks. et helt hypotetisk eksempel:
Man har fra før mulighet til å angi litt tags på et-eller-annet:

<tags>
  <tag>tag1<tag>
  <tag>tag2<tag>
</tags>

Også finner man senere ut at det kan være aktuelt med ulike sett med tags for ulike aktører, og man trenger å skille disse fra hverandre:

<tags>
  <applicable-for>OWNER</applicable-for>
  <tag>tag1</tag>
  <tag>secret-tag</tag>
</tags>
<tags>
  <applicable-for>PUBLIC</applicable-for>
  <tag>tag1</tag>
  <tag>public-tag</tag>
</tags>

Eller ev. applicable-for er et attributt på tags, man trenger uansett en måte å angi noe mer data for ulike sett med tags.
Sånt går an å innføre bakoverkompatibelt med litt nifty bruk av choice i XSD-en, men da er det viktig at de opprinnelige elementene er navngitt i entall, slik at man enten kan ha XML slik:

  <tag>tag1</tag>
  <tag>secret-tag</tag>

Eller slik:

<tags>
  <applicable-for>PUBLIC</applicable-for>
  <tag>tag1</tag>
  <tag>public-tag</tag>
</tags>
<tags>
  ...
</tags>

Java-koden må da mappe til både tag-elementet som en collection, og tags som en collection av en type som inneholder "tagger" og hvem de er for, også sørger skjema-validering for at man alltid bare vil få enten den ene eller andre collection-en.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants