Skip to content

Refactor EADs to not use global state#170

Merged
geonnave merged 8 commits intolake-rs:mainfrom
geonnave:new-ead-architecture
Dec 26, 2023
Merged

Refactor EADs to not use global state#170
geonnave merged 8 commits intolake-rs:mainfrom
geonnave:new-ead-architecture

Conversation

@geonnave
Copy link
Copy Markdown
Collaborator

@geonnave geonnave commented Dec 8, 2023 •
edited
Loading

This is a step in the direction of the separate message processing, initially discussed in #99, and further discussed in the meeting this week.

Edit: this includes:

  • no more global mutable state
  • no more worrying about having the same API on every EAD handler (in fact I don't think it is possible without dyn or some crazy generic stuff)
  • better code organization, including typestated device (U) and authenticator (V)

@geonnave geonnave force-pushed the new-ead-architecture branch from e5e6148 to 79cd4a3 Compare December 11, 2023 10:36
@geonnave
Copy link
Copy Markdown
Collaborator Author

geonnave commented Dec 11, 2023 •
edited
Loading

The new lakers-ead-authz crate is ready by itself, but now it depends on lakers to be updated to support multi-step message processing. Thus for now I'll keep it as draft.

@geonnave
Copy link
Copy Markdown
Collaborator Author

geonnave commented Dec 11, 2023

@chrysn I tried to use the typestate pattern here, could you please take a look? (see the 2 commits with "typestatify" in the name")

@geonnave geonnave force-pushed the new-ead-architecture branch 4 times, most recently from 0633117 to 4fd0db3 Compare December 12, 2023 13:57
@geonnave geonnave force-pushed the new-ead-architecture branch from 4fd0db3 to 8b12f33 Compare December 12, 2023 14:40
@geonnave geonnave marked this pull request as ready for review December 12, 2023 14:44
@geonnave geonnave mentioned this pull request Dec 12, 2023
Merged
7 tasks
@chrysn
Copy link
Copy Markdown
Member

chrysn commented Dec 20, 2023

The typestatification looks about right (as far as I can tell that from mainly looking at types and signatures). It looks like there are fewer types than there were enum states before, but apparently not all were used. Is ZeroTouchAuthenticatorState left around on purpose?

@geonnave
Copy link
Copy Markdown
Collaborator Author

geonnave commented Dec 21, 2023 •
edited
Loading

Thank you for the review.

apparently not all (types) were used

Yes, some were unused and the Error state is now simply a variant of Result.

I can't find the symbol ZeroTouchAuthenticatorState, but maybe you are referring to the whole lakers-ead-zeroconf crate, which I did not remove yet because it breaks tests this new EAD approach cannot be used without the changes in the split PR (#174). I plan to remove it once the split is merged.

@geonnave geonnave merged commit 6a5f0bf into lake-rs:main Dec 26, 2023
@geonnave geonnave deleted the new-ead-architecture branch December 26, 2023 12:24
@geonnave geonnave restored the new-ead-architecture branch December 26, 2023 13:28
@geonnave geonnave deleted the new-ead-architecture branch December 26, 2023 13:28
@geonnave geonnave mentioned this pull request Jan 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

type:enhancement New feature or request type:new feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@geonnave @chrysn