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

Refactor logic to build objects. #76

Merged
merged 4 commits into from
Sep 14, 2021
Merged

Refactor logic to build objects. #76

merged 4 commits into from
Sep 14, 2021

Conversation

demiankatz
Copy link
Collaborator

@demiankatz demiankatz commented Sep 4, 2021

This PR consolidates and simplifies object creation, with the intent of making the server-side implementation of #72 significantly simpler.

TODO:

  • Test everything

@demiankatz
Copy link
Collaborator Author

I've been thinking about how our ingest process works, and in the existing code, there are a lot of manual steps to put together objects which I think are better consolidated into a single function, to increase consistency and simplify calls. This PR attempts to address that problem, and this will serve as a foundation for #72.

The idea is that every object should have three models: the core model (which every object has), the type model (which is either "Collection" or "Document"), and the specific model (Page, PDF, Folder, List, etc., etc.). The old code had us adding these manually one at a time. The new code infers the first two models from the third model, using the configuration as a guide.

We had a lot of methods for adding models, but most of them did very little. A couple of them added sort attributes, but I've just moved that into a switch statement in the single big "initialize object" function.

There is probably still room for improvement here, but I think this is a little tighter and more comprehensible than before.

I'm interested in everyone's thoughts when time permits! Once everyone is happy with this, we can merge it into #72 and wire up the object creation there (along with some validation, since we make some assumptions about integrity during ingest that we cannot safely make when dealing with arbitrary user input).

@demiankatz
Copy link
Collaborator Author

A bit more thought on this led me to the conclusion that while I'm refactoring, I should also move the pid generation logic out of the model and into the new factory service -- it reduces the code further and feels like a better fit. See bdfef4d.

@Geoffsc Geoffsc merged commit 5da2e36 into dev Sep 14, 2021
@demiankatz demiankatz deleted the refactor-object-creation branch September 2, 2022 12:47
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.

None yet

2 participants